Skip to content

Conversation

@walterddr
Copy link
Contributor

@walterddr walterddr commented Feb 1, 2024

follow up with #12305

this should also close #12035

@Jackie-Jiang Jackie-Jiang added testing multi-stage Related to the multi-stage query engine labels Feb 1, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 1, 2024

Codecov Report

❌ Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.73%. Comparing base (17db0fd) to head (86a3027).
⚠️ Report is 2963 commits behind head on master.

Files with missing lines Patch % Lines
...ime/operator/operands/TransformOperandFactory.java 50.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #12349      +/-   ##
============================================
+ Coverage     61.69%   61.73%   +0.03%     
  Complexity      207      207              
============================================
  Files          2424     2424              
  Lines        132340   132512     +172     
  Branches      20436    20481      +45     
============================================
+ Hits          81651    81804     +153     
- Misses        44693    44711      +18     
- Partials       5996     5997       +1     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 61.69% <50.00%> (+26.66%) ⬆️
java-21 61.61% <50.00%> (+0.03%) ⬆️
skip-bytebuffers-false 61.70% <50.00%> (+0.01%) ⬆️
skip-bytebuffers-true 61.58% <50.00%> (+0.02%) ⬆️
temurin 61.73% <50.00%> (+0.03%) ⬆️
unittests 61.72% <50.00%> (+0.03%) ⬆️
unittests1 46.93% <50.00%> (+0.01%) ⬆️
unittests2 27.69% <0.00%> (+0.05%) ⬆️

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.

"sql": "SELECT bool_col, COALESCE(min(double_col) FILTER (WHERE string_col = 'a' OR string_col = 'b'), 0), COALESCE(max(double_col) FILTER (WHERE string_col = 'a' OR int_col > 10), 0), avg(double_col), sum(double_col), count(double_col), count(distinct(double_col)) FILTER (WHERE string_col = 'b' OR int_col > 10), count(string_col) FROM {tbl} WHERE string_col='b' GROUP BY bool_col"
},
{
"sql": "SELECT string_col, count(bool_col) FILTER ( WHERE double_col NOT IN (1, 3, 5, 7)) FROM {tbl} WHERE double_col < 10 AND int_col BETWEEN 1 AND 1 AND int_col <> 1 GROUP BY string_col"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a super corner case situation:

  • Where clause reduces the filter into a no-match (xxx between 1 and 1 and xxx <> 1)
  • A futher agg filter on top of an already filtered logic (e.g. yyy < 10 and yyy NOT IN (1, 3, 5, 7)

then it triggered some rule that didn't merge the filter pushdown and causes the NOT_IN to be eval as an actual method.

@walterddr walterddr merged commit 9314c11 into apache:master Feb 6, 2024
suyashpatel98 pushed a commit to suyashpatel98/pinot that referenced this pull request Feb 28, 2024
* adding case tests for IN/NOT-IN operation
* adding filter agg test for IN/NOT_IN as well
* reordered switch case

---------

Co-authored-by: Rong Rong <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug][multistage] IN/NOT_IN function issue

4 participants