Skip to content

Comments

Skip computation incase of Null Literal for case/when transformation#11444

Merged
Jackie-Jiang merged 4 commits intoapache:masterfrom
abhioncbr:case-when-null-literal-performance
Aug 29, 2023
Merged

Skip computation incase of Null Literal for case/when transformation#11444
Jackie-Jiang merged 4 commits intoapache:masterfrom
abhioncbr:case-when-null-literal-performance

Conversation

@abhioncbr
Copy link
Contributor

@abhioncbr abhioncbr commented Aug 25, 2023

As per the discussion of the PR

  • Skipping computation of value block in case of Null Literal for case/when transformation.

cc @shenyu0127 @Jackie-Jiang

@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2023

Codecov Report

Merging #11444 (b876be1) into master (2b40362) will increase coverage by 0.02%.
Report is 2 commits behind head on master.
The diff coverage is 83.33%.

@@             Coverage Diff              @@
##             master   #11444      +/-   ##
============================================
+ Coverage     62.99%   63.02%   +0.02%     
+ Complexity     1107     1095      -12     
============================================
  Files          2303     2302       -1     
  Lines        124041   124032       -9     
  Branches      18895    18901       +6     
============================================
+ Hits          78137    78168      +31     
+ Misses        40352    40320      -32     
+ Partials       5552     5544       -8     
Flag Coverage Δ
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 62.99% <83.33%> (+0.02%) ⬆️
java-17 62.85% <83.33%> (+<0.01%) ⬆️
java-20 62.86% <83.33%> (+0.01%) ⬆️
temurin 63.02% <83.33%> (+0.02%) ⬆️
unittests 63.01% <83.33%> (+0.02%) ⬆️
unittests1 67.56% <83.33%> (+0.03%) ⬆️
unittests2 14.47% <0.00%> (+<0.01%) ⬆️

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

Files Changed Coverage Δ
...ator/transform/function/CaseTransformFunction.java 57.32% <81.81%> (+1.74%) ⬆️
...r/transform/function/LiteralTransformFunction.java 74.19% <100.00%> (-0.81%) ⬇️

... and 28 files with indirect coverage changes

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

@abhioncbr abhioncbr marked this pull request as ready for review August 27, 2023 01:50
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

Copy link
Contributor

@shenyu0127 shenyu0127 left a comment

Choose a reason for hiding this comment

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

If would be good to add a test to CaseTransformFunctionTest which uses a query "SELECT CASE WHEN xx THEN xx ELSE NULL" and assert the query results to exercise the modified code paths.

@shenyu0127
Copy link
Contributor

If would be good to add a test to CaseTransformFunctionTest which uses a query "SELECT CASE WHEN xx THEN xx ELSE NULL" and assert the query results to exercise the modified code paths.

Did you add a test? The rest looks good to me.

@Jackie-Jiang Jackie-Jiang merged commit 2e1e475 into apache:master Aug 29, 2023
KKcorps pushed a commit to KKcorps/incubator-pinot that referenced this pull request Sep 5, 2023
@abhioncbr abhioncbr deleted the case-when-null-literal-performance branch April 25, 2024 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants