Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/24208 )
Change subject: IMPALA-14903: Calcite planner: Simplify code for string literals ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/24208/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24208/5//COMMIT_MSG@47 PS5, Line 47: it is possible that an cast of a tinyint to an integer gets simplified to an int > typo: "that a cast" Done http://gerrit.cloudera.org:8080/#/c/24208/5//COMMIT_MSG@68 PS5, Line 68: return type is no longer changed, and the nullbility of the rank function is now set > typo: "nullability" Done http://gerrit.cloudera.org:8080/#/c/24208/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java: http://gerrit.cloudera.org:8080/#/c/24208/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@313 PS5, Line 313: private static RelNode processValuesNode(RelNode relNode, List<RelNode> inputs, > This doesn't appear to override anything. It looks like it's kept just to k Makes sense to just remove it. http://gerrit.cloudera.org:8080/#/c/24208/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java: http://gerrit.cloudera.org:8080/#/c/24208/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java@98 PS5, Line 98: // Always treat all string literals as type STRING > This comment is no longer clear. A comment probably isn't necessary, but I'll put one in if you think I should. http://gerrit.cloudera.org:8080/#/c/24208/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java: http://gerrit.cloudera.org:8080/#/c/24208/5/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@196 PS5, Line 196: // ImpalaCoreRules.FILTER_VALUES_MERGE, > Does this imply some sort of performance regression? Potentially, but I'd imagine it would be pretty small. It could involve having a SelectNode on top of a UnionNode. But the UnionNode for constants isn't going to be very many rows. Also, 1.42 is coming out very soon. It looks like it is in process and I'd imagine it is coming out within a month. -- To view, visit http://gerrit.cloudera.org:8080/24208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id8e61b2555afd81ef52f19431fdd1224d4039c00 Gerrit-Change-Number: 24208 Gerrit-PatchSet: 5 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: Mon, 04 May 2026 16:21:42 +0000 Gerrit-HasComments: Yes
