Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/22086 )
Change subject: IMPALA-13523: Decimal precision and scale needs to be in return type ...................................................................... Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/22086/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/22086/8//COMMIT_MSG@7 PS8, Line 7: Precision > nit: precision and scale Done http://gerrit.cloudera.org:8080/#/c/22086/8//COMMIT_MSG@9 PS8, Line 9: Inferred > nit: lowercase 'inferred' Done http://gerrit.cloudera.org:8080/#/c/22086/8//COMMIT_MSG@16 PS8, Line 16: select appx_median(c1), appx_median(c2), appx_median(c3) > nit: it's hard to tell what gets fixed in this query. Suggest putting the Changed the text, but it was throwing an exception before this commit http://gerrit.cloudera.org:8080/#/c/22086/8//COMMIT_MSG@23 PS8, Line 23: 1) when the function is a case statement, in which all cases need > I would think the same logic that applies to a CASE .. WHEN would also appl Didn't see one so I added a new test http://gerrit.cloudera.org:8080/#/c/22086/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java: http://gerrit.cloudera.org:8080/#/c/22086/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@304 PS8, Line 304: fn.getNumArgs() > Can we be sure that this method is only called when getNumArgs() > 0 ? If I added a Precondition for this. It wouldn't be in this loop if there were no argTypes http://gerrit.cloudera.org:8080/#/c/22086/8/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@378 PS8, Line 378: if (fromType.getSqlTypeName().equals(SqlTypeName.NULL)) { : if (toType.getSqlTypeName().equals(SqlTypeName.DECIMAL)) { > It would be better to use the public static methods in Calcite's SqlTypeUti Done -- To view, visit http://gerrit.cloudera.org:8080/22086 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie10521b587a74930a01c08b711364f897bb2dc33 Gerrit-Change-Number: 22086 Gerrit-PatchSet: 8 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]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Tue, 04 Feb 2025 01:08:27 +0000 Gerrit-HasComments: Yes
