Skip to content

Apply filter reduce expressions Calcite rule at the end to prevent literal-only filter pushdown to leaf stage #14025

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
yashmayya wants to merge 2 commits into from
Closed

Apply filter reduce expressions Calcite rule at the end to prevent literal-only filter pushdown to leaf stage #14025

yashmayya wants to merge 2 commits into from

Conversation

@yashmayya
Copy link
Contributor

@yashmayya yashmayya commented Sep 18, 2024 

LogicalProject(AirTime=[CASE(>($4, 0), _UTF-8'Positive', _UTF-8'Negative')])
  LogicalFilter(condition=[OR(AND(>($4, 0), OR(=(_UTF-8'Positive', _UTF-8'Negative'), =(_UTF-8'Positive', _UTF-8'Positive'))), AND(OR(=(_UTF-8'Negative', _UTF-8'Negative'), =(_UTF-8'Negative', _UTF-8'Positive')), <=($4, 0)))])
    LogicalTableScan(table=[[default, mytable]])
  • This is a problem because literal-only filter expressions are being pushed down to the leaf stage which doesn't support evaluating such filter expressions. The above query failed with the error:
java.lang.RuntimeException: Caught exception while running CombinePlanNode.
...
Caused by: java.lang.NullPointerException: DataSource for null should not be null. Potentially invalid column name specified.
  • Moving the FILTER_REDUCE_EXPRESSIONS Calcite rule to the end ensures that these literal-only filter expressions will be reduced. The plan for the same query now looks like:
LogicalProject(AirTime=[CASE(>($4, 0), _UTF-8'Positive', _UTF-8'Negative')])
  LogicalTableScan(table=[[default, mytable]])

@yashmayya yashmayya added bugfix multi-stage Related to the multi-stage query engine labels Sep 18, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 18, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.02%. Comparing base (59551e4) to head (816399d).
Report is 2193 commits behind head on master.

Additional details and impacted files 
@@             Coverage Diff              @@
##             master   #14025      +/-   ##
============================================
+ Coverage     61.75%   65.02%   +3.27%     
- Complexity      207     1533    +1326     
============================================
  Files          2436     2564     +128     
  Lines        133233   140748    +7515     
  Branches      20636    21607     +971     
============================================
+ Hits          82274    91525    +9251     
+ Misses        44911    42483    -2428     
- Partials       6048     6740     +692
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 64.98% <ø> (+3.27%) ⬆️
java-21 64.91% <ø> (+3.29%) ⬆️
skip-bytebuffers-false 65.00% <ø> (+3.25%) ⬆️
skip-bytebuffers-true 64.89% <ø> (+37.16%) ⬆️
temurin 65.02% <ø> (+3.27%) ⬆️
unittests 65.02% <ø> (+3.27%) ⬆️
unittests1 56.54% <ø> (+9.65%) ⬆️
unittests2 35.01% <ø> (+7.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yashmayya yashmayya force-pushed the filter-reduce-expressions-rule-reorder branch from ba7b577 to 816399d Compare September 19, 2024 12:03
@yashmayya
Copy link
Contributor Author

yashmayya commented Sep 19, 2024

I just realized that this is the same as #13174.

cc - @gortiz

@gortiz
Copy link
Contributor

gortiz commented Sep 19, 2024

Yep. It looks very similar to #13174. The reason why we didn't merged that is because it may increase the planning time and at the same time we are not sure we are getting rid of all literal pushdown cases.

@yashmayya
Copy link
Contributor Author

yashmayya commented Sep 23, 2024

Closing this since it's basically a duplicate.

@yashmayya yashmayya closed this Sep 23, 2024
@yashmayya yashmayya mentioned this pull request Nov 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bugfix multi-stage Related to the multi-stage query engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Query execution error when filtering constant columns from subquery

3 participants

@yashmayya @codecov-commenter @gortiz