diff --git a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java index 0f5174a2196..c8e62cb7453 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexSimplify.java +++ b/core/src/main/java/org/apache/calcite/rex/RexSimplify.java @@ -2692,44 +2692,29 @@ && sameTypeOrNarrowsNullability(e.getType(), operand.getType()) } if (RexUtil.isLosslessCast(operand)) { // x :: y below means cast(x as y) (which is PostgreSQL-specific cast by the way) - // A) Remove lossless casts: + // A) Both casts can be removed: // A.1) intExpr :: bigint :: int => intExpr // A.2) char2Expr :: char(5) :: char(2) => char2Expr - // B) There are cases when we can't remove two casts, but we could probably remove inner one - // B.1) char2expression :: char(4) :: char(5) -> char2expression :: char(5) - // B.2) char2expression :: char(10) :: char(5) -> char2expression :: char(5) - // B.3) char2expression :: varchar(10) :: char(5) -> char2expression :: char(5) - // B.4) char6expression :: varchar(10) :: char(5) -> char6expression :: char(5) - // C) Simplification is not possible: - // C.1) char6expression :: char(3) :: char(5) -> must not be changed + // B) Simplification is not possible: + // B.1) char6expression :: char(3) :: char(5) => must not be changed // the input is truncated to 3 chars, so we can't use char6expression :: char(5) - // C.2) varchar2Expr :: char(5) :: varchar(2) -> must not be changed - // the input have to be padded with spaces (up to 2 chars) - // C.3) char2expression :: char(4) :: varchar(5) -> must not be changed + // B.2) varchar2Expr :: char(5) :: varchar(2) => must not be changed + // the input has to be padded with spaces (up to 2 chars) + // B.3) char2expression :: char(4) :: varchar(5) => must not be changed // would not have the padding - // The approach seems to be: - // 1) Ensure inner cast is lossless (see if above) - // 2) If operand of the inner cast has the same type as the outer cast, - // remove two casts except C.2 or C.3-like pattern (== inner cast is CHAR) - // 3) If outer cast is lossless, remove inner cast (B-like cases) - - // Here we try to remove two casts in one go (A-like cases) RexNode intExpr = ((RexCall) operand).operands.get(0); - // intExpr == CHAR detects A.1 - // operand != CHAR detects C.2 if ((intExpr.getType().getSqlTypeName() == SqlTypeName.CHAR || operand.getType().getSqlTypeName() != SqlTypeName.CHAR) && sameTypeOrNarrowsNullability(e.getType(), intExpr.getType())) { + // Remove both casts (A-like cases): the outer type is the source type, + // so the two casts are a round trip, except through a CHAR type from a + // non-CHAR source, as in int :: char(11) :: int: not every runtime + // parses the padded text back return intExpr; } - // Here we try to remove inner cast (B-like cases) if (RexUtil.isLosslessCast(intExpr.getType(), operand.getType()) - && (e.getType().getSqlTypeName() == operand.getType().getSqlTypeName() - || e.getType().getSqlTypeName() == SqlTypeName.CHAR - || operand.getType().getSqlTypeName() != SqlTypeName.CHAR) - && SqlTypeCoercionRule.instance() - .canApplyFrom(intExpr.getType().getSqlTypeName(), e.getType().getSqlTypeName())) { + && canRemoveInnerCast(intExpr.getType(), operand.getType(), e.getType())) { return rexBuilder.makeCast(e.getParserPosition(), e.getType(), intExpr); } } @@ -2737,7 +2722,7 @@ && sameTypeOrNarrowsNullability(e.getType(), intExpr.getType())) { switch (operand.getKind()) { case LITERAL: final RexLiteral literal = (RexLiteral) operand; - final Comparable value = literal.getValueAs(Comparable.class); + final Comparable value = literal.getValueAs(Comparable.class); final SqlTypeName typeName = literal.getTypeName(); // First, try to remove the cast without changing the value. @@ -2787,6 +2772,73 @@ && sameTypeOrNarrowsNullability(e.getType(), intExpr.getType())) { } } + /** Returns whether {@code CAST(CAST(x AS inner) AS outer)} can be simplified to + * {@code CAST(x AS outer)}, where x has type {@code source} and the cast from + * {@code source} to {@code inner} is lossless. */ + private static boolean canRemoveInnerCast(RelDataType source, RelDataType inner, + RelDataType outer) { + // x :: y below means cast(x as y) + // Removing the inner cast changes the type of the operand of the outer cast + // from inner to source, but not its value. The result of the outer cast can + // still change, because a cast to a character or binary type formats its + // operand according to the operand's type, and a cast of a character operand + // parses its text. + // Cases A and B are in simplifyCast. + // C) The inner cast can be removed: + // C.1) char2expression :: char(4) :: char(5) => char2expression :: char(5) + // C.2) char2expression :: char(10) :: char(5) => char2expression :: char(5) + // C.3) char2expression :: varchar(10) :: char(5) => char2expression :: char(5) + // C.4) char6expression :: varchar(10) :: char(5) => char6expression :: char(5) + // C.5) smallintExpr :: int :: varchar => smallintExpr :: varchar + // 20 => '20' in both cases + // C.6) intExpr :: varchar :: int => intExpr :: int + // 20 => 20 in both cases + // C.7) intExpr :: varchar :: bigint => intExpr :: bigint + // 7369 => 7369 in both cases; the two casts to tinyint give an error in both cases + // D) The inner cast cannot be removed (see also B.2 and B.3 in simplifyCast): + // D.1) intExpr :: double :: varchar => must not be changed + // 20 => right: '20.0', wrong: '20' + // realExpr :: double :: varchar => must not be changed + // 266.66666 => right: '266.6666564941406', wrong: '266.66666' + // D.2) intExpr :: decimal(12, 2) :: varchar => must not be changed + // 20 => right: '20.00', wrong: '20' + // D.3) smallintExpr :: bigint :: varbinary => must not be changed + // 20 => right: X'0000000000000014', wrong: X'0014' + // D.4) intExpr :: varchar :: timestamp => must not be changed + // 7369 => right: error, wrong: 1970-01-01 00:00:07 + // D.5) tinyintExpr :: smallint :: variant => must not be changed + // a VARIANT keeps the type of its value; TYPEOF gives + // right: SMALLINT, wrong: TINYINT + if (SqlTypeUtil.isBinary(outer)) { + // D.3 + return false; + } + if (outer.getSqlTypeName() == SqlTypeName.VARIANT) { + // D.5 + return false; + } + if (SqlTypeUtil.inCharFamily(inner)) { + if (inner.getSqlTypeName() == SqlTypeName.CHAR + && outer.getSqlTypeName() != SqlTypeName.CHAR) { + // B.2 and B.3 from simplifyCast + return false; + } + if (!SqlTypeUtil.inCharFamily(source) + && !SqlTypeUtil.inCharFamily(outer) + && outer.getSqlTypeName() != source.getSqlTypeName() + && !(SqlTypeUtil.isIntType(source) && SqlTypeUtil.isNumeric(outer))) { + // D.4; C.6 and C.7 are the exceptions + return false; + } + } else if (SqlTypeUtil.inCharFamily(outer) + && !(SqlTypeUtil.isIntType(source) && SqlTypeUtil.isIntType(inner))) { + // D.1 and D.2; C.5 is the exception + return false; + } + return SqlTypeCoercionRule.instance() + .canApplyFrom(outer.getSqlTypeName(), source.getSqlTypeName()); + } + /** Tries to simplify CEIL/FLOOR function on top of CEIL/FLOOR. * *

Examples: diff --git a/core/src/main/java/org/apache/calcite/rex/RexUtil.java b/core/src/main/java/org/apache/calcite/rex/RexUtil.java index 5c51eab9cec..71f4acfef90 100644 --- a/core/src/main/java/org/apache/calcite/rex/RexUtil.java +++ b/core/src/main/java/org/apache/calcite/rex/RexUtil.java @@ -1906,12 +1906,26 @@ public static boolean isLosslessCast(RelDataType source, RelDataType target) { // NUMERIC -> CHARACTER: allow when target length accommodates sign/scale if (sourceSqlTypeName.getFamily() == SqlTypeFamily.NUMERIC && targetSqlTypeName.getFamily() == SqlTypeFamily.CHARACTER) { + final int targetPrecision = target.getPrecision(); + if (targetPrecision == PRECISION_NOT_SPECIFIED) { + return true; + } + // The precision does not bound the length of the text of an approximate + // number, or of a DECIMAL with a negative scale: + // CAST(6.666666666666667E0 AS VARCHAR(3)) is '6.6' + // CAST(CAST(20000 AS DECIMAL(2, -3)) AS VARCHAR(3)) is '200' + // The text of a DOUBLE has at most 24 characters, and that of a REAL at most 15, + // but this value depends on the formatting performed by the runtime, + // so we are NOT considering such casts as lossless currently. + final int sourceScale = source.getScale(); + if (!SqlTypeUtil.isExactNumeric(source) || sourceScale < 0) { + return false; + } int sourceLength = source.getPrecision() + 1; // include sign - if (source.getScale() != -1 && source.getScale() != 0) { - sourceLength += source.getScale() + 1; // include decimal mark + if (sourceScale != 0) { + sourceLength += sourceScale + 1; // include decimal mark } - final int targetPrecision = target.getPrecision(); - return targetPrecision == PRECISION_NOT_SPECIFIED || targetPrecision >= sourceLength; + return targetPrecision >= sourceLength; } // DECIMAL -> DECIMAL: allow when precision/scale can only expand diff --git a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java index e90b967482c..ee7f9d5803c 100644 --- a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java +++ b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java @@ -3559,7 +3559,9 @@ private SqlDialect nonOrdinalDialect() { + "CAST(employee_id AS STRING), " + "CAST(CAST(employee_id AS STRING) AS BYTES), " + "CAST(CAST(employee_id AS STRING) AS BYTES), " - + "CAST(employee_id AS TIMESTAMP), " + // CAST('7369' AS TIMESTAMP) is an error, + // but CAST(7369 AS TIMESTAMP) is 1970-01-01 00:00:07 + + "CAST(CAST(employee_id AS STRING) AS TIMESTAMP), " + "CAST(employee_id AS FLOAT64), " + "CAST(employee_id AS NUMERIC), " + "CAST(CAST(employee_id AS STRING) AS DATE), " diff --git a/core/src/test/java/org/apache/calcite/rex/RexLosslessCastTest.java b/core/src/test/java/org/apache/calcite/rex/RexLosslessCastTest.java index 86c13c74273..70684c5174b 100644 --- a/core/src/test/java/org/apache/calcite/rex/RexLosslessCastTest.java +++ b/core/src/test/java/org/apache/calcite/rex/RexLosslessCastTest.java @@ -384,6 +384,27 @@ class RexLosslessCastTest extends RexProgramTestBase { rexBuilder.makeCast(dec_19_0, rexBuilder.makeInputRef(doubleType, 0))), is(false)); } + /** Test case for + * [CALCITE-7870] + * RexSimplify removes some casts which change semantics. */ + @Test void testLosslessCastApproximateToChar() { + final RelDataType realType = typeFactory.createSqlType(SqlTypeName.REAL); + final RelDataType doubleType = typeFactory.createSqlType(SqlTypeName.DOUBLE); + final RelDataType varcharType = typeFactory.createSqlType(SqlTypeName.VARCHAR); + final RelDataType varcharType3 = typeFactory.createSqlType(SqlTypeName.VARCHAR, 3); + + // The text of an approximate number may not fit in a bounded string + assertThat( + RexUtil.isLosslessCast( + rexBuilder.makeCast(varcharType3, rexBuilder.makeInputRef(doubleType, 0))), is(false)); + assertThat( + RexUtil.isLosslessCast( + rexBuilder.makeCast(varcharType3, rexBuilder.makeInputRef(realType, 0))), is(false)); + assertThat( + RexUtil.isLosslessCast( + rexBuilder.makeCast(varcharType, rexBuilder.makeInputRef(doubleType, 0))), is(true)); + } + @Test void testLosslessCastWithCustomTypeSystem() { final RelDataType dec_10_0 = typeFactory.createSqlType(SqlTypeName.DECIMAL, 10, 0); final RelDataType decDefault = typeFactory.createSqlType(SqlTypeName.DECIMAL); @@ -428,6 +449,7 @@ class RexLosslessCastTest extends RexProgramTestBase { @Test void removeLosslessCastInt() { checkSimplifyUnchanged(cast(vInt(), tBigInt())); checkSimplifyUnchanged(cast(vInt(), tDouble())); + // The case labels below are those of RexSimplify.simplifyCast // A.1 INT -> BIGINT checkSimplify(cast(cast(vInt(), tBigInt()), tInt()), "CAST(?0.int0):INTEGER NOT NULL"); checkSimplify(cast(cast(vInt(), tDouble()), tInt()), "CAST(?0.int0):INTEGER NOT NULL"); @@ -476,35 +498,82 @@ class RexLosslessCastTest extends RexProgramTestBase { RexNode char2 = vParam("char(2)_", tChar(2)); RexNode char6 = vParam("char(6)_", tChar(6)); - RexNode varchar2 = vParam("varchar(2)_", tChar(2)); - // A.2 in RexSimplify + RexNode varchar2 = vParam("varchar(2)_", tVarchar(2)); + // The case labels below are those of RexSimplify.simplifyCast (A and B) + // and RexSimplify.canRemoveInnerCast (C) + // A.2 checkSimplify( cast(cast(char2, tChar(5)), tChar(2)), "CAST(?0.char(2)_0):CHAR(2) NOT NULL"); - // B.1 + // C.1 checkSimplify( cast(cast(char2, tChar(4)), tChar(5)), "CAST(?0.char(2)_0):CHAR(5) NOT NULL"); - // B.2 + // C.2 checkSimplify( cast(cast(char2, tChar(10)), tChar(5)), "CAST(?0.char(2)_0):CHAR(5) NOT NULL"); - // B.3 + // C.3 checkSimplify( cast(cast(char2, tVarchar(10)), tChar(5)), "CAST(?0.char(2)_0):CHAR(5) NOT NULL"); - // B.4 + // C.4 checkSimplify( cast(cast(char6, tVarchar(10)), tChar(5)), "CAST(?0.char(6)_0):CHAR(5) NOT NULL"); - // C.1 + // B.1 checkSimplifyUnchanged( cast(cast(char6, tChar(3)), tChar(5))); - // C.2 + // B.2 checkSimplifyUnchanged( cast(cast(varchar2, tChar(5)), tVarchar(2))); - // C.3 + // B.3 checkSimplifyUnchanged( cast(cast(char2, tChar(4)), tVarchar(5))); } + + /** Test case for + * [CALCITE-7870] + * RexSimplify removes some casts which change semantics. */ + @Test void keepInnerCastThatChangesResult() { + final RelDataType dec_5_2 = typeFactory.createSqlType(SqlTypeName.DECIMAL, 5, 2); + final RelDataType dec_10_4 = typeFactory.createSqlType(SqlTypeName.DECIMAL, 10, 4); + final RelDataType dec_12_2 = typeFactory.createSqlType(SqlTypeName.DECIMAL, 12, 2); + // The case labels below are those of RexSimplify.canRemoveInnerCast + // D.1 + checkSimplifyUnchanged(cast(cast(vInt(), tDouble()), tVarchar())); + checkSimplifyUnchanged(cast(cast(vParam("real_", tReal()), tDouble()), tVarchar())); + // D.2 + checkSimplifyUnchanged(cast(cast(vInt(), dec_12_2), tVarchar())); + checkSimplifyUnchanged(cast(cast(vParam("decimal(5,2)_", dec_5_2), dec_10_4), tVarchar())); + // D.3 + checkSimplifyUnchanged(cast(cast(vSmallInt(), tBigInt()), tVarbinary())); + // D.4 + checkSimplifyUnchanged(cast(cast(vInt(), tVarchar()), tBool())); + // D.5 + final RelDataType variant = typeFactory.createSqlType(SqlTypeName.VARIANT); + checkSimplifyUnchanged(cast(cast(vSmallInt(), tInt()), variant)); + checkSimplifyUnchanged(cast(cast(vParam("varchar(5)_", tVarchar(5)), tVarchar(10)), variant)); + // C.5 + checkSimplify(cast(cast(vSmallInt(), tInt()), tVarchar()), + "CAST(?0.smallint0):VARCHAR NOT NULL"); + // C.6 + checkSimplify(cast(cast(vInt(), tVarchar()), tInt()), "CAST(?0.int0):INTEGER NOT NULL"); + // C.7 + checkSimplify(cast(cast(vInt(), tVarchar()), tBigInt()), "CAST(?0.int0):BIGINT NOT NULL"); + checkSimplify(cast(cast(vInt(), tVarchar()), tSmallInt()), + "CAST(?0.int0):SMALLINT NOT NULL"); + // The text of a DOUBLE may not fit in VARCHAR(3) + final RexNode d = vParam("double_", tDouble(true)); + checkSimplifyUnchanged(cast(cast(d, tVarchar(3)), tVarchar())); + checkSimplifyUnchanged(cast(cast(d, tVarchar(3)), tDouble())); + // C-like case; BOOLEAN is castable from REAL, but REAL is not castable from BOOLEAN. + // (A cast from an exact type to BOOLEAN becomes a comparison before it is simplified.) + checkSimplify(cast(cast(vParam("real_", tReal()), tDouble()), tBool()), + "CAST(?0.real_0):BOOLEAN NOT NULL"); + // C-like case; GEOMETRY is castable from VARCHAR, but VARCHAR is not castable from GEOMETRY + final RelDataType geometryType = typeFactory.createSqlType(SqlTypeName.GEOMETRY); + checkSimplify(cast(cast(vParam("varchar(5)_", tVarchar(5)), tVarchar(10)), geometryType), + "CAST(?0.varchar(5)_0):GEOMETRY NOT NULL"); + } } diff --git a/core/src/test/resources/sql/cast.iq b/core/src/test/resources/sql/cast.iq index 9bfa71ed5f7..9ec5a918292 100644 --- a/core/src/test/resources/sql/cast.iq +++ b/core/src/test/resources/sql/cast.iq @@ -2129,4 +2129,116 @@ values (cast(0.1e0 as real) = cast(0.1 as real), !ok +# Test case for [CALCITE-7870] RexSimplify removes some casts which change +# semantics +SELECT CAST(CAST(deptno AS DOUBLE) AS VARCHAR) AS d1, + CAST(CAST(deptno AS DECIMAL(5, 2)) AS VARCHAR) AS d2, + CAST(CAST(sal AS DECIMAL(10, 4)) AS VARCHAR) AS d3, + CAST(CAST(deptno AS INTEGER) AS VARCHAR) AS d4 +FROM emp +WHERE empno = 7369; ++------+-------+----------+----+ +| D1 | D2 | D3 | D4 | ++------+-------+----------+----+ +| 20.0 | 20.00 | 800.0000 | 20 | ++------+-------+----------+----+ +(1 row) + +!ok + +SELECT CAST(CAST(deptno AS BIGINT) AS VARBINARY) AS b1, + CAST(CAST(CAST(deptno AS DOUBLE) / 3e0 AS VARCHAR(3)) AS VARCHAR) AS s1, + CAST(CAST(CAST(deptno AS DOUBLE) / 3e0 AS VARCHAR(3)) AS DOUBLE) AS s2 +FROM emp +WHERE empno = 7369; ++------------------+-----+-----+ +| B1 | S1 | S2 | ++------------------+-----+-----+ +| 0000000000000014 | 6.6 | 6.6 | ++------------------+-----+-----+ +(1 row) + +!ok + +SELECT CAST(CAST(deptno AS VARCHAR) AS BOOLEAN) AS b +FROM emp +WHERE empno = 7369; +Invalid character for cast: 20 +!error + +SELECT CAST(CAST(deptno AS VARCHAR) AS TIMESTAMP) AS t +FROM emp +WHERE empno = 7369; +Invalid DATE value, '20' +!error + +SELECT CAST(CAST(deptno AS VARCHAR) AS BIGINT) AS b1, + CAST(CAST(deptno AS VARCHAR) AS DOUBLE) AS b2 +FROM emp +WHERE empno = 7369; ++----+------+ +| B1 | B2 | ++----+------+ +| 20 | 20.0 | ++----+------+ +(1 row) + +!ok + +SELECT CAST(CAST(sal / 3 AS REAL) AS VARCHAR) AS r1, + CAST(CAST(CAST(sal / 3 AS REAL) AS DOUBLE) AS VARCHAR) AS r2 +FROM emp +WHERE empno = 7369; ++-----------+-------------------+ +| R1 | R2 | ++-----------+-------------------+ +| 266.66666 | 266.6666564941406 | ++-----------+-------------------+ +(1 row) + +!ok + +# The inner casts to DOUBLE are removed; the results do not change +SELECT CAST(CAST(CAST(deptno AS REAL) AS DOUBLE) AS BOOLEAN) AS b1, + CAST(CAST(CAST(deptno - 20 AS REAL) AS DOUBLE) AS BOOLEAN) AS b2 +FROM emp +WHERE empno = 7369; ++------+-------+ +| B1 | B2 | ++------+-------+ +| true | false | ++------+-------+ +(1 row) + +!ok + +# A VARIANT keeps the type of its value, so the inner casts must stay +SELECT TYPEOF(CAST(CAST(deptno AS SMALLINT) AS VARIANT)) AS t1, + TYPEOF(CAST(deptno AS VARIANT)) AS t2, + CAST(CAST(deptno AS SMALLINT) AS VARIANT) = CAST(deptno AS VARIANT) AS e, + TYPEOF(CAST(CAST(ename AS VARCHAR(20)) AS VARIANT)) AS t3 +FROM emp +WHERE empno = 7369; ++----------+---------+-------+---------+ +| T1 | T2 | E | T3 | ++----------+---------+-------+---------+ +| SMALLINT | TINYINT | false | VARCHAR | ++----------+---------+-------+---------+ +(1 row) + +!ok + +!use scott-negative-scale +SELECT CAST(CAST(CAST(deptno * 1000 AS DECIMAL(2, -3)) AS VARCHAR(3)) AS VARCHAR) AS n1 +FROM emp +WHERE empno = 7369; ++-----+ +| N1 | ++-----+ +| 200 | ++-----+ +(1 row) + +!ok + # End cast.iq