Skip to content

Comments

[multistage][hotfix] fix filter operator type convert#9450

Merged
walterddr merged 3 commits intoapache:masterfrom
walterddr:pr_hotfix_filter_type_cast
Sep 23, 2022
Merged

[multistage][hotfix] fix filter operator type convert#9450
walterddr merged 3 commits intoapache:masterfrom
walterddr:pr_hotfix_filter_type_cast

Conversation

@walterddr
Copy link
Contributor

@walterddr walterddr commented Sep 22, 2022

FilterOperand should use the non-cast result type, the current behavior to always use the left-hand side is wrong b/c sometimes there's a type cast on the left.

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2022

Codecov Report

Merging #9450 (2325dc8) into master (44da348) will decrease coverage by 34.89%.
The diff coverage is 35.29%.

@@              Coverage Diff              @@
##             master    #9450       +/-   ##
=============================================
- Coverage     69.88%   34.99%   -34.90%     
+ Complexity     5099      185     -4914     
=============================================
  Files          1897     1902        +5     
  Lines        101320   101577      +257     
  Branches      15395    15423       +28     
=============================================
- Hits          70807    35542    -35265     
- Misses        25518    62992    +37474     
+ Partials       4995     3043     -1952     
Flag Coverage Δ
integration1 25.84% <0.00%> (-0.13%) ⬇️
integration2 ?
unittests1 ?
unittests2 15.46% <35.29%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
...query/runtime/operator/operands/FilterOperand.java 62.29% <35.29%> (-3.78%) ⬇️
...in/java/org/apache/pinot/spi/utils/BytesUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/BaseRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/NoOpRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/config/table/FSTType.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/config/user/RoleType.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/data/MetricFieldSpec.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/common/tier/TierFactory.java 0.00% <0.00%> (-100.00%) ⬇️
...a/org/apache/pinot/spi/config/table/TableType.java 0.00% <0.00%> (-100.00%) ⬇️
.../org/apache/pinot/spi/data/DimensionFieldSpec.java 0.00% <0.00%> (-100.00%) ⬇️
... and 1106 more

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

@siddharthteotia
Copy link
Contributor

Let's add a test for = operator as well

@walterddr walterddr merged commit fea74a4 into apache:master Sep 23, 2022
61yao pushed a commit to 61yao/pinot that referenced this pull request Oct 3, 2022
FilterOperand should use the non-cast result type, 
- fix the current behavior to always use the left-hand side for type casting
- fixing type resulting in null issue

Co-authored-by: Rong Rong <rongr@startree.ai>
@walterddr walterddr deleted the pr_hotfix_filter_type_cast branch December 6, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants