Joe McDonnell 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 20: (2 comments) 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 = > What kind of unit test are you thinking here? I'm assuming this is implementing the same thing as Impala's default compatibility. If it isn't, then that's a can of worms we would need to deal with (and document). The goal of the test would be to make sure this is producing the same decision as Impala's default compatibility. i.e. compare the decision from supportsImplicitCasting(X, Y) to what Impala would return for the Impala equivalents of X and Y. http://gerrit.cloudera.org:8080/#/c/21335/18/java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/ImplicitTypeChecker.java@78 PS18, Line 78: .add(Pair.of(SqlTypeName.DECIMAL, SqlTypeName.DECIMAL)) : .build(); > So this is interesting... If I remove this? One of my unit tests in calcit Nevermind, it looks like there are specific cases where float and double can't be converted to decimal. That applies when using strict compatibility. The documentation says that this applies for insert and union, but I can't get it to complain for my union statements. I would need to look at it more closely. I think we can leave these for now. -- 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: 20 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: Wed, 18 Sep 2024 23:20:17 +0000 Gerrit-HasComments: Yes
