Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1870,7 +1870,7 @@ public RelNode convertToSingleValueSubq(
if (leftKeys.size() == 1) {
SqlCall sqlCall =
comparisonOp.createCall(rightVals.getParserPosition(), leftKeys.get(0), rightVals);
rexComparison = bb.convertExpression(sqlCall);
rexComparison = ensureComparisonTypes(bb.convertExpression(sqlCall));
} else {
assert rightVals instanceof SqlCall;
final SqlBasicCall call = (SqlBasicCall) rightVals;
Expand All @@ -1880,9 +1880,10 @@ public RelNode convertToSingleValueSubq(
RexUtil.composeConjunction(rexBuilder,
transform(
Pair.zip(leftKeys, call.getOperandList()),
pair -> bb.convertExpression(
comparisonOp.createCall(rightVals.getParserPosition(),
pair.left, pair.right))));
pair -> ensureComparisonTypes(
bb.convertExpression(
comparisonOp.createCall(rightVals.getParserPosition(),
pair.left, pair.right)))));
}
comparisons.add(rexComparison);
}
Expand All @@ -1901,6 +1902,31 @@ public RelNode convertToSingleValueSubq(
}
}

/**
* Ensures that a comparison expression has matching operand types. If the
* operands have different type names, casts the right operand to match the
* left operand's type. This handles the case where type coercion is disabled
* and the IN-to-OR expansion produces comparisons with mismatched types
* (e.g., DATE = CHAR).
*/
private RexNode ensureComparisonTypes(RexNode node) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this produce the same result as type coercion would?
Why not the leastRestrictive type?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, same result, before the fix it didn't

Why not the leastRestrictive type?

  1. follow the approach that was before CALCITE-6435
  2. in case of no leastRestrictive it will fail with NPE which is not user friednly
    in case of cast failure the error message will contain more helpful error description

if (validator != null && validator.config().typeCoercionEnabled()) {
return node;
}
if (node instanceof RexCall) {
final RexCall call = (RexCall) node;
if (call.operands.size() == 2) {
final RexNode left = call.operands.get(0);
final RexNode right = call.operands.get(1);
if (left.getType().getSqlTypeName() != right.getType().getSqlTypeName()) {
final RexNode castRight = rexBuilder.ensureType(left.getType(), right, true);
return rexBuilder.makeCall(call.getOperator(), left, castRight);
}
}
}
return node;
}

/**
* Converts a {@link SqlNodeList} (for example an IN-list or VALUES list)
* into a relational expression and produces a Rex-level sub-query that
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2213,6 +2213,24 @@ void checkCorrelatedMapSubQuery(boolean expand) {
sql(sql).withExpand(false).ok();
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-7562">[CALCITE-7562]
* SqlToRel misses CAST in case IN expression without type coercion</a>. */
@Test void testInDateColumnWithoutTypeCoercion() {
final String sql =
"select * from emp_b where birthdate in ('2000-06-30', '2000-09-27')";
sql(sql).withTypeCoercion(false).ok();
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-7562">[CALCITE-7562]
* SqlToRel misses CAST in case IN expression without type coercion</a>. */
@Test void testInDateColumnWithTypeCoercion() {
final String sql =
"select * from emp_b where birthdate in ('2000-06-30', '2000-09-27')";
sql(sql).ok();
}

@Test void testInValueListLong() {
// Go over the default threshold of 20 to force a sub-query.
final String sql = "select empno from emp where deptno in"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3266,6 +3266,30 @@ LogicalFilter(condition=[=($cor0.DEPTNO, $7)])
LogicalAggregate(group=[{0}], S=[SUM($1)], agg#1=[COUNT()])
LogicalProject(DEPTNO=[$7], SAL=[$5])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
</Resource>
</TestCase>
<TestCase name="testInDateColumnWithTypeCoercion">
<Resource name="sql">
<![CDATA[select * from emp_b where birthdate in ('2000-06-30', '2000-09-27')]]>
</Resource>
<Resource name="plan">
<![CDATA[
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], BIRTHDATE=[$9])
LogicalFilter(condition=[OR(=($9, CAST('2000-06-30'):DATE NOT NULL), =($9, CAST('2000-09-27'):DATE NOT NULL))])
LogicalTableScan(table=[[CATALOG, SALES, EMP_B]])
]]>
</Resource>
</TestCase>
<TestCase name="testInDateColumnWithoutTypeCoercion">
<Resource name="sql">
<![CDATA[select * from emp_b where birthdate in ('2000-06-30', '2000-09-27')]]>
</Resource>
<Resource name="plan">
<![CDATA[
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8], BIRTHDATE=[$9])
LogicalFilter(condition=[OR(=($9, CAST('2000-06-30'):DATE NOT NULL), =($9, CAST('2000-09-27'):DATE NOT NULL))])
LogicalTableScan(table=[[CATALOG, SALES, EMP_B]])
]]>
</Resource>
</TestCase>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,11 @@ public SqlToRelFixture withConformance(SqlConformance conformance) {
.withValidatorConfig(c -> c.withConformance(conformance)));
}

public SqlToRelFixture withTypeCoercion(boolean enabled) {
return withFactory(f ->
f.withValidatorConfig(c -> c.withTypeCoercionEnabled(enabled)));
}

public SqlToRelFixture withDiffRepos(DiffRepository diffRepos) {
return new SqlToRelFixture(sql, decorrelate, tester, factory, trim,
expression, diffRepos);
Expand Down
Loading