Skip to content

fix(sql): collect dependent columns during top-down projection propagation#6600

Merged
bluestreak01 merged 13 commits intomasterfrom
projection_topdown_with_projection_bug
Jan 8, 2026
Merged

fix(sql): collect dependent columns during top-down projection propagation#6600
bluestreak01 merged 13 commits intomasterfrom
projection_topdown_with_projection_bug

Conversation

@kafka1991
Copy link
Copy Markdown
Collaborator

@kafka1991 kafka1991 commented Jan 6, 2026 •
edited
Loading

Close #6579

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 6, 2026

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kafka1991 kafka1991 changed the title fix(sql): Projection topdown with projection bug fix(sql): collect dependent columns during top-down projection propagation Jan 6, 2026
@kafka1991 kafka1991 changed the title fix(sql): collect dependent columns during top-down projection propagation fix(sql): collect dependent columns during top-down projection propagation Jan 6, 2026
@kafka1991 kafka1991 added the Bug Incorrect or unexpected behavior label Jan 6, 2026
bluestreak01
bluestreak01 requested changes Jan 7, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@bluestreak01 bluestreak01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a test for

select x as a, a as a from t

@bluestreak01
Copy link
Copy Markdown
Member

bluestreak01 commented Jan 7, 2026

Also please check if other models, such as group-by have similar issues?

@kafka1991
Copy link
Copy Markdown
Collaborator Author

kafka1991 commented Jan 8, 2026

could you add a test for

select x as a, a as a from t

test added

@kafka1991
Copy link
Copy Markdown
Collaborator Author

kafka1991 commented Jan 8, 2026 •
edited
Loading

Also please check if other models, such as group-by have similar issues?

Other query models shouldn't have same issues, because by the time the optimizer reaches this step, function expression it must already in a virtualModelType model.

GroupByModel can also have function expressions, but it never has (and does not support) the column where an expression references other columns from the same model.

@glasstiger
Copy link
Copy Markdown
Contributor

glasstiger commented Jan 8, 2026

[PR Coverage check]

😍 pass : 38 / 38 (100.00%)

file detail

path covered line new line coverage
🔵 io/questdb/griffin/SqlCodeGenerator.java 3 3 100.00%
🔵 io/questdb/griffin/SqlOptimiser.java 35 35 100.00%

@bluestreak01 bluestreak01 merged commit 7a4654e into master Jan 8, 2026
43 checks passed
@bluestreak01 bluestreak01 deleted the projection_topdown_with_projection_bug branch January 8, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bluestreak01 bluestreak01 bluestreak01 approved these changes

Assignees

No one assigned

Labels

Bug Incorrect or unexpected behavior

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Calculated columns aliases error on avg nested queries

3 participants

@kafka1991 @bluestreak01 @glasstiger