Skip to content
Open
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
30 changes: 30 additions & 0 deletions core/src/main/java/org/apache/calcite/rel/core/TableModify.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
import org.apache.calcite.rel.metadata.RelMetadataQuery;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rex.RexNode;
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.type.SqlTypeUtil;

Expand Down Expand Up @@ -261,9 +263,37 @@ public boolean isMerge() {
null);
}

inputRowType = expectedInputRowTypeForAssignment(inputRowType);
return inputRowType;
}

private RelDataType expectedInputRowTypeForAssignment(
RelDataType expectedRowType) {
final RelDataType actualRowType = getInput().getRowType();
if (actualRowType.getFieldCount() != expectedRowType.getFieldCount()) {
return expectedRowType;
}
final RelDataTypeFactory typeFactory = getCluster().getTypeFactory();
final RelDataTypeFactory.Builder builder = typeFactory.builder();
boolean changed = false;
final List<RelDataTypeField> expectedFields = expectedRowType.getFieldList();
final List<RelDataTypeField> actualFields = actualRowType.getFieldList();
for (int i = 0; i < expectedFields.size(); i++) {
final RelDataTypeField expectedField = expectedFields.get(i);
final RelDataType actualType = actualFields.get(i).getType();
final RelDataType expectedType = expectedField.getType();
if (!SqlTypeUtil.equalSansNullability(typeFactory, actualType, expectedType)
&& SqlTypeUtil.canAssignFrom(expectedType, actualType)
&& !RexUtil.isLosslessCast(actualType, expectedType)) {
builder.add(expectedField.getName(), actualType);
changed = true;
} else {
builder.add(expectedField);
}
}
return changed ? builder.build() : expectedRowType;
}

@Override public RelWriter explainTerms(RelWriter pw) {
return super.explainTerms(pw)
.item("table", table.getQualifiedName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rex.RexUtil;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlCallBinding;
import org.apache.calcite.sql.SqlFunction;
Expand Down Expand Up @@ -830,8 +831,13 @@ && coerceOperandType(binding.getScope(), binding.getCall(), i, implicitType)
}
boolean coerced = false;
for (int i = 0; i < sourceFields.size(); i++) {
RelDataType sourceType = sourceFields.get(i).getType();
RelDataType targetType = targetFields.get(i).getType();
coerced = coerceSourceRowType(scope, query, i, targetType) || coerced;
if (!SqlTypeUtil.equalSansNullability(validator.getTypeFactory(), sourceType, targetType)
&& (RexUtil.isLosslessCast(sourceType, targetType)
|| !SqlTypeUtil.canAssignFrom(targetType, sourceType))) {
coerced = coerceSourceRowType(scope, query, i, targetType) || coerced;
}
}
return coerced;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9932,7 +9932,7 @@ private void checkLiteral2(String expression, String expected) {
+ "ON \"DEPT\".\"DEPTNO\" = \"DEPT0\".\"DEPTNO\"\n"
+ "WHEN MATCHED THEN UPDATE SET \"DNAME\" = \"DEPT\".\"DNAME\"\n"
+ "WHEN NOT MATCHED THEN INSERT (\"DEPTNO\", \"DNAME\", \"LOC\") "
+ "VALUES CAST(\"DEPT\".\"DEPTNO\" + 1 AS TINYINT),\n"
+ "VALUES \"DEPT\".\"DEPTNO\" + 1,\n"
+ "LOWER(\"DEPT\".\"DNAME\"),\n"
+ "UPPER(\"DEPT\".\"LOC\")";
sql(sql1)
Expand Down Expand Up @@ -9966,9 +9966,9 @@ private void checkLiteral2(String expression, String expected) {
+ "ON \"DEPT\".\"DEPTNO\" = \"DEPT0\".\"DEPTNO\"\n"
+ "WHEN MATCHED THEN UPDATE SET \"DNAME\" = \"DEPT\".\"DNAME\"\n"
+ "WHEN NOT MATCHED THEN INSERT (\"DEPTNO\", \"DNAME\", \"LOC\") "
+ "VALUES CAST(\"DEPT\".\"DEPTNO\" + 1 AS TINYINT),\n"
+ "VALUES \"DEPT\".\"DEPTNO\" + 1,\n"
+ "'abc',\n"
+ "CAST(LOWER(\"DEPT\".\"DNAME\") AS VARCHAR(13) CHARACTER SET \"ISO-8859-1\")";
+ "LOWER(\"DEPT\".\"DNAME\")";
sql(sql3)
.schema(CalciteAssert.SchemaSpec.JDBC_SCOTT)
.ok(expected3);
Expand Down Expand Up @@ -9998,7 +9998,7 @@ private void checkLiteral2(String expression, String expected) {
+ "USING \"SCOTT\".\"DEPT\"\n"
+ "ON \"DEPT\".\"DEPTNO\" = \"DEPT0\".\"DEPTNO\"\n"
+ "WHEN NOT MATCHED THEN INSERT (\"DEPTNO\", \"DNAME\", \"LOC\") "
+ "VALUES CAST(\"DEPT\".\"DEPTNO\" + 1 AS TINYINT),\n"
+ "VALUES \"DEPT\".\"DEPTNO\" + 1,\n"
+ "LOWER(\"DEPT\".\"DNAME\"),\n"
+ "UPPER(\"DEPT\".\"LOC\")";
sql(sql5)
Expand All @@ -10018,7 +10018,7 @@ private void checkLiteral2(String expression, String expected) {
+ "WHERE CAST(\"DEPTNO\" AS INTEGER) <> 5) AS \"t0\"\n"
+ "ON \"t0\".\"DEPTNO\" = \"DEPT0\".\"DEPTNO\"\n"
+ "WHEN NOT MATCHED THEN INSERT (\"DEPTNO\", \"DNAME\", \"LOC\") "
+ "VALUES CAST(\"t0\".\"DEPTNO\" + 1 AS TINYINT),\n"
+ "VALUES \"t0\".\"DEPTNO\" + 1,\n"
+ "LOWER(\"t0\".\"DNAME\"),\n"
+ "UPPER(\"t0\".\"LOC\")";
sql(sql6)
Expand All @@ -10040,7 +10040,7 @@ private void checkLiteral2(String expression, String expected) {
+ "ON \"t0\".\"EXPR$0\" = \"t1\".\"DEPTNO0\"\n"
+ "WHEN MATCHED THEN UPDATE SET \"DNAME\" = 'abc'\n"
+ "WHEN NOT MATCHED THEN INSERT (\"DEPTNO\", \"DNAME\", \"LOC\") "
+ "VALUES CAST(\"t0\".\"EXPR$0\" + 1 AS TINYINT),\n"
+ "VALUES \"t0\".\"EXPR$0\" + 1,\n"
+ "CAST(LOWER(\"t0\".\"EXPR$1\") AS VARCHAR(14) CHARACTER SET \"ISO-8859-1\"),\n"
+ "CAST(UPPER(\"t0\".\"EXPR$2\") AS VARCHAR(13) CHARACTER SET \"ISO-8859-1\")";
sql(sql7)
Expand Down Expand Up @@ -10229,9 +10229,8 @@ private void checkLiteral2(String expression, String expected) {
final String sql0 = "update \"foodmart\".\"product\" "
+ "set \"product_name\" = \"product_name\" || '_'\n"
+ "where \"product_id\" > 10";
final String expected0 = "UPDATE \"foodmart\".\"product\" SET \"product_name\" = CAST"
+ "(\"product_name\" || '_' AS VARCHAR(60) CHARACTER SET \"ISO-8859-1\")\nWHERE "
+ "\"product_id\" > 10";
final String expected0 = "UPDATE \"foodmart\".\"product\" SET \"product_name\" = "
+ "\"product_name\" || '_'\nWHERE \"product_id\" > 10";
sql(sql0).ok(expected0);

final String sql1 = "update \"foodmart\".\"product\""
Expand All @@ -10247,8 +10246,8 @@ private void checkLiteral2(String expression, String expected) {
+ " \"product_name\" = \"product_name\" || '_' \n"
+ "where \"product_id\" > 10";
final String expected2 = "UPDATE \"foodmart\".\"product\" SET \"product_id\" = \"product_id\""
+ " + CHAR_LENGTH(\"product_name\"), \"product_name\" = CAST(\"product_name\" || '_' AS "
+ "VARCHAR(60) CHARACTER SET \"ISO-8859-1\")\nWHERE \"product_id\" > 10";
+ " + CHAR_LENGTH(\"product_name\"), \"product_name\" = \"product_name\" || '_'"
+ "\nWHERE \"product_id\" > 10";
sql(sql2).ok(expected2);

final String sql3 = "update \"foodmart\".\"product\"\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1456,21 +1456,21 @@ private LockWrapper exclusiveCleanDb(Connection c) throws SQLException {
+ " JdbcTableModify(table=[[foodmart, expense_fact]], operation=[MERGE],"
+ " updateColumnList=[[amount]], flattened=[false])\n"
+ " JdbcProject(STORE_ID=[$0], $f1=[666], $f2=[1997-01-01 00:00:00], $f3=[666],"
+ " $f4=['666':VARCHAR(30)], $f5=[666], AMOUNT=[CAST($1):DECIMAL(10, 4) NOT NULL],"
+ " $f4=['666':VARCHAR(30)], $f5=[666], AMOUNT=[$1],"
+ " store_id=[$2],"
+ " account_id=[$3], exp_date=[$4], time_id=[$5], category_id=[$6], currency_id=[$7],"
+ " amount=[$8], AMOUNT0=[CAST($1):DECIMAL(10, 4) NOT NULL])\n"
+ " amount=[$8], AMOUNT0=[$1])\n"
+ " JdbcJoin(condition=[=($2, $0)], joinType=[left])\n"
+ " JdbcValues(tuples=[[{ 666, 42 }]])\n"
+ " JdbcTableScan(table=[[foodmart, expense_fact]])\n";
final String jdbcSql = "MERGE INTO \"foodmart\".\"expense_fact\"\n"
+ "USING (VALUES (666, 42)) AS \"t\" (\"STORE_ID\", \"AMOUNT\")\n"
+ "ON \"t\".\"STORE_ID\" = \"expense_fact\".\"store_id\"\n"
+ "WHEN MATCHED THEN UPDATE SET \"amount\" = CAST(\"t\".\"AMOUNT\" AS DECIMAL(10, 4))\n"
+ "WHEN MATCHED THEN UPDATE SET \"amount\" = \"t\".\"AMOUNT\"\n"
+ "WHEN NOT MATCHED THEN INSERT (\"store_id\", \"account_id\", \"exp_date\", \"time_id\", "
+ "\"category_id\", \"currency_id\", \"amount\") VALUES \"t\".\"STORE_ID\",\n"
+ "666,\nTIMESTAMP '1997-01-01 00:00:00',\n666,\n'666',\n666,\n"
+ "CAST(\"t\".\"AMOUNT\" AS DECIMAL(10, 4))";
+ "\"t\".\"AMOUNT\"";
final AssertThat that =
CalciteAssert.model(FoodmartSchema.FOODMART_MODEL)
.enable(CalciteAssert.DB == DatabaseInstance.HSQLDB);
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/apache/calcite/test/JdbcTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ static void forEachExpand(Runnable r) {
+ "expr#7=[null:JavaType(class java.lang.Integer)], "
+ "empid=[$t3], deptno=[$t4], name=[$t5], salary=[$t6], "
+ "commission=[$t7])\n"
+ " EnumerableValues(tuples=[[{ 'Fred', 56, 123.4000015258789E0 }]])\n";
+ " EnumerableValues(tuples=[[{ 'Fred', 56, 123.4 }]])\n";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These variations in precision may also require an explanation.

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.

The original SQL uses 123.4. The old plan cast the value through a narrower floating-point type, which produced 123.4000015258789E0; without that cast the value stays 123.4.

assertThat(resultSet.getString(1), isLinux(expected));

// With named columns
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5972,7 +5972,7 @@ private void checkEmptyJoin(RelOptFixture f) {
@Test void testReduceCastsNullable() {
HepProgram program = new HepProgramBuilder()

// Simulate the way INSERT will insert casts to the target types
// Simulate the way INSERT can insert casts to the target types.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand why this comment needs to be changed.

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.

I changed it because CoerceInputsRule may now decide not to add a cast. I can revert this comment if you think it is better to avoid the unrelated diff.

.addRuleInstance(
CoerceInputsRule.Config.DEFAULT
.withCoerceNames(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,4 +236,36 @@ public static void checkActualAndReferenceFiles() {
String sql = "select CAST(null AS INTEGER) union select '10'";
sql(sql).ok();
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-7607">[CALCITE-7607]
* Avoid implicit narrowing casts in DML assignment coercion</a>. */
@Test void testInsertVarcharNarrowingKeepsSourceType() {
final String sql = "insert into t1 select "
+ "CAST('AVeryLongLongStringValue' AS VARCHAR(64)), "
+ "t2_smallint, t2_int, t2_bigint,\n"
+ "t2_real, t2_double, t2_decimal, t2_timestamp, "
+ "t2_date, t2_binary, t2_boolean from t2";
sql(sql).ok();
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-7607">[CALCITE-7607]
* Avoid implicit narrowing casts in DML assignment coercion</a>. */
@Test void testUpdateVarcharNarrowingKeepsSourceType() {
final String sql = "update t1 set t1_varchar20 = "
+ "CAST('AVeryLongLongStringValue' AS VARCHAR(64))";
sql(sql).ok();
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-7607">[CALCITE-7607]
* Avoid implicit narrowing casts in DML assignment coercion</a>. */
@Test void testMergeVarcharNarrowingKeepsSourceType() {
final String sql = "merge into t1\n"
+ "using t2 on t1.t1_smallint = t2.t2_smallint\n"
+ "when matched then update set t1_varchar20 = "
+ "CAST('AVeryLongLongStringValue' AS VARCHAR(64))";
sql(sql).ok();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16240,14 +16240,14 @@ select empno, cast(job as varchar(128)) from sales.empnullables]]>
<Resource name="planBefore">
<![CDATA[
LogicalTableModify(table=[[CATALOG, SALES, DEPT]], operation=[INSERT], flattened=[false])
LogicalProject(DEPTNO=[$0], NAME=[$2])
LogicalProject(DEPTNO=[$0], NAME=[CAST($2):VARCHAR(128)])
LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
]]>
</Resource>
<Resource name="planAfter">
<![CDATA[
LogicalTableModify(table=[[CATALOG, SALES, DEPT]], operation=[INSERT], flattened=[false])
LogicalCalc(expr#0..8=[{inputs}], expr#9=[CAST($t2):VARCHAR(10) NOT NULL], DEPTNO=[$t0], NAME=[$t9])
LogicalCalc(expr#0..8=[{inputs}], expr#9=[CAST($t2):VARCHAR(128)], DEPTNO=[$t0], NAME=[$t9])
LogicalTableScan(table=[[CATALOG, SALES, EMPNULLABLES]])
]]>
</Resource>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5517,7 +5517,7 @@ values(t.empno, t.ename, 10, t.sal * .15)]]>
<Resource name="plan">
<![CDATA[
LogicalTableModify(table=[[CATALOG, SALES, EMPNULLABLES]], operation=[MERGE], updateColumnList=[[ENAME, DEPTNO, SAL]], flattened=[true])
LogicalProject(EMPNO=[$0], ENAME=[$1], $f2=[null:VARCHAR(10)], $f3=[null:INTEGER], $f4=[null:TIMESTAMP(0)], $f5=[CAST(*($5, 0.15:DECIMAL(2, 2))):INTEGER NOT NULL], $f6=[null:INTEGER], $f7=[10], $f8=[null:BOOLEAN], EMPNO0=[$9], ENAME0=[$10], JOB0=[$11], MGR0=[$12], HIREDATE0=[$13], SAL0=[$14], COMM0=[$15], DEPTNO0=[$16], SLACKER0=[$17], ENAME1=[$1], DEPTNO=[$7], $f20=[CAST(*($5, 0.1:DECIMAL(1, 1))):INTEGER NOT NULL])
LogicalProject(EMPNO=[$0], ENAME=[$1], $f2=[null:VARCHAR(10)], $f3=[null:INTEGER], $f4=[null:TIMESTAMP(0)], $f5=[*($5, 0.15:DECIMAL(2, 2))], $f6=[null:INTEGER], $f7=[10], $f8=[null:BOOLEAN], EMPNO0=[$9], ENAME0=[$10], JOB0=[$11], MGR0=[$12], HIREDATE0=[$13], SAL0=[$14], COMM0=[$15], DEPTNO0=[$16], SLACKER0=[$17], ENAME1=[$1], DEPTNO=[$7], $f20=[*($5, 0.1:DECIMAL(1, 1))])
LogicalJoin(condition=[=($9, $0)], joinType=[left])
LogicalProject(EMPNO=[$0], ENAME=[$1], JOB=[$2], MGR=[$3], HIREDATE=[$4], SAL=[$5], COMM=[$6], DEPTNO=[$7], SLACKER=[$8])
LogicalFilter(condition=[IS NULL($7)])
Expand Down Expand Up @@ -7166,7 +7166,7 @@ LogicalTableModify(table=[[CATALOG, SALES, DEPT]], operation=[INSERT], flattened
LogicalAggregate(group=[{0, 1}])
LogicalProject(EMPNO=[$0], ENAME=[$1])
LogicalFilter(condition=[$2])
LogicalProject(EMPNO=[$0], ENAME=[CAST($1):VARCHAR(10) NOT NULL], QualifyExpression=[=(RANK() OVER (PARTITION BY $1 ORDER BY $8 DESC), 1)])
LogicalProject(EMPNO=[$0], ENAME=[$1], QualifyExpression=[=(RANK() OVER (PARTITION BY $1 ORDER BY $8 DESC), 1)])
LogicalFilter(condition=[>($7, 5)])
LogicalTableScan(table=[[CATALOG, SALES, EMP]])
]]>
Expand Down
Loading
Loading