Skip to content

[multistage] Support IN and NOT-IN Clauses#9374

Merged
walterddr merged 8 commits intoapache:masterfrom
ankitsultana:inc-1
Sep 15, 2022
Merged

[multistage] Support IN and NOT-IN Clauses#9374
walterddr merged 8 commits intoapache:masterfrom
ankitsultana:inc-1

Conversation

@ankitsultana
Copy link
Contributor

@ankitsultana ankitsultana commented Sep 10, 2022

Related to #9345

Query to verify:

SELECT COUNT(*)
        FROM baseballStats_OFFLINE
        WHERE teamID IN ('BL2', 'FL1')

cc: @walterddr

Copy link
Contributor

@walterddr walterddr 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 some test to validate this?

@walterddr
Copy link
Contributor

this should be included in #9375

@ankitsultana
Copy link
Contributor Author

@walterddr : I was working on the IN/NOT-IN/Range support. Does #9375 take care of that as well?

@ankitsultana ankitsultana changed the title [multistage] Support IN Clause With 1 Argument [multistage] Support IN and NOT-IN Clauses Sep 12, 2022
@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2022

Codecov Report

Merging #9374 (3a1cbd6) into master (ff2a333) will increase coverage by 5.08%.
The diff coverage is 92.30%.

@@             Coverage Diff              @@
##             master    #9374      +/-   ##
============================================
+ Coverage     63.40%   68.49%   +5.08%     
- Complexity     4762     4780      +18     
============================================
  Files          1832     1886      +54     
  Lines         98146   100428    +2282     
  Branches      15020    15282     +262     
============================================
+ Hits          62231    68786    +6555     
+ Misses        31321    26701    -4620     
- Partials       4594     4941     +347     
Flag Coverage Δ
integration1 26.13% <0.00%> (?)
unittests1 66.97% <92.30%> (+0.04%) ⬆️
unittests2 15.36% <92.30%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...inot/query/planner/logical/RexExpressionUtils.java 88.88% <88.88%> (ø)
...che/pinot/query/planner/logical/RexExpression.java 74.35% <100.00%> (ø)
...inot/query/runtime/operator/AggregateOperator.java 84.21% <100.00%> (-0.17%) ⬇️
...e/pinot/segment/local/io/util/PinotDataBitSet.java 95.62% <0.00%> (-1.46%) ⬇️
...org/apache/pinot/core/common/ObjectSerDeUtils.java 90.71% <0.00%> (-0.78%) ⬇️
...ugin/inputformat/avro/KafkaAvroMessageDecoder.java 0.00% <0.00%> (ø)
...t/server/starter/ServerQueriesDisabledTracker.java 86.36% <0.00%> (ø)
...server/starter/helix/SegmentReloadStatusValue.java 0.00% <0.00%> (ø)
...rg/apache/pinot/server/starter/ServerInstance.java 73.38% <0.00%> (ø)
...inot/server/api/resources/HealthCheckResource.java 0.00% <0.00%> (ø)
... and 367 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@walterddr
Copy link
Contributor

walterddr commented Sep 12, 2022

@walterddr : I was working on the IN/NOT-IN/Range support. Does #9375 take care of that as well?

ah. no that only takes care of the CHAR/VARCHAR codepath to decode NlsString into regular String. but that should be able to consolidate easily.

could you add some test cases to validate your change for IN as well?

@Jackie-Jiang Jackie-Jiang added the multi-stage Related to the multi-stage query engine label Sep 13, 2022
Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

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

looks good to me overall with the approach. left a few comments. plz kindly take a look

@walterddr walterddr merged commit 61fc919 into apache:master Sep 15, 2022
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants