Skip to content

Conversation

@zhtaoxiang
Copy link
Contributor

@zhtaoxiang zhtaoxiang commented Sep 6, 2023

Map float and double to the same RelDataType to disable calcite float to double cast so that queries like select count(*) from table where aFloatColumn = 0.05 works correctly in multi-stage query engine.

Without this PR, select count(*) from table where aFloatColumn = 0.05 will be converted to select count(*) from table where CAST(aFloatColumn as "DOUBLE") = 0.05. While casting from float to double does not always produce the same double value as the original float value, this leads to wrong query result.

With this PR, the behavior in multi-stage query engine will be the same as the query in v1 query engine.

@codecov-commenter
Copy link

codecov-commenter commented Sep 6, 2023

Codecov Report

Merging #11518 (9df6837) into master (5ed8fb0) will decrease coverage by 0.02%.
Report is 1 commits behind head on master.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master   #11518      +/-   ##
============================================
- Coverage     63.08%   63.07%   -0.02%     
  Complexity     1108     1108              
============================================
  Files          2320     2320              
  Lines        124597   124598       +1     
  Branches      19022    19022              
============================================
- Hits          78604    78586      -18     
- Misses        40385    40411      +26     
+ Partials       5608     5601       -7     
Flag Coverage Δ
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 63.05% <100.00%> (-0.01%) ⬇️
java-17 62.90% <100.00%> (-0.02%) ⬇️
java-20 62.91% <100.00%> (-0.03%) ⬇️
temurin 63.07% <100.00%> (-0.02%) ⬇️
unittests 63.06% <100.00%> (-0.02%) ⬇️
unittests1 67.49% <100.00%> (+0.02%) ⬆️
unittests2 14.49% <0.00%> (-0.04%) ⬇️

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

Files Changed Coverage Δ
.../java/org/apache/pinot/query/type/TypeFactory.java 68.96% <100.00%> (ø)

... and 15 files with indirect coverage changes

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

@gortiz
Copy link
Contributor

gortiz commented Jul 18, 2024

We are going to rollback this change in #13616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants