Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21335 )
Change subject: IMPALA-13022: Added infrastructure for implicit casting of functions ...................................................................... Patch Set 15: (6 comments) http://gerrit.cloudera.org:8080/#/c/21335/15/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/21335/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@200 PS15, Line 200: private static RelNode processSortNode(RelNode relNode, List<RelNode> inputs, : RexBuilder rexBuilder, boolean isInputChanged) { : return isInputChanged ? relNode.copy(relNode.getTraitSet(), inputs) : relNode; : } > Are there any expressions in the sort that need to be handled? e.g. the sor The sort expressions just consist of a number within the sort. If there is an actual expression with a function applied, it will appear in a Project RelNode http://gerrit.cloudera.org:8080/#/c/21335/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@211 PS15, Line 211: Additional 'cast' RexCalls will be added into the Project : * and the AggregateCall will point into these input columns. > Does this have any significance to the tuple layout or is it purely logical I think I overlooked this one. This will affect the layout. This will complicate the logic within CoerceNodes, but I'd imagine it is doable. I'd have to map all used projects, even in the aggCalls that weren't affected, and remove the ones that are no longer used. Should I do this now or should I file a Jira? http://gerrit.cloudera.org:8080/#/c/21335/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@335 PS15, Line 335: getTighterDataType > I get the meaning that it is avoiding unnecessary widening, but the name is Done http://gerrit.cloudera.org:8080/#/c/21335/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@396 PS15, Line 396: * compare 2 datatypes and return a datatype that > Also, this comment looks unfinished Done http://gerrit.cloudera.org:8080/#/c/21335/15/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/21335/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@61 PS15, Line 61: Impala-XXX > Fill this in when you get a chance Done http://gerrit.cloudera.org:8080/#/c/21335/15/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@92 PS15, Line 92: // Eliminate the "Sarg" function which is unknown to Impala. : // TODO: this is kinda hacky. It would be better if Impala can handle this : // directly, so this needs investigation. > My understanding is that sargability is related to the ability to use index Ack -- To view, visit http://gerrit.cloudera.org:8080/21335 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I13a349673f185463276ad7ddb83f0cfc2d73218c Gerrit-Change-Number: 21335 Gerrit-PatchSet: 15 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: Sun, 08 Sep 2024 00:48:35 +0000 Gerrit-HasComments: Yes
