[
https://issues.apache.org/jira/browse/CALCITE-6885?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ruben Q L updated CALCITE-6885:
-------------------------------
Description:
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}
was:
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 regression 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 behave in the same way to avoid any
regression:
{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}
> 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
> Priority: Major
> Labels: pull-request-available
>
> 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)
