This is an automated email from the ASF dual-hosted git repository.
mbudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new c228804e25 [CALCITE-6389] RexBuilder.removeCastFromLiteral does not
preserve semantics for some types of literal
c228804e25 is described below
commit c228804e258f72cd5df0f1f263dcc4f86410d3ff
Author: Mihai Budiu <mbu...@feldera.com>
AuthorDate: Tue May 7 14:29:40 2024 -0700
[CALCITE-6389] RexBuilder.removeCastFromLiteral does not preserve semantics
for some types of literal
Signed-off-by: Mihai Budiu <mbu...@feldera.com>
---
.../adapter/enumerable/RexToLixTranslator.java | 31 ++++++++-----
.../java/org/apache/calcite/rex/RexBuilder.java | 37 +++++++++------
.../org/apache/calcite/sql/type/SqlTypeUtil.java | 30 ++++++++++++
.../org/apache/calcite/rex/RexBuilderTest.java | 53 ++++++++++++++++++++++
4 files changed, 125 insertions(+), 26 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
index e49788c594..7886c5353d 100644
---
a/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
+++
b/core/src/main/java/org/apache/calcite/adapter/enumerable/RexToLixTranslator.java
@@ -268,22 +268,29 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
/**
* Used for safe operators that return null if an exception is thrown.
*/
- private static Expression expressionHandlingSafe(Expression body, boolean
safe) {
- return safe ? safeExpression(body) : body;
+ private Expression expressionHandlingSafe(
+ Expression body, boolean safe, RelDataType targetType) {
+ return safe ? safeExpression(body, targetType) : body;
}
- private static Expression safeExpression(Expression body) {
+ private Expression safeExpression(Expression body, RelDataType targetType) {
final ParameterExpression e_ =
Expressions.parameter(Exception.class, new
BlockBuilder().newName("e"));
- return Expressions.call(
- Expressions.lambda(
- Expressions.block(
- Expressions.tryCatch(
- Expressions.return_(null, body),
- Expressions.catch_(e_,
- Expressions.return_(null, constant(null)))))),
- BuiltInMethod.FUNCTION0_APPLY.method);
+ // The type received for the targetType is never nullable.
+ // But safe casts may return null
+ RelDataType nullableTargetType =
typeFactory.createTypeWithNullability(targetType, true);
+ Expression result =
+ Expressions.call(
+ Expressions.lambda(
+ Expressions.block(
+ Expressions.tryCatch(
+ Expressions.return_(null, body),
+ Expressions.catch_(e_,
+ Expressions.return_(null, constant(null)))))),
+ BuiltInMethod.FUNCTION0_APPLY.method);
+ // FUNCTION0 always returns Object, so we need a cast to the target type
+ return EnumUtils.convert(result,
typeFactory.getJavaClass(nullableTargetType));
}
Expression translateCast(
@@ -294,7 +301,7 @@ public class RexToLixTranslator implements
RexVisitor<RexToLixTranslator.Result>
ConstantExpression format) {
Expression convert = getConvertExpression(sourceType, targetType, operand,
format);
Expression convert2 = checkExpressionPadTruncate(convert, sourceType,
targetType);
- Expression convert3 = expressionHandlingSafe(convert2, safe);
+ Expression convert3 = expressionHandlingSafe(convert2, safe, targetType);
return scaleValue(sourceType, targetType, convert3);
}
diff --git a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
index 341759f57b..7a7f247749 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexBuilder.java
@@ -694,7 +694,9 @@ public class RexBuilder {
return false;
}
if (toType.getSqlTypeName() != fromTypeName
- && SqlTypeFamily.DATETIME.getTypeNames().contains(fromTypeName)) {
+ && (SqlTypeFamily.DATETIME.getTypeNames().contains(fromTypeName)
+ ||
SqlTypeFamily.INTERVAL_DAY_TIME.getTypeNames().contains(fromTypeName)
+ ||
SqlTypeFamily.INTERVAL_YEAR_MONTH.getTypeNames().contains(fromTypeName))) {
return false;
}
if (value instanceof NlsString) {
@@ -720,9 +722,10 @@ public class RexBuilder {
}
}
- if (toType.getSqlTypeName() == SqlTypeName.DECIMAL) {
+ if (toType.getSqlTypeName() == SqlTypeName.DECIMAL
+ && fromTypeName.getFamily() == SqlTypeFamily.NUMERIC) {
final BigDecimal decimalValue = (BigDecimal) value;
- return SqlTypeUtil.isValidDecimalValue(decimalValue, toType);
+ return SqlTypeUtil.canBeRepresentedExactly(decimalValue, toType);
}
if (SqlTypeName.INT_TYPES.contains(sqlType)) {
@@ -731,17 +734,23 @@ public class RexBuilder {
if (s != 0) {
return false;
}
- long l = decimalValue.longValue();
- switch (sqlType) {
- case TINYINT:
- return l >= Byte.MIN_VALUE && l <= Byte.MAX_VALUE;
- case SMALLINT:
- return l >= Short.MIN_VALUE && l <= Short.MAX_VALUE;
- case INTEGER:
- return l >= Integer.MIN_VALUE && l <= Integer.MAX_VALUE;
- case BIGINT:
- default:
- return true;
+ try {
+ // will trigger ArithmeticException when the value
+ // cannot be represented exactly as a long
+ long l = decimalValue.longValueExact();
+ switch (sqlType) {
+ case TINYINT:
+ return l >= Byte.MIN_VALUE && l <= Byte.MAX_VALUE;
+ case SMALLINT:
+ return l >= Short.MIN_VALUE && l <= Short.MAX_VALUE;
+ case INTEGER:
+ return l >= Integer.MIN_VALUE && l <= Integer.MAX_VALUE;
+ case BIGINT:
+ default:
+ return true;
+ }
+ } catch (ArithmeticException ex) {
+ return false;
}
}
diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
index 3e91b1b781..7f0bc64516 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
@@ -51,6 +51,7 @@ import
org.checkerframework.checker.nullness.qual.EnsuresNonNullIf;
import org.checkerframework.checker.nullness.qual.Nullable;
import java.math.BigDecimal;
+import java.math.RoundingMode;
import java.nio.charset.Charset;
import java.util.AbstractList;
import java.util.ArrayList;
@@ -1822,6 +1823,35 @@ public abstract class SqlTypeUtil {
type.getFieldList().subList(fieldsCnt - numToKeep, fieldsCnt));
}
+ /**
+ * Returns whether the decimal value can be represented without information
loss
+ * using the specified type.
+ * For example, 1111.11
+ * - cannot be represented exactly using DECIMAL(3, 1) since it overflows.
+ * - cannot be represented exactly using DECIMAL(6, 3) since it overflows.
+ * - cannot be represented exactly using DECIMAL(6, 1) since it requires
rounding.
+ * - can be represented exactly using DECIMAL(6, 2)
+ *
+ * @param value A decimal value
+ * @param toType A DECIMAL type.
+ * @return whether the value is valid for the type
+ */
+ public static boolean canBeRepresentedExactly(@Nullable BigDecimal value,
RelDataType toType) {
+ assert toType.getSqlTypeName() == SqlTypeName.DECIMAL;
+ if (value == null) {
+ return true;
+ }
+ value = value.stripTrailingZeros();
+ if (value.scale() < 0) {
+ // Negative scale, convert to 0 scale.
+ // Rounding mode is irrelevant, since value is integer
+ value = value.setScale(0, RoundingMode.DOWN);
+ }
+ final int intDigits = value.precision() - value.scale();
+ final int maxIntDigits = toType.getPrecision() - toType.getScale();
+ return (intDigits <= maxIntDigits) && (value.scale() <= toType.getScale());
+ }
+
/**
* Returns whether the decimal value is valid for the type. For example,
1111.11 is not
* valid for DECIMAL(3, 1) since it overflows.
diff --git a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
index 9db5f8ed07..535232a105 100644
--- a/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
+++ b/core/src/test/java/org/apache/calcite/rex/RexBuilderTest.java
@@ -69,6 +69,7 @@ import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasToString;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -213,6 +214,58 @@ class RexBuilderTest {
hasToString("1969-07-21 02:56:15.102"));
}
+ /** Test cases for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6389";>[CALCITE-6389]
+ * RexBuilder.removeCastFromLiteral does not preserve semantics for some
types of literal</a>. */
+ @Test void testRemoveCast() {
+ final RelDataTypeFactory typeFactory = new
SqlTypeFactoryImpl(RelDataTypeSystem.DEFAULT);
+ RexBuilder builder = new RexBuilder(typeFactory);
+
+ // Can remove cast of an integer to an integer
+ BigDecimal value = new BigDecimal(10);
+ RelDataType toType =
builder.typeFactory.createSqlType(SqlTypeName.INTEGER);
+ assertTrue(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.INTEGER));
+
+ // Can remove cast from integer to decimal
+ toType = builder.typeFactory.createSqlType(SqlTypeName.DECIMAL);
+ assertTrue(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.INTEGER));
+
+ // 250 is too large for a TINYINT
+ value = new BigDecimal(250);
+ toType = builder.typeFactory.createSqlType(SqlTypeName.TINYINT);
+ assertFalse(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.INTEGER));
+
+ // 50 isn't too large for a TINYINT
+ value = new BigDecimal(50);
+ toType = builder.typeFactory.createSqlType(SqlTypeName.TINYINT);
+ assertTrue(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.INTEGER));
+
+ // 120.25 cannot be represented with precision 2 and scale 2 without loss
+ value = new BigDecimal("120.25");
+ toType = builder.typeFactory.createSqlType(SqlTypeName.DECIMAL, 2, 2);
+ assertFalse(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.DECIMAL));
+
+ // 120.25 cannot be represented with precision 5 and scale 1 without
rounding
+ value = new BigDecimal("120.25");
+ toType = builder.typeFactory.createSqlType(SqlTypeName.DECIMAL, 5, 1);
+ assertFalse(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.DECIMAL));
+
+ // longmax + 1 cannot be represented as a long
+ value = new BigDecimal(Long.MAX_VALUE).add(BigDecimal.ONE);
+ toType = builder.typeFactory.createSqlType(SqlTypeName.BIGINT);
+ assertFalse(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.DECIMAL));
+
+ // Cast to decimal of an INTERVAL '5' seconds cannot be removed
+ value = new BigDecimal("5");
+ toType = builder.typeFactory.createSqlType(SqlTypeName.DECIMAL, 5, 1);
+ assertFalse(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.INTERVAL_SECOND));
+
+ // Cast to decimal of an INTERVAL '5' minutes cannot be removed
+ value = new BigDecimal("5");
+ toType = builder.typeFactory.createSqlType(SqlTypeName.DECIMAL, 5, 1);
+ assertFalse(builder.canRemoveCastFromLiteral(toType, value,
SqlTypeName.INTERVAL_MINUTE));
+ }
+
@Test void testTimestampString() {
final TimestampString ts = new TimestampString(1969, 7, 21, 2, 56, 15);
assertThat(ts, hasToString("1969-07-21 02:56:15"));
