asolimando commented on code in PR #4557:
URL: https://github.com/apache/calcite/pull/4557#discussion_r2381206330
##########
core/src/test/java/org/apache/calcite/rex/RexLosslessCastTest.java:
##########
@@ -132,22 +137,305 @@ class RexLosslessCastTest extends RexProgramTestBase {
varcharType, rexBuilder.makeInputRef(intType, 0))), is(true));
}
+ @Test void testLosslessCastIntegerToApproximate() {
+ final RelDataType tinyIntType =
typeFactory.createSqlType(SqlTypeName.TINYINT);
+ final RelDataType smallIntType =
typeFactory.createSqlType(SqlTypeName.SMALLINT);
+ final RelDataType intType = typeFactory.createSqlType(SqlTypeName.INTEGER);
+ final RelDataType bigIntType =
typeFactory.createSqlType(SqlTypeName.BIGINT);
+ final RelDataType doubleType =
typeFactory.createSqlType(SqlTypeName.DOUBLE);
+ final RelDataType realType = typeFactory.createSqlType(SqlTypeName.REAL);
+
+ // Positive: tiny/small/int -> double
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(doubleType,
+ rexBuilder.makeInputRef(tinyIntType, 0))), is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(doubleType,
+ rexBuilder.makeInputRef(smallIntType, 0))), is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(doubleType,
+ rexBuilder.makeInputRef(intType, 0))), is(true));
+
+ // Negative: bigint -> double is not guaranteed to be exact
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(doubleType,
+ rexBuilder.makeInputRef(bigIntType, 0))), is(false));
+
+ // Positive: tiny/small -> real
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(realType,
+ rexBuilder.makeInputRef(tinyIntType, 0))), is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(realType,
+ rexBuilder.makeInputRef(smallIntType, 0))), is(true));
+
+ // Negative: int/bigint -> real not guaranteed exact
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(realType,
+ rexBuilder.makeInputRef(intType, 0))), is(false));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(realType,
+ rexBuilder.makeInputRef(bigIntType, 0))), is(false));
+ }
+
+ @Test void testLosslessCastIntegerToDecimal() {
+ final RelDataType tinyIntType =
typeFactory.createSqlType(SqlTypeName.TINYINT);
+ final RelDataType smallIntType =
typeFactory.createSqlType(SqlTypeName.SMALLINT);
+ final RelDataType intType = typeFactory.createSqlType(SqlTypeName.INTEGER);
+ final RelDataType bigIntType =
typeFactory.createSqlType(SqlTypeName.BIGINT);
+
+ final RelDataType decPrec3Scale0 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 3, 0);
+ final RelDataType decPrec5Scale0 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 5, 0);
+ final RelDataType decPrec9Scale0 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 9, 0);
+ final RelDataType decPrec10Scale0 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 10, 0);
+ final RelDataType decPrec18Scale0 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 18, 0);
+ final RelDataType decPrec19Scale0 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 19, 0);
+ final RelDataType decPrec4Scale1 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 4, 1);
+ final RelDataType decPrec5Scale1 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 5, 1);
+ final RelDataType decPrec12Scale2 =
typeFactory.createSqlType(SqlTypeName.DECIMAL, 12, 2);
+
+ // Positive: integer digits "precision - scale" is large enough
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec3Scale0,
+ rexBuilder.makeInputRef(tinyIntType, 0))), is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec5Scale0,
+ rexBuilder.makeInputRef(smallIntType, 0))), is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec10Scale0,
+ rexBuilder.makeInputRef(intType, 0))), is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec19Scale0,
+ rexBuilder.makeInputRef(bigIntType, 0))), is(true));
+
+ // Negative: not enough integer digits
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec9Scale0,
+ rexBuilder.makeInputRef(intType, 0))), is(false));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec18Scale0,
+ rexBuilder.makeInputRef(bigIntType, 0))), is(false));
+
+ // Non-zero scale is still lossless if "precision - scale" covers integer
digits
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec12Scale2,
+ rexBuilder.makeInputRef(intType, 0))), is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec4Scale1,
+ rexBuilder.makeInputRef(smallIntType, 0))), is(false));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(decPrec5Scale1,
+ rexBuilder.makeInputRef(tinyIntType, 0))), is(true));
+ }
+
+ @Test void testLosslessCastUnsignedAndSignedIntegers() {
+ final RelDataType uTinyIntType =
typeFactory.createSqlType(SqlTypeName.UTINYINT);
+ final RelDataType uIntType =
typeFactory.createSqlType(SqlTypeName.UINTEGER);
+ final RelDataType uBigIntType =
typeFactory.createSqlType(SqlTypeName.UBIGINT);
+
+ final RelDataType tinyIntType =
typeFactory.createSqlType(SqlTypeName.TINYINT);
+ final RelDataType smallIntType =
typeFactory.createSqlType(SqlTypeName.SMALLINT);
+ final RelDataType intType = typeFactory.createSqlType(SqlTypeName.INTEGER);
+ final RelDataType bigIntType =
typeFactory.createSqlType(SqlTypeName.BIGINT);
+
+ // unsigned -> signed of same width should be considered lossy
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(tinyIntType,
rexBuilder.makeInputRef(uTinyIntType, 0))),
+ is(false));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(intType, rexBuilder.makeInputRef(uIntType,
0))),
+ is(false));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(bigIntType,
rexBuilder.makeInputRef(uBigIntType, 0))),
+ is(false));
+
+ // unsigned -> wider signed is safe
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(smallIntType,
rexBuilder.makeInputRef(uTinyIntType, 0))),
+ is(true));
+ assertThat(
+ RexUtil.isLosslessCast(
+ rexBuilder.makeCast(bigIntType, rexBuilder.makeInputRef(uIntType,
0))),
+ is(true));
+
+ // signed -> unsigned stays conservative for now
Review Comment:
Since this is a purely static analysis I agree with you, I did consider
looking into optionally use metadata to support some more cases but I gave up
on that idea, I will do a pass to clean up comments and remove this wording
where not appropriate
--
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]
