[ https://issues.apache.org/jira/browse/CALCITE-6885?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17934608#comment-17934608 ]
Ruben Q L commented on CALCITE-6885: ------------------------------------ Thanks for the feedback, [~zabetak]. The PR got approval from [~mbudiu] (thanks for the quick review)! I have squashed commits, I think it's good to go. Since we are in a code-freeze situation in the middle of a RC generation, I'll let you handle the merge as RM, [~zabetak]. > SqlToRelConverter#convertUsing should not fail if > commonTypeForBinaryComparison returns null > -------------------------------------------------------------------------------------------- > > Key: CALCITE-6885 > URL: https://issues.apache.org/jira/browse/CALCITE-6885 > Project: Calcite > Issue Type: Bug > Components: core > Reporter: Ruben Q L > Assignee: Ruben Q L > Priority: Blocker > Labels: pull-request-available > Fix For: 1.39.0 > > > CALCITE-6413 introduced the following code in > {{SqlToRelConverter#convertUsing}} : > {code} > RelDataType resultType = > validator().getTypeCoercion().commonTypeForBinaryComparison( > comparedTypes.get(0), comparedTypes.get(1)); > if (resultType == null) { > // This should never happen, since the program has been validated. > throw new IllegalArgumentException("Cannot join on field `" + name > + "` because the types are not comparable: " + comparedTypes); > } > {code} > This can cause regressions on downstream projects which use their own > TypeCoercion, and which decide to return {{null}} in some circumstances, e.g. > to avoid forcing any coercion from Calcite planner side, and leave the > original expression as it is because they rely on a runtime engine coercion > (outside of Calcite scope). > Notice that the above code (to convert Join's using condition) is NOT aligned > with code for binary call expressions, as we can see here: > https://github.com/apache/calcite/blob/main/core/src/main/java/org/apache/calcite/sql/validate/implicit/TypeCoercionImpl.java#L244 > If the commonTypeForBinaryComparison returns null, then the binary call is > left untouched (no coercion), and the process goes on, without any exception. > {{SqlToRelConverter#convertUsing}} should probably behave in the same way to > avoid any regression, i.e.: > {code} > RelDataType resultType = > validator().getTypeCoercion().commonTypeForBinaryComparison( > comparedTypes.get(0), comparedTypes.get(1)); > if (resultType == null) { > // keep old code, no expcetion > list.add(rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, operands)); > } else { > // new code from CALCITE-6413 > List<RexNode> castedOperands = new ArrayList<>(); > ... > list.add(rexBuilder.makeCall(SqlStdOperatorTable.EQUALS, > } > {code} -- This message was sent by Atlassian Jira (v8.20.10#820010)