This is an automated email from the ASF dual-hosted git repository. zstan pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push: new 9995443666 IGNITE-19353: Sql. Incorrect type conversion for dynamic parameters - CAST operation ignores type precision. (#2220) 9995443666 is described below commit 99954436663de5f3b2fe9d86b7c27487c0463a18 Author: Max Zhuravkov <shh...@gmail.com> AuthorDate: Wed Jun 28 17:44:55 2023 +0300 IGNITE-19353: Sql. Incorrect type conversion for dynamic parameters - CAST operation ignores type precision. (#2220) --- .../internal/sql/engine/ItDataTypesTest.java | 268 +++++++++++++++++++++ .../sql/engine/ItDynamicParameterTest.java | 57 +++-- .../sql/engine/exec/exp/IgniteSqlFunctions.java | 77 ++++-- .../sql/engine/sql/IgniteSqlDecimalLiteral.java | 20 +- .../engine/exec/exp/IgniteSqlFunctionsTest.java | 73 ++++++ .../engine/sql/IgniteSqlDecimalLiteralTest.java | 30 +++ 6 files changed, 488 insertions(+), 37 deletions(-) diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java index 765628b178..adb6be9f34 100644 --- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java +++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java @@ -17,8 +17,12 @@ package org.apache.ignite.internal.sql.engine; +import static org.apache.ignite.lang.IgniteStringFormatter.format; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.params.provider.Arguments.arguments; import java.math.BigDecimal; import java.time.LocalDate; @@ -26,14 +30,30 @@ import java.time.LocalDateTime; import java.util.List; import java.util.Set; import java.util.stream.Collectors; +import java.util.stream.Stream; +import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.runtime.CalciteContextException; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.calcite.sql.type.SqlTypeUtil; +import org.apache.ignite.internal.sql.engine.util.Commons; +import org.apache.ignite.internal.sql.engine.util.QueryChecker; +import org.apache.ignite.lang.IgniteException; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; /** * Test SQL data types. */ public class ItDataTypesTest extends ClusterPerClassIntegrationTest { + + private static final String NUMERIC_OVERFLOW_ERROR = "Numeric field overflow"; + + private static final String NUMERIC_FORMAT_ERROR = "neither a decimal digit number"; + /** * Drops all created tables. */ @@ -315,6 +335,254 @@ public class ItDataTypesTest extends ClusterPerClassIntegrationTest { assertQuery("SELECT id FROM tbl WHERE val = DECIMAL '10.20'").returns(1).check(); } + + /** decimal casts - cast literal to decimal. */ + @ParameterizedTest(name = "{2}:{1} AS {3} = {4}") + @MethodSource("decimalCastFromLiterals") + public void testDecimalCastsNumericLiterals(CaseStatus status, RelDataType inputType, Object input, + RelDataType targetType, Result<BigDecimal> result) { + + Assumptions.assumeTrue(status == CaseStatus.RUN); + + String literal = asLiteral(input, inputType); + String query = format("SELECT CAST({} AS {})", literal, targetType); + + QueryChecker checker = assertQuery(query); + expectResult(checker, result); + } + + private static Stream<Arguments> decimalCastFromLiterals() { + RelDataType varcharType = varcharType(); + // ignored + RelDataType numeric = decimalType(4); + + return Stream.of( + // String + arguments(CaseStatus.RUN, varcharType, "100", decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, varcharType, "100.12", decimalType(5, 1), bigDecimalVal("100.1")), + arguments(CaseStatus.RUN, varcharType, "lame", decimalType(5, 1), error(NUMERIC_FORMAT_ERROR)), + arguments(CaseStatus.RUN, varcharType, "12345", decimalType(5, 1), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.RUN, varcharType, "1234", decimalType(5, 1), bigDecimalVal("1234.0")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, varcharType, "100.12", decimalType(1, 0), error(NUMERIC_OVERFLOW_ERROR)), + + // Numeric + arguments(CaseStatus.RUN, numeric, "100", decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, numeric, "100", decimalType(3, 0), bigDecimalVal("100")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, numeric, "100.12", decimalType(5, 1), bigDecimalVal("100.1")), + arguments(CaseStatus.SKIP, numeric, "100.12", decimalType(5, 0), bigDecimalVal("100")), + arguments(CaseStatus.SKIP, numeric, "100", decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, numeric, "100.12", decimalType(5, 2), bigDecimalVal("100.12")) + ); + } + + /** decimal casts - cast dynamic param to decimal. */ + @ParameterizedTest(name = "{2}:?{1} AS {3} = {4}") + @MethodSource("decimalCasts") + public void testDecimalCastsDynamicParams(CaseStatus ignore, RelDataType inputType, Object input, + RelDataType targetType, Result<BigDecimal> result) { + // We ignore status because every case should work for dynamic parameter. + + String query = format("SELECT CAST(? AS {})", targetType); + + QueryChecker checker = assertQuery(query).withParams(input); + expectResult(checker, result); + } + + /** decimals casts - cast numeric literal to specific type then cast the result to decimal. */ + @ParameterizedTest(name = "{1}: {2}::{1} AS {3} = {4}") + @MethodSource("decimalCasts") + public void testDecimalCastsFromNumeric(CaseStatus status, RelDataType inputType, Object input, + RelDataType targetType, Result<BigDecimal> result) { + + Assumptions.assumeTrue(status == CaseStatus.RUN); + + String literal = asLiteral(input, inputType); + String query = format("SELECT CAST({}::{} AS {})", literal, inputType, targetType); + + QueryChecker checker = assertQuery(query); + expectResult(checker, result); + } + + static String asLiteral(Object value, RelDataType type) { + if (SqlTypeUtil.isCharacter(type)) { + String str = (String) value; + return format("'{}'", str); + } else { + return String.valueOf(value); + } + } + + /** + * Indicates whether a test case should run or should be skipped. + * We need this because the set of test cases is the same for both dynamic params + * and numeric values. + * + * <p>TODO Should be removed after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + */ + enum CaseStatus { + /** Case should run. */ + RUN, + /** Case should be skipped. */ + SKIP + } + + private static Stream<Arguments> decimalCasts() { + RelDataType varcharType = varcharType(); + RelDataType tinyIntType = sqlType(SqlTypeName.TINYINT); + RelDataType smallIntType = sqlType(SqlTypeName.SMALLINT); + RelDataType integerType = sqlType(SqlTypeName.INTEGER); + RelDataType bigintType = sqlType(SqlTypeName.BIGINT); + RelDataType realType = sqlType(SqlTypeName.REAL); + RelDataType doubleType = sqlType(SqlTypeName.DOUBLE); + + return Stream.of( + // String + arguments(CaseStatus.RUN, varcharType, "100", decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, varcharType, "100", decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, varcharType, "100", decimalType(3, 0), bigDecimalVal("100")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, varcharType, "100", decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, varcharType, "100", decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + + // Tinyint + arguments(CaseStatus.SKIP, tinyIntType, (byte) 100, decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, tinyIntType, (byte) 100, decimalType(3, 0), bigDecimalVal("100")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, tinyIntType, (byte) 100, decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, tinyIntType, (byte) 100, decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + + // Smallint + arguments(CaseStatus.RUN, smallIntType, (short) 100, decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, smallIntType, (short) 100, decimalType(3, 0), bigDecimalVal("100")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, smallIntType, (short) 100, decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, smallIntType, (short) 100, decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + + // Integer + arguments(CaseStatus.RUN, integerType, 100, decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, integerType, 100, decimalType(3, 0), bigDecimalVal("100")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, integerType, 100, decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, integerType, 100, decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + + // Bigint + arguments(CaseStatus.RUN, bigintType, 100L, decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, bigintType, 100L, decimalType(3, 0), bigDecimalVal("100")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, bigintType, 100L, decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, bigintType, 100L, decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + + // Real + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, realType, 100.0f, decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.SKIP, realType, 100.0f, decimalType(3, 0), bigDecimalVal("100")), + arguments(CaseStatus.SKIP, realType, 100.0f, decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, realType, 100.0f, decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, realType, 0.1f, decimalType(1, 1), bigDecimalVal("0.1")), + arguments(CaseStatus.SKIP, realType, 0.1f, decimalType(2, 2), bigDecimalVal("0.10")), + arguments(CaseStatus.SKIP, realType, 10.12f, decimalType(2, 1), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, realType, 0.12f, decimalType(1, 2), error(NUMERIC_OVERFLOW_ERROR)), + + // Double + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, doubleType, 100.0d, decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.SKIP, doubleType, 100.0d, decimalType(3, 0), bigDecimalVal("100")), + arguments(CaseStatus.SKIP, doubleType, 100.0d, decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, doubleType, 100.0d, decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, doubleType, 0.1d, decimalType(1, 1), bigDecimalVal("0.1")), + arguments(CaseStatus.SKIP, doubleType, 0.1d, decimalType(2, 2), bigDecimalVal("0.10")), + arguments(CaseStatus.SKIP, doubleType, 10.12d, decimalType(2, 1), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, doubleType, 0.12d, decimalType(1, 2), error(NUMERIC_OVERFLOW_ERROR)), + + // Decimal + arguments(CaseStatus.RUN, decimalType(1, 1), new BigDecimal("0.1"), decimalType(1, 1), bigDecimalVal("0.1")), + arguments(CaseStatus.RUN, decimalType(3), new BigDecimal("100"), decimalType(3), bigDecimalVal("100")), + arguments(CaseStatus.RUN, decimalType(3), new BigDecimal("100"), decimalType(3, 0), bigDecimalVal("100")), + // TODO Uncomment these test cases after https://issues.apache.org/jira/browse/IGNITE-19822 is fixed. + arguments(CaseStatus.SKIP, decimalType(3), new BigDecimal("100"), decimalType(4, 1), bigDecimalVal("100.0")), + arguments(CaseStatus.SKIP, decimalType(3), new BigDecimal("100"), decimalType(2, 0), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, decimalType(1, 1), new BigDecimal("0.1"), decimalType(2, 2), bigDecimalVal("0.10")), + arguments(CaseStatus.SKIP, decimalType(4, 2), new BigDecimal("10.12"), decimalType(2, 1), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, decimalType(2, 2), new BigDecimal("0.12"), decimalType(1, 2), error(NUMERIC_OVERFLOW_ERROR)), + arguments(CaseStatus.SKIP, decimalType(1, 1), new BigDecimal("0.1"), decimalType(1, 1), bigDecimalVal("0.1")) + ); + } + + + private static RelDataType sqlType(SqlTypeName typeName) { + return Commons.typeFactory().createSqlType(typeName); + } + + private static RelDataType decimalType(int precision, int scale) { + return Commons.typeFactory().createSqlType(SqlTypeName.DECIMAL, precision, scale); + } + + private static RelDataType decimalType(int precision) { + return Commons.typeFactory().createSqlType(SqlTypeName.DECIMAL, precision, RelDataType.SCALE_NOT_SPECIFIED); + } + + private static RelDataType varcharType() { + return Commons.typeFactory().createSqlType(SqlTypeName.VARCHAR); + } + + /** + * Result contains a {@code BigDecimal} value represented by the given string. + */ + private static Result<BigDecimal> bigDecimalVal(String value) { + return new Result<>(new BigDecimal(value), null); + } + + /** Result contains an error which message contains the following substring. */ + private static <T> Result<T> error(String error) { + return new Result<>(null, error); + } + + /** + * Contains result of a test case. It can either be a value or an error. + * + * @param <T> Value type. + */ + private static class Result<T> { + final T value; + final String error; + + Result(T value, String error) { + if (error != null && value != null) { + throw new IllegalArgumentException("Both error and value have been specified"); + } + if (error == null && value == null) { + throw new IllegalArgumentException("Neither error nor value have been specified"); + } + this.value = value; + this.error = error; + } + + @Override + public String toString() { + if (value != null) { + return "VAL:" + value; + } else { + return "ERR:" + error; + } + } + } + + @Override + protected int nodes() { + return 1; + } + + private void expectResult(QueryChecker checker, Result<?> result) { + if (result.error == null) { + checker.returns(result.value).check(); + } else { + IgniteException err = assertThrows(IgniteException.class, checker::check); + assertThat(err.getMessage(), containsString(result.error)); + } + } + private LocalDate sqlDate(String str) { return LocalDate.parse(str); } diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDynamicParameterTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDynamicParameterTest.java index caf4e0f470..3058d7f581 100644 --- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDynamicParameterTest.java +++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDynamicParameterTest.java @@ -22,11 +22,17 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.params.provider.Arguments.arguments; import java.sql.Date; import java.time.LocalDate; import java.util.List; +import java.util.stream.Stream; +import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.runtime.CalciteContextException; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.ignite.internal.sql.engine.type.IgniteTypeFactory; +import org.apache.ignite.internal.sql.engine.util.Commons; import org.apache.ignite.internal.sql.engine.util.MetadataMatcher; import org.apache.ignite.internal.sql.util.SqlTestUtils; import org.apache.ignite.internal.testframework.IgniteTestUtils; @@ -36,9 +42,10 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.EnumSource.Mode; +import org.junit.jupiter.params.provider.MethodSource; /** Dynamic parameters checks. */ public class ItDynamicParameterTest extends ClusterPerClassIntegrationTest { @@ -235,21 +242,41 @@ public class ItDynamicParameterTest extends ClusterPerClassIntegrationTest { assertUnexpectedNumberOfParameters("SELECT * FROM t1 OFFSET ?", 1, 2); } - /** var char casts. */ + /** varchar casts - literals. */ @ParameterizedTest - @CsvSource({ - //input, type, result - "abcde, VARCHAR, abcde", - "abcde, VARCHAR(3), abc", - "abcde, CHAR(3), abc", - "abcde, CHAR, a", - }) - public void testVarcharCasts(String param, String type, String expected) { - String q1 = format("SELECT CAST('{}' AS {})", param, type); - assertQuery(q1).returns(expected).check(); - - String q2 = format("SELECT CAST(? AS {})", type); - assertQuery(q2).withParams(param).returns(expected).check(); + @MethodSource("varcharCasts") + public void testVarcharCastsLiterals(String value, RelDataType type, String result) { + String query = format("SELECT CAST('{}' AS {})", value, type); + assertQuery(query).returns(result).check(); + } + + /** varchar casts - dynamic params. */ + @ParameterizedTest + @MethodSource("varcharCasts") + public void testVarcharCastsDynamicParams(String value, RelDataType type, String result) { + String query = format("SELECT CAST(? AS {})", type); + assertQuery(query).withParams(value).returns(result).check(); + } + + private static Stream<Arguments> varcharCasts() { + IgniteTypeFactory typeFactory = Commons.typeFactory(); + + return Stream.of( + // varchar + arguments("abcde", typeFactory.createSqlType(SqlTypeName.VARCHAR, 3), "abc"), + arguments("abcde", typeFactory.createSqlType(SqlTypeName.VARCHAR, 5), "abcde"), + arguments("abcde", typeFactory.createSqlType(SqlTypeName.VARCHAR, 6), "abcde"), + arguments("abcde", typeFactory.createSqlType(SqlTypeName.VARCHAR), "abcde"), + + // char + arguments("abcde", typeFactory.createSqlType(SqlTypeName.CHAR), "a"), + arguments("abcde", typeFactory.createSqlType(SqlTypeName.CHAR, 3), "abc") + ); + } + + @Override + protected int nodes() { + return 1; } private static void assertUnexpectedNumberOfParameters(String query, Object... params) { diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java index a00007f172..6cb3ef60ba 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctions.java @@ -54,6 +54,7 @@ import org.apache.ignite.internal.sql.engine.type.IgniteTypeSystem; import org.apache.ignite.internal.sql.engine.util.Commons; import org.apache.ignite.internal.sql.engine.util.TypeUtils; import org.apache.ignite.lang.IgniteInternalException; +import org.apache.ignite.sql.SqlException; import org.jetbrains.annotations.Nullable; /** @@ -61,6 +62,7 @@ import org.jetbrains.annotations.Nullable; */ public class IgniteSqlFunctions { private static final DateTimeFormatter ISO_LOCAL_DATE_TIME_EX; + private static final String NUMERIC_FIELD_OVERFLOW_ERROR = "Numeric field overflow"; static { ISO_LOCAL_DATE_TIME_EX = new DateTimeFormatterBuilder() @@ -133,38 +135,36 @@ public class IgniteSqlFunctions { /** CAST(DOUBLE AS DECIMAL). */ public static BigDecimal toBigDecimal(double val, int precision, int scale) { - BigDecimal decimal = BigDecimal.valueOf(val); - return setScale(precision, scale, decimal); + return toBigDecimal((Double) val, precision, scale); } /** CAST(FLOAT AS DECIMAL). */ public static BigDecimal toBigDecimal(float val, int precision, int scale) { - BigDecimal decimal = new BigDecimal(String.valueOf(val)); - return setScale(precision, scale, decimal); + return toBigDecimal((Float) val, precision, scale); } /** CAST(java long AS DECIMAL). */ public static BigDecimal toBigDecimal(long val, int precision, int scale) { BigDecimal decimal = BigDecimal.valueOf(val); - return setScale(precision, scale, decimal); + return convertDecimal(decimal, precision, scale); } /** CAST(INT AS DECIMAL). */ public static BigDecimal toBigDecimal(int val, int precision, int scale) { BigDecimal decimal = new BigDecimal(val); - return setScale(precision, scale, decimal); + return convertDecimal(decimal, precision, scale); } /** CAST(java short AS DECIMAL). */ public static BigDecimal toBigDecimal(short val, int precision, int scale) { - BigDecimal decimal = new BigDecimal(String.valueOf(val)); - return setScale(precision, scale, decimal); + BigDecimal decimal = new BigDecimal(val); + return convertDecimal(decimal, precision, scale); } /** CAST(java byte AS DECIMAL). */ public static BigDecimal toBigDecimal(byte val, int precision, int scale) { - BigDecimal decimal = new BigDecimal(String.valueOf(val)); - return setScale(precision, scale, decimal); + BigDecimal decimal = new BigDecimal(val); + return convertDecimal(decimal, precision, scale); } /** CAST(BOOL AS DECIMAL). */ @@ -178,7 +178,7 @@ public class IgniteSqlFunctions { return null; } BigDecimal decimal = new BigDecimal(s.trim()); - return setScale(precision, scale, decimal); + return convertDecimal(decimal, precision, scale); } /** CAST(REAL AS DECIMAL). */ @@ -186,13 +186,21 @@ public class IgniteSqlFunctions { if (num == null) { return null; } - // There are some values of "long" that cannot be represented as "double". - // Not so "int". If it isn't a long, go straight to double. - BigDecimal decimal = num instanceof BigDecimal ? ((BigDecimal) num) - : num instanceof BigInteger ? new BigDecimal((BigInteger) num) - : num instanceof Long ? new BigDecimal(num.longValue()) - : BigDecimal.valueOf(num.doubleValue()); - return setScale(precision, scale, decimal); + + BigDecimal dec; + if (num instanceof Float) { + dec = new BigDecimal(num.floatValue()); + } else if (num instanceof Double) { + dec = new BigDecimal(num.doubleValue()); + } else if (num instanceof BigDecimal) { + dec = (BigDecimal) num; + } else if (num instanceof BigInteger) { + dec = new BigDecimal((BigInteger) num); + } else { + dec = new BigDecimal(num.longValue()); + } + + return convertDecimal(dec, precision, scale); } /** Cast object depending on type to DECIMAL. */ @@ -209,6 +217,39 @@ public class IgniteSqlFunctions { : toBigDecimal(o.toString(), precision, scale); } + /** + * Converts the given {@code BigDecimal} to a decimal with the given {@code precision} and {@code scale} + * according to SQL spec for CAST specification: General Rules, 8. + */ + public static BigDecimal convertDecimal(BigDecimal value, int precision, int scale) { + assert precision > 0 : "Invalid precision: " + precision; + + int defaultPrecision = IgniteTypeSystem.INSTANCE.getDefaultPrecision(SqlTypeName.DECIMAL); + if (precision == defaultPrecision) { + // This branch covers at least one known case: access to dynamic parameter from context. + // In this scenario precision = DefaultTypePrecision, because types for dynamic params + // are created by toSql(createType(param.class)). + return value; + } + + boolean nonZero = !value.unscaledValue().equals(BigInteger.ZERO); + + if (nonZero) { + if (scale > precision) { + throw new SqlException(QUERY_INVALID_ERR, NUMERIC_FIELD_OVERFLOW_ERROR); + } else { + int currentSignificantDigits = value.precision() - value.scale(); + int expectedSignificantDigits = precision - scale; + + if (currentSignificantDigits > expectedSignificantDigits) { + throw new SqlException(QUERY_INVALID_ERR, NUMERIC_FIELD_OVERFLOW_ERROR); + } + } + } + + return value.setScale(scale, RoundingMode.HALF_UP); + } + /** CAST(VARCHAR AS VARBINARY). */ public static ByteString toByteString(String s) { return s == null ? null : new ByteString(s.getBytes(Commons.typeFactory().getDefaultCharset())); diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteral.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteral.java index fd6981513a..d6569f17dd 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteral.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteral.java @@ -40,9 +40,7 @@ public final class IgniteSqlDecimalLiteral extends SqlNumericLiteral { * Constructor. */ private IgniteSqlDecimalLiteral(BigDecimal value, SqlParserPos pos) { - // We are using precision/scale from BigDecimal because calcite's values - // for those are not incorrect as they include an additional digit in precision for negative numbers. - super(value, value.precision(), value.scale(), true, pos); + super(value, getPrecision(value), value.scale(), true, pos); } /** Creates a decimal literal. */ @@ -65,8 +63,9 @@ public final class IgniteSqlDecimalLiteral extends SqlNumericLiteral { @Override public RelDataType createSqlType(RelDataTypeFactory typeFactory) { var value = getDecimalValue(); + var precision = getPrecision(value); - return typeFactory.createSqlType(SqlTypeName.DECIMAL, value.precision(), value.scale()); + return typeFactory.createSqlType(SqlTypeName.DECIMAL, precision, value.scale()); } /** {@inheritDoc} **/ @@ -98,4 +97,17 @@ public final class IgniteSqlDecimalLiteral extends SqlNumericLiteral { assert value != null : "bigDecimalValue returned null for a subclass exact numeric literal: " + this; return value; } + + private static int getPrecision(BigDecimal value) { + int scale = value.scale(); + + if (value.precision() == 1 && value.compareTo(BigDecimal.ONE) < 0) { + // For numbers less than 1 we have different precision between Java's BigDecimal and Calcite: + // 0.01 - BigDecimal precision=1, scale=2, Calcite: precision=3, scale=2 + + return 1 + scale; + } else { + return value.precision(); + } + } } diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctionsTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctionsTest.java index 0b8fb2a8dd..be033ab911 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctionsTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctionsTest.java @@ -19,10 +19,17 @@ package org.apache.ignite.internal.sql.engine.exec.exp; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import java.math.BigDecimal; +import java.util.function.Supplier; +import org.apache.calcite.sql.type.SqlTypeName; +import org.apache.ignite.internal.sql.engine.type.IgniteTypeSystem; +import org.apache.ignite.sql.SqlException; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; /** * Sql functions test. @@ -138,4 +145,70 @@ public class IgniteSqlFunctionsTest { IgniteSqlFunctions.toBigDecimal(Double.valueOf(10.101d), 10, 3) ); } + + /** Access of dynamic parameter value - parameter is not transformed. */ + @Test + public void testToBigDecimalFromObject() { + Object value = new BigDecimal("100.1"); + int defaultPrecision = IgniteTypeSystem.INSTANCE.getDefaultPrecision(SqlTypeName.DECIMAL); + + assertSame(value, IgniteSqlFunctions.toBigDecimal(value, defaultPrecision, 0)); + } + + /** Tests for decimal conversion function. */ + @ParameterizedTest + @CsvSource({ + // input, precision, scale, result (number or error) + "0, 1, 0, 0", + "0, 1, 2, 0.00", + "0, 2, 2, 0.00", + "0, 2, 4, 0.0000", + + "1, 1, 0, 1", + "1, 3, 2, 1.00", + "1, 2, 2, overflow", + + "0.1, 1, 1, 0.1", + + "0.12, 2, 1, 0.1", + "0.12, 2, 2, 0.12", + "0.123, 2, 2, 0.12", + "0.123, 2, 1, 0.1", + "0.123, 5, 5, 0.12300", + + "1.23, 2, 1, 1.2", + + "10, 2, 0, 10", + "10.0, 2, 0, 10", + "10, 3, 0, 10", + + "10.01, 2, 0, 10", + "10.1, 3, 0, 10", + "10.11, 2, 1, overflow", + "10.11, 3, 1, 10.1", + "10.00, 3, 1, 10.0", + + "100.0, 3, 1, overflow", + + "100.01, 4, 1, 100.0", + "100.01, 4, 0, 100", + "100.111, 3, 1, overflow", + + "11.1, 5, 3, 11.100", + "11.100, 5, 3, 11.100", + + "-10.1, 3, 1, -10.1", + "-10.1, 2, 0, -10", + "-10.1, 2, 1, overflow", + }) + public void testConvertDecimal(String input, int precision, int scale, String result) { + Supplier<BigDecimal> convert = () -> IgniteSqlFunctions.convertDecimal(new BigDecimal(input), precision, scale); + + if (!"overflow".equalsIgnoreCase(result)) { + BigDecimal expected = convert.get(); + assertEquals(new BigDecimal(result), expected); + } else { + assertThrows(SqlException.class, convert::get); + } + } } diff --git a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteralTest.java b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteralTest.java index aa3ab707ed..926e6c1105 100644 --- a/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteralTest.java +++ b/modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/sql/IgniteSqlDecimalLiteralTest.java @@ -26,6 +26,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.math.BigDecimal; +import org.apache.calcite.rel.type.RelDataType; import org.apache.calcite.sql.SqlLiteral; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.SqlWriter; @@ -34,10 +35,13 @@ import org.apache.calcite.sql.pretty.SqlPrettyWriter; import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.util.Litmus; import org.apache.ignite.internal.sql.engine.planner.AbstractPlannerTest; +import org.apache.ignite.internal.sql.engine.rel.IgniteRel; +import org.apache.ignite.internal.sql.engine.schema.IgniteSchema; import org.apache.ignite.internal.sql.engine.util.Commons; import org.apache.ignite.sql.SqlException; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; /** @@ -66,6 +70,32 @@ public class IgniteSqlDecimalLiteralTest extends AbstractPlannerTest { assertEquals(expectedType, actualType, "type"); } + /** + * Type of numeric decimal literal and type of decimal literal should match. + */ + @ParameterizedTest + @CsvSource({ + "-0.01", + "-0.1", + "-10.0", + "-10.122", + "0.0", + "0.1", + "0.01", + "10.0", + "10.122", + }) + public void testLiteralTypeMatch(String val) throws Exception { + String query = format("SELECT {}, DECIMAL '{}'", val, val); + + IgniteRel rel = physicalPlan(query, new IgniteSchema("PUBLIC")); + + RelDataType numericLitType = rel.getRowType().getFieldList().get(0).getType(); + RelDataType decimalLitType = rel.getRowType().getFieldList().get(1).getType(); + + assertEquals(numericLitType, decimalLitType); + } + /** * Tests {@link IgniteSqlDecimalLiteral#unparse(SqlWriter, int, int)}. */