Hello Aman Sinha, Joe McDonnell, Michael Smith, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24690
to look at the new patch set (#2).
Change subject: IMPALA-15256: Calcite planner returning wrong results for
floor, ceil
......................................................................
IMPALA-15256: Calcite planner returning wrong results for floor, ceil
The floor and ceil functions were returning a wrong result for
negative values, e.g. floor(-5.2) was returning -5.
This was caused by a regression from IMPALA-14904, which cast the
operand for floor to match the return type. floor(-5.2) was being
treated as floor(cast(-5.2 as decimal(2,0)) which truncated the
decimal part.
This commit ensures that this will not happen for these functions.
The "ceil" keyword had to be treated as a ReservedFunctionName word in
Parser.jj
Also, the Parser.jj file changed to ensure that we do not use the default
Calcite operators to determine the precision.
The new ImpalaAdjustPrecisionFunction operator matches the logic
for floor and ceil that exists in FunctionCallExpr. Note that this
adds one to the precision of the return type. I'm not sure this is
necessary since a decimal like -9.9 already has a precision of 2 as
it gets changed to -10, but the logic now matches the original
planner.
The functions for ImpalaAdjustScaleFunction also ensure that the
param is not changed to be the same as the return type. I did a scan
through all the math functions and these are the only functions
where the return type for decimal does not need to match the param
type. The default logic is kept the same.
Tests: Added floor and ceil tests to both original and calcite planner.
Change-Id: I794cb181ebca59dae880eb96ae0f0570b5b9f8ed
---
M java/calcite-planner/src/main/codegen/templates/Parser.jj
M
java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java
A
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaAdjustPrecisionFunction.java
M
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
M
testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q02.test
M testdata/workloads/functional-query/queries/QueryTest/calcite.test
M testdata/workloads/functional-query/queries/QueryTest/exprs.test
7 files changed, 139 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/24690/2
--
To view, visit http://gerrit.cloudera.org:8080/24690
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I794cb181ebca59dae880eb96ae0f0570b5b9f8ed
Gerrit-Change-Number: 24690
Gerrit-PatchSet: 2
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
