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: (3 comments) http://gerrit.cloudera.org:8080/#/c/21335/16/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/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java@209 PS16, Line 209: * Instead, a Project RelNode is placed as an input to the Aggregate RelNode. The > I don't see any changes uploaded here. Heh, I know I made the change because I explicitly remember i had to reformat multiple lines due to line length! Oh well...should be there now. http://gerrit.cloudera.org:8080/#/c/21335/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java: http://gerrit.cloudera.org:8080/#/c/21335/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java@72 PS16, Line 72: .add(SqlKind.PLUS) > Can you add a comment explaining that? Done http://gerrit.cloudera.org:8080/#/c/21335/16/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java@90 PS16, Line 90: return getFunction(call.getOperator().getName(), call.getKind(), argTypes, false); > I meant adding Ok, I think I see what you're saying now. Well...perhaps I just made it worse in your opinion, but I like your idea with the shorter signatures? Tell me what you think of what I did... I got rid of the "exactMatch" parameter because I can easily see a caller screwing that up. Instead, I made the name explicit. But since the name "getFunction" is a big generic, I created getExactFunction and getSupertypeFunction. This way, the caller knows exactly what they are calling. The public get*Function calls are now wrappers around the internals, which all use private getFunction calls. What do you think? I'm still willing to change it to a different way. -- 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: Fri, 13 Sep 2024 14:59:53 +0000 Gerrit-HasComments: Yes
