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 18: (4 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@211 PS15, Line 211: regate. Additional 'cast' RexCalls will be added into the Project : * and the AggregateCall will point into these input columns. > I'm unclear about how projects translate down to tuple layout. Oh...ok! Actually, I take back my previous comment. I think we're good! Yeah, you're right. The Project is a function of columns of the underlying node. Furthermore, the Aggregate node only grabs the expressions it needs. So any extra expressions should get thrown away. This is already coded into the Aggregate. So I think we're good here. http://gerrit.cloudera.org:8080/#/c/21335/18/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/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@539 PS18, Line 539: The last "false" parameter makes sure that we : // are not looking for an exact match, just one where type conversion can : // be done. > This no longer applies now that we use getSupertypeFunction() Done http://gerrit.cloudera.org:8080/#/c/21335/18/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/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@201 PS18, Line 201: alter > Nit: alternate Done http://gerrit.cloudera.org:8080/#/c/21335/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/ImplicitTypeChecker.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/ImplicitTypeChecker.java: http://gerrit.cloudera.org:8080/#/c/21335/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/ImplicitTypeChecker.java@42 PS18, Line 42: private static Set<Pair<SqlTypeName, SqlTypeName>> SUPPORTED_IMPLICIT_CASTS = > So, we have this table for Calcite types. Is there some relationship to the I think this should be the same as strict compatibility. Not sure right now about allow_unsafe_casts. But probably? The caller just decides whether to put an explicit "CAST" function around it, and as long as we support it in some way, it sounds easy enough. Having said that, the Jira you mentioned says it's only for set and insert operations? I'm not sure what "set" operations are, but this Calcite code does not support "insert" operations yet. -- 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: 18 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, 17 Sep 2024 16:34:09 +0000 Gerrit-HasComments: Yes
