fix: handle nested arrays in UNNEST by checking for grouping in left query (#17949) - #20094
Open
zhang-arvin wants to merge 1 commit into
Open
fix: handle nested arrays in UNNEST by checking for grouping in left query (#17949)#20094zhang-arvin wants to merge 1 commit into
zhang-arvin wants to merge 1 commit into
Conversation
FrankChen021
left a comment
Member
There was a problem hiding this comment.
| Severity | Findings |
|---|---|
| P0 | 0 |
| P1 | 1 |
| P2 | 0 |
| P3 | 0 |
| Total | 1 |
Reviewed 1 of 1 changed files.
This is an automated review by Codex GPT-5.6-Luna(max)
| final RowSignature leftSignature = DruidRels.dataSourceSignature(newLeftDruidRel); | ||
| if (whereFilter == null) { | ||
| if (computeLeftRequiresSubquery(newLeftDruidRel)) { | ||
| if (computeLeftRequiresSubquery(newLeftDruidRel) || updatedLeftQuery.getQuery() instanceof GroupByQuery) { |
Member
There was a problem hiding this comment.
[P1] GroupBy detection is unreachable
This check runs only when the partial-query stage is SCAN, WHERE_FILTER, or SELECT_PROJECT. DruidQuery emits GroupByQuery only when an aggregate exists, which places the stage at AGGREGATE or later. Therefore the new condition cannot change the data-source choice for the reported grouped-left-side case, leaving the fix ineffective.
zhang-arvin
force-pushed
the
fix/issue-17949-unnest-nested-arrays
branch
from
August 21, 2026 15:50
ce94d2c to
45a1b0c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #17949: UNNEST with nested arrays fails when the left side of the correlate contains GROUP BY aggregation.
Root Cause
DruidCorrelateUnnestRel.toDruidQuery()usescomputeLeftRequiresSubquery()to determine whether to wrap the left query in aQueryDataSource. However,computeLeftRequiresSubquery()only checks thePartialDruidQuerystage (SCAN), but when UNNEST is pulled up above a GROUP BY, the left side may contain grouping/aggregation pushed into theDruidQueryitself. In this case,getDataSource()returns the raw table scan instead of a subquery that wraps the aggregation.Fix
Added an additional check in
toDruidQuery(): when the computedupdatedLeftQueryis aGroupByQuery(i.e., contains grouping), wrap it in aQueryDataSourceeven ifcomputeLeftRequiresSubquery()returns false. This ensures the grouping/aggregation logic is preserved in the UNNEST data source.Changes
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidCorrelateUnnestRel.java: Addedinstanceof GroupByQuerycheck in thetoDruidQuery()method to detect when the left query contains grouping, and useQueryDataSourceaccordingly.