dssysolyatin commented on code in PR #4572:
URL: https://github.com/apache/calcite/pull/4572#discussion_r2443272059
##########
core/src/main/java/org/apache/calcite/sql/validate/implicit/TypeCoercionImpl.java:
##########
@@ -766,16 +765,10 @@ private boolean coerceSourceRowType(
targetType);
case UPDATE:
SqlUpdate update = (SqlUpdate) query;
- final SqlNodeList sourceExpressionList =
update.getSourceExpressionList();
- if (sourceExpressionList != null) {
- return coerceColumnType(sourceScope, sourceExpressionList,
columnIndex, targetType);
- } else {
- // Note: this is dead code since sourceExpressionList is always
non-null
- return coerceSourceRowType(sourceScope,
- castNonNull(update.getSourceSelect()),
- columnIndex,
- targetType);
- }
+ return coerceSourceRowType(sourceScope,
Review Comment:
Coerce whole source SELECT instead of just sourceExpressionList, then grab
the last N items from sourceSelect and assign them to expressionList inside
`validateUpdate`, because sourceSelect already contains expressionList
##########
core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml:
##########
Review Comment:
Changes in these tests:
1. Casts are inside LogicalProject instead of LogicalTableModify
2. LogicalTableModify now only contains RexInputRef
##########
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##########
@@ -5599,15 +5599,6 @@ protected void checkTypeAssignment(
// matched
boolean isUpdateModifiableViewTable = false;
if (query instanceof SqlUpdate) {
- final SqlNodeList targetColumnList =
Review Comment:
It’s not needed anymore because the whole source select is now validated
##########
core/src/test/resources/org/apache/calcite/test/TypeCoercionConverterTest.xml:
##########
@@ -281,8 +281,8 @@ LogicalUnion(all=[false])
</Resource>
<Resource name="plan">
<![CDATA[
-LogicalTableModify(table=[[CATALOG, SALES, T1]], operation=[UPDATE],
updateColumnList=[[t1_varchar20, t1_date, t1_int]],
sourceExpressionList=[[CAST(123):VARCHAR(20) NOT NULL, CAST(2020-01-03
10:14:34):DATE NOT NULL, CAST(12.3:DECIMAL(3, 1)):INTEGER NOT NULL]],
flattened=[false])
- LogicalProject(t1_varchar20=[$0], t1_smallint=[$1], t1_int=[$2],
t1_bigint=[$3], t1_real=[$4], t1_double=[$5], t1_decimal=[$6],
t1_timestamp=[$7], t1_date=[$8], t1_binary=[$9], t1_boolean=[$10],
EXPR$0=[123], EXPR$1=[2020-01-03 10:14:34], EXPR$2=[12.3:DECIMAL(3, 1)])
+LogicalTableModify(table=[[CATALOG, SALES, T1]], operation=[UPDATE],
updateColumnList=[[t1_varchar20, t1_date, t1_int]], sourceExpressionList=[[$11,
$12, $13]], flattened=[false])
+ LogicalProject(t1_varchar20=[$0], t1_smallint=[$1], t1_int=[$2],
t1_bigint=[$3], t1_real=[$4], t1_double=[$5], t1_decimal=[$6],
t1_timestamp=[$7], t1_date=[$8], t1_binary=[$9], t1_boolean=[$10],
EXPR$0=['123':VARCHAR(20)], EXPR$1=[2020-01-03], EXPR$2=[12])
Review Comment:
It’s correct that `CAST(2020-01-03 10:14:34):DATE NOT NULL` got simplified
to `2020-01-03`. This didn’t happen before because the cast used to be inside
LogicalTableModify and was applied to a RexInputRef. But now it’s in the
`selectList` and applied directly to literals, so RexSimplify just folded it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
