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 e664ae0702 [CALCITE-3522] SqlValidator.validateLiteral rejects
literals with a DECIMAL type that require more than 64 bits
e664ae0702 is described below
commit e664ae0702e86d276ada2174c3a19a60feeea75e
Author: Mihai Budiu <[email protected]>
AuthorDate: Thu Aug 1 10:50:29 2024 -0700
[CALCITE-3522] SqlValidator.validateLiteral rejects literals with a DECIMAL
type that require more than 64 bits
Signed-off-by: Mihai Budiu <[email protected]>
---
.../org/apache/calcite/sql/SqlNumericLiteral.java | 3 +-
.../calcite/sql/validate/SqlValidatorImpl.java | 30 +++---
.../src/main/java/org/apache/calcite/util/Bug.java | 5 -
.../org/apache/calcite/test/SqlOperatorTest.java | 106 +++++++++++++--------
4 files changed, 81 insertions(+), 63 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
b/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
index 7b9f6ac31a..6867d199f0 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlNumericLiteral.java
@@ -103,7 +103,7 @@ public class SqlNumericLiteral extends SqlLiteral {
BigDecimal bd = getValueNonNull();
SqlTypeName result;
// Will throw if the number cannot be represented as a long.
- long l = bd.longValue();
+ long l = bd.longValueExact();
if ((l >= Integer.MIN_VALUE) && (l <= Integer.MAX_VALUE)) {
result = SqlTypeName.INTEGER;
} else {
@@ -115,7 +115,6 @@ public class SqlNumericLiteral extends SqlLiteral {
// Fallback to DECIMAL.
}
}
-
// else we have a decimal
return typeFactory.createSqlType(
SqlTypeName.DECIMAL,
diff --git
a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
index 9daadadec0..5adfea1375 100644
--- a/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
+++ b/core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java
@@ -133,7 +133,6 @@ import org.checkerframework.dataflow.qual.Pure;
import org.slf4j.Logger;
import java.math.BigDecimal;
-import java.math.BigInteger;
import java.util.AbstractList;
import java.util.ArrayDeque;
import java.util.ArrayList;
@@ -3417,20 +3416,21 @@ public class SqlValidatorImpl implements
SqlValidatorWithHints {
@Override public void validateLiteral(SqlLiteral literal) {
switch (literal.getTypeName()) {
case DECIMAL:
- // Decimal and long have the same precision (as 64-bit integers), so
- // the unscaled value of a decimal must fit into a long.
-
- // REVIEW jvs 4-Aug-2004: This should probably be calling over to
- // the available calculator implementations to see what they
- // support. For now use ESP instead.
- //
- // jhyde 2006/12/21: I think the limits should be baked into the
- // type system, not dependent on the calculator implementation.
- BigDecimal bd = literal.getValueAs(BigDecimal.class);
- BigInteger unscaled = bd.unscaledValue();
- long longValue = unscaled.longValue();
- if (!BigInteger.valueOf(longValue).equals(unscaled)) {
- // overflow
+ // Accept any decimal value that does not exceed the max
+ // precision and scale of the type system.
+ final RelDataTypeSystem typeSystem = getTypeFactory().getTypeSystem();
+ final BigDecimal bd = literal.getValueAs(BigDecimal.class);
+ final BigDecimal noTrailingZeros = bd.stripTrailingZeros();
+ // If we don't strip trailing zeros we may reject values such as
1.000....0.
+
+ final int maxPrecision = typeSystem.getMaxNumericPrecision();
+ if (noTrailingZeros.precision() > maxPrecision) {
+ throw newValidationError(literal,
+ RESOURCE.numberLiteralOutOfRange(bd.toString()));
+ }
+
+ final int maxScale = typeSystem.getMaxNumericScale();
+ if (noTrailingZeros.scale() > maxScale) {
throw newValidationError(literal,
RESOURCE.numberLiteralOutOfRange(bd.toString()));
}
diff --git a/core/src/main/java/org/apache/calcite/util/Bug.java
b/core/src/main/java/org/apache/calcite/util/Bug.java
index 3c39697816..521da4d29d 100644
--- a/core/src/main/java/org/apache/calcite/util/Bug.java
+++ b/core/src/main/java/org/apache/calcite/util/Bug.java
@@ -152,11 +152,6 @@ public abstract class Bug {
* Improve RelMdPredicates performance</a> is fixed. */
public static final boolean CALCITE_2401_FIXED = false;
- /** Whether
- * <a
href="https://issues.apache.org/jira/browse/CALCITE-2539";>[CALCITE-2539]
- * Several test case not passed in CalciteSqlOperatorTest.java</a> is fixed.
*/
- public static final boolean CALCITE_2539_FIXED = false;
-
/** Whether
* <a
href="https://issues.apache.org/jira/browse/CALCITE-2869";>[CALCITE-2869]
* JSON data type support</a> is fixed. */
diff --git a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
index 7e880e94c6..72b3559336 100644
--- a/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
+++ b/testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
@@ -430,6 +430,43 @@ public class SqlOperatorTest {
false);
}
+ /** Test case for <a
href="https://issues.apache.org/jira/browse/CALCITE-3522";>
+ * Sql validator limits decimal literals to 64 bits</a>. */
+ @Test void testLargeLiterals() {
+ // Some of these literals were too large to be accepted previously, but
+ // now are legal as decimal literals.
+ SqlOperatorFixture f = fixture();
+ f.checkCastFails("9223372036854775808", "INTEGER",
+ OUT_OF_RANGE_MESSAGE, true, SqlOperatorFixture.CastType.CAST);
+ f.checkCastFails("9223372036854775808.1", "INTEGER",
+ "Numeric literal.*out of range", false,
SqlOperatorFixture.CastType.CAST);
+ f.checkCastFails("223372036854775808", "INTEGER",
+ OUT_OF_RANGE_MESSAGE, true, SqlOperatorFixture.CastType.CAST);
+ f.checkCastFails("9223372036854775808", "BIGINT",
+ "Overflow", true, SqlOperatorFixture.CastType.CAST);
+ f.checkCastFails("'" + Numeric.TINYINT.maxOverflowNumericString + "'",
+ "TINYINT", OUT_OF_RANGE_MESSAGE, true,
SqlOperatorFixture.CastType.CAST);
+ String largePrecision = "1234567891011.0";
+ String largeScale = "1.01234567891011";
+ f.checkScalarExact(largePrecision, "DECIMAL(14, 1) NOT NULL",
largePrecision);
+ f.checkScalarExact(largeScale, "DECIMAL(15, 14) NOT NULL", largeScale);
+
+ // Check that the type system can reject large decimal literals
+ SqlOperatorFixture f0 = f.withFactory(tf ->
+ tf.withTypeSystem(typeSystem ->
+ new DelegatingTypeSystem(typeSystem) {
+ @Override public int getMaxNumericPrecision() {
+ return 10;
+ }
+
+ @Override public int getMaxNumericScale() {
+ return 10;
+ }
+ }));
+ f0.checkFails("^" + largePrecision + "^", OUT_OF_RANGE_MESSAGE, false);
+ f0.checkFails("^" + largeScale + "^", OUT_OF_RANGE_MESSAGE, false);
+ }
+
@Test void testNotBetween() {
final SqlOperatorFixture f = fixture();
f.setFor(SqlStdOperatorTable.NOT_BETWEEN, VM_EXPAND);
@@ -626,13 +663,11 @@ public class SqlOperatorTest {
// Overflow test
if (numeric == Numeric.BIGINT) {
- // Calcite cannot even represent a literal so large, so
- // for this query even the safe casts fail at compile-time
- // (runtime == false).
- f.checkFails("cast(^" + numeric.maxOverflowNumericString + "^ as
BIGINT)",
- LITERAL_OUT_OF_RANGE_MESSAGE, false);
- f.checkFails("cast(^" + numeric.minOverflowNumericString + "^ as
BIGINT)",
- LITERAL_OUT_OF_RANGE_MESSAGE, false);
+ // Overflow for casting decimals produces a different error
+ f.checkCastFails(numeric.maxOverflowNumericString,
+ type, "Overflow", true, castType);
+ f.checkCastFails(numeric.minOverflowNumericString,
+ type, "Overflow", true, castType);
} else {
if (numeric != Numeric.DECIMAL5_2) {
// This condition is for bug [CALCITE-6078], not yet fixed
@@ -1576,11 +1611,6 @@ public class SqlOperatorTest {
final SqlOperatorFixture f = fixture();
f.setFor(SqlLibraryOperators.MSSQL_CONVERT, VmName.EXPAND);
// ensure 'style' argument is ignored
- // 3rd argument 'style' is a literal. However,
- // AbstractSqlTester converts values to a single value in a column.
- // see AbstractSqlTester.buildQuery2
- // But CONVERT 'style' is supposed to be a literal.
- // So for now, they are put in a @Disabled test
f.checkScalar("convert(INTEGER, 45.4, 999)", "45", "INTEGER NOT NULL");
f.checkScalar("convert(DATE, '2000-01-01', 999)", "2000-01-01", "DATE NOT
NULL");
// including 'NULL' style argument
@@ -4265,15 +4295,12 @@ public class SqlOperatorTest {
+ " \\^";
f.checkFails("'yd3223' similar to '[:LOWER:]{2}[:DIGIT:]{,5}'",
expectedError, true);
- if (Bug.CALCITE_2539_FIXED) {
- f.checkFails("'cd' similar to '[(a-e)]d' ",
- "Invalid regular expression: \\[\\(a-e\\)\\]d at 1",
- true);
-
- f.checkFails("'yd' similar to '[(a-e)]d' ",
- "Invalid regular expression: \\[\\(a-e\\)\\]d at 1",
- true);
- }
+ f.checkFails("'cd' similar to '[(a-e)]d' ",
+ ".*Invalid regular expression '\\[\\(a-e\\)\\]d', index 1",
+ true);
+ f.checkFails("'yd' similar to '[(a-e)]d' ",
+ ".*Invalid regular expression '\\[\\(a-e\\)\\]d', index 1",
+ true);
// all the following tests wrong results due to missing functionality
// or defect (FRG-375, 377).
@@ -15547,36 +15574,33 @@ public class SqlOperatorTest {
final List<RelDataType> types =
SqlTests.getTypes(f.getFactory().getTypeFactory());
for (RelDataType type : types) {
+ SqlTypeName sqlTypeName = type.getSqlTypeName();
for (Object o : getValues((BasicSqlType) type, false)) {
SqlLiteral literal =
- type.getSqlTypeName().createLiteral(o, SqlParserPos.ZERO);
+ sqlTypeName.createLiteral(o, SqlParserPos.ZERO);
SqlString literalString =
literal.toSqlString(AnsiSqlDialect.DEFAULT);
- if ((type.getSqlTypeName() == SqlTypeName.BIGINT)
- || ((type.getSqlTypeName() == SqlTypeName.DECIMAL)
- && (type.getPrecision() == 19))) {
- // Values which are too large to be literals fail at
- // validate time.
- f.checkFails("CAST(^" + literalString + "^ AS " + type + ")",
- "Numeric literal '.*' out of range", false);
- } else if ((type.getSqlTypeName() == SqlTypeName.CHAR)
- || (type.getSqlTypeName() == SqlTypeName.VARCHAR)
- || (type.getSqlTypeName() == SqlTypeName.BINARY)
- || (type.getSqlTypeName() == SqlTypeName.VARBINARY)) {
+ if (sqlTypeName == SqlTypeName.DECIMAL) {
+ // Casting to decimal does not fail
+ } else if (sqlTypeName == SqlTypeName.BIGINT) {
+ // This is in fact a cast of a decimal literal to BIGINT,
+ // so the error is different.
+ f.checkFails("CAST(" + literalString + " AS " + type + ")",
+ "Overflow",
+ true);
+ } else if ((sqlTypeName == SqlTypeName.CHAR)
+ || (sqlTypeName == SqlTypeName.VARCHAR)
+ || (sqlTypeName == SqlTypeName.BINARY)
+ || (sqlTypeName == SqlTypeName.VARBINARY)) {
// Casting overlarge string/binary values do not fail -
// they are truncated. See testCastTruncates().
} else {
// Value outside legal bound should fail at runtime (not
// validate time).
- //
- // NOTE: Because Java and Fennel calcs give
- // different errors, the pattern hedges its bets.
- if (Bug.CALCITE_2539_FIXED) {
- f.checkFails("CAST(" + literalString + " AS " + type + ")",
- "(?s).*(Overflow during calculation or cast\\.|Code=22003).*",
- true);
- }
+ f.checkFails("CAST(" + literalString + " AS " + type + ")",
+ OUT_OF_RANGE_MESSAGE,
+ true);
}
}
}
