Skip to content

Commit

Permalink
IGNITE-23172 Sql. Lack of bounds validation in CAST(1 as DECIMAL(MAX_…
Browse files Browse the repository at this point in the history
…PREC, MAX_SCALE)) (#4566)
  • Loading branch information
korlov42 authored Oct 16, 2024
1 parent d3b0cc2 commit 55a896a
Show file tree
Hide file tree
Showing 17 changed files with 640 additions and 518 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ public async Task TestCustomDecimalScale()
[Test]
public async Task TestMaxDecimalScale()
{
await using var resultSet = await Client.Sql.ExecuteAsync(null, "select (10 / ?)", 3m);
await using var resultSet = await Client.Sql.ExecuteAsync(null, "select (10 / ?::decimal)", 3m);
IIgniteTuple res = await resultSet.SingleAsync();

var bigDecimal = (BigDecimal)res[0]!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,11 +416,11 @@ public void testFunctionArgsToNumericImplicitConversion() {
assertQuery("select decode(?, 0, 0, 1, 1.0)").withParams(0).returns(new BigDecimal("0.0")).check();
assertQuery("select decode(?, 0, 0, 1, 1.0)").withParams(1).returns(new BigDecimal("1.0")).check();
assertQuery("select decode(1, 0, 0, ?, 1.0)").withParams(1).returns(new BigDecimal("1.0")).check();
assertQuery("select decode(1, 0, 0, 1, ?)").withParams(new BigDecimal("1.0")).returns(new BigDecimal("1.0")).check();
assertQuery("select decode(1, 0, 0, 1, ?)").withParams(new BigDecimal("1.0")).returns(new BigDecimal("1.000000")).check();
assertQuery("select decode(?, 0, 0, 1, 1.000)").withParams(0).returns(new BigDecimal("0.000")).check();
assertQuery("select decode(?, 0, 0, 1, 1.000)").withParams(1).returns(new BigDecimal("1.000")).check();
assertQuery("select decode(1, 0, 0, ?, 1.000)").withParams(1).returns(new BigDecimal("1.000")).check();
assertQuery("select decode(1, 0, 0, 1, ?)").withParams(new BigDecimal("1.00")).returns(new BigDecimal("1.00")).check();
assertQuery("select decode(1, 0, 0, 1, ?)").withParams(new BigDecimal("1.00")).returns(new BigDecimal("1.000000")).check();
assertQuery("select decode(?, 0, 0.0, 1, 1.000)").withParams(0).returns(new BigDecimal("0.000")).check();
assertQuery("select decode(?, 0, 0.000, 1, 1.0)").withParams(1).returns(new BigDecimal("1.000")).check();
assertQuery("select decode(?, 0, 1.0, 1, 1, 5)").withParams(3).returns(new BigDecimal("5.0")).check();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
package org.apache.ignite.internal.sql.engine;

import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
import static org.apache.ignite.internal.sql.engine.prepare.IgniteSqlValidator.DECIMAL_DYNAMIC_PARAM_PRECISION;
import static org.apache.ignite.internal.sql.engine.prepare.IgniteSqlValidator.DECIMAL_DYNAMIC_PARAM_SCALE;
import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
import static org.apache.ignite.internal.testframework.IgniteTestUtils.await;
import static org.apache.ignite.lang.ErrorGroups.Sql.RUNTIME_ERR;
Expand All @@ -35,6 +37,7 @@
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
import org.apache.ignite.internal.sql.engine.prepare.IgniteSqlValidator;
import org.apache.ignite.internal.sql.engine.prepare.ParameterType;
import org.apache.ignite.internal.sql.engine.property.SqlProperties;
import org.apache.ignite.internal.sql.engine.property.SqlPropertiesHelper;
Expand All @@ -56,6 +59,7 @@
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.EnumSource.Mode;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

/** Dynamic parameters checks. */
public class ItDynamicParameterTest extends BaseSqlIntegrationTest {
Expand All @@ -81,9 +85,12 @@ void testMetadataTypesForDynamicParameters(ColumnType type) {
// TODO https://issues.apache.org/jira/browse/IGNITE-19162 Ignite SQL doesn't support precision more than 3 for temporal types.
if (type == ColumnType.TIME || type == ColumnType.TIMESTAMP || type == ColumnType.DATETIME) {
param = SqlTestUtils.generateValueByType(type, 3, -1);
} else if (type == ColumnType.DECIMAL) {
param = SqlTestUtils.generateValueByType(type, DECIMAL_DYNAMIC_PARAM_PRECISION, DECIMAL_DYNAMIC_PARAM_SCALE);
} else {
param = SqlTestUtils.generateValueByTypeWithMaxScalePrecisionForSql(type);
}

List<List<Object>> ret = sql("SELECT typeof(?)", param);
String type0 = (String) ret.get(0).get(0);

Expand All @@ -92,6 +99,46 @@ void testMetadataTypesForDynamicParameters(ColumnType type) {
assertQuery("SELECT ?").withParams(param).returns(param).columnMetadata(new MetadataMatcher().type(type)).check();
}

/**
* By default derived type of parameter of type BigDecimal is DECIMAL({@value IgniteSqlValidator#DECIMAL_DYNAMIC_PARAM_PRECISION},
* {@value IgniteSqlValidator#DECIMAL_DYNAMIC_PARAM_SCALE}). This test makes sure it's possible to go beyond default precision and scale
* by wrapping dynamic param placeholder with CAST operation.
*/
@ParameterizedTest
@ValueSource(ints = {10, 20, 30, 60, 120})
void testMetadataTypesForDecimalDynamicParameters(int precision) {
int scale = precision / 2;
Object param = SqlTestUtils.generateValueByType(ColumnType.DECIMAL, precision, scale);

String typeString = decimalTypeString(precision, scale);

assertQuery("SELECT typeof(CAST(? as " + typeString + "))")
.withParams(param)
.returns(typeString)
.check();

assertQuery("SELECT typeof(?::" + typeString + ")")
.withParams(param)
.returns(typeString)
.check();

assertQuery("SELECT CAST(? as " + typeString + ")")
.withParams(param)
.columnMetadata(new MetadataMatcher().type(ColumnType.DECIMAL).precision(precision).scale(scale))
.returns(param)
.check();

assertQuery("SELECT ?::" + typeString)
.withParams(param)
.columnMetadata(new MetadataMatcher().type(ColumnType.DECIMAL).precision(precision).scale(scale))
.returns(param)
.check();
}

private static String decimalTypeString(int precision, int scale) {
return format("DECIMAL({}, {})", precision, scale);
}

@Test
public void testDynamicParameters() {
assertQuery("SELECT COALESCE(null, ?)").withParams(13).returns(13).check();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ SELECT 0.3::DECIMAL(3, 5);
statement error: Numeric field overflow. A field with precision 3, scale 3 must round to an absolute value less than 1.
SELECT -1::DECIMAL(3, 3);

statement error: Numeric field overflow. A field with precision 3, scale 3 must round to an absolute value less than 1.
SELECT 1::DECIMAL(3, 3);

statement error: Numeric field overflow. A field with precision 32767, scale 32767 must round to an absolute value less than 1.
SELECT 1::DECIMAL(32767, 32767);

# the first cast produces must report an error
statement error: Numeric field overflow. A field with precision 3, scale 0 must round to an absolute value less than 10^3.
SELECT 9999999::DECIMAL(3)::DECIMAL(4, 2);
Expand Down Expand Up @@ -60,22 +66,22 @@ SELECT CAST(${val} AS DECIMAL);
query T
SELECT CAST(100.1::DECIMAL(4, 1) AS DECIMAL);
----
100.1
100

query T
SELECT CAST(100.1::REAL AS DECIMAL);
----
100.1
100

query T
SELECT CAST(100.1::FLOAT AS DECIMAL);
----
100.1
100

query T
SELECT CAST(100.1::DOUBLE AS DECIMAL);
----
100.1
100

query T
SELECT CAST(100.1234::DOUBLE AS DECIMAL(7, 2));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,10 @@ statement error
SELECT 17014118346046923173168730371588410572::DOUBLE::DECIMAL(4,0)

# decimal
query RRR
SELECT 0.01::DECIMAL * 10, 0.01::DECIMAL(10,1) * 10, 0.01::DECIMAL(10,3) * 10
query RRRR
SELECT 0.01::DECIMAL * 10, 0.01::DECIMAL(3,2) * 10, 0.01::DECIMAL(10,1) * 10, 0.01::DECIMAL(10,3) * 10
----
0.10 0.0 0.100
0 0.10 0.0 0.100

query R
SELECT 10::INTEGER::DECIMAL(10,3)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,25 @@ PRAGMA enable_verification

# negate
query II
SELECT -('0.1'::DECIMAL), -('-0.1'::DECIMAL)
SELECT -('0.1'::DECIMAL(2,1)), -('-0.1'::DECIMAL(2,1))
----
-0.1 0.1

# unary +
query II
SELECT +('0.1'::DECIMAL), +('-0.1'::DECIMAL)
SELECT +('0.1'::DECIMAL(2,1)), +('-0.1'::DECIMAL(2,1))
----
0.1 -0.1

# addition
query I
SELECT '0.1'::DECIMAL + '0.1'::DECIMAL
SELECT '0.1'::DECIMAL(2,1) + '0.1'::DECIMAL(2,1)
----
0.2

# addition with non-decimal
query I
SELECT '0.1'::DECIMAL + 1::INTEGER
SELECT '0.1'::DECIMAL(2,1) + 1::INTEGER
----
1.1

Expand Down Expand Up @@ -107,14 +107,14 @@ statement ok
INSERT INTO decimals VALUES (decimal '0.1'), (decimal '0.2');

query II
SELECT d + '0.1'::DECIMAL, d + 10000 FROM decimals ORDER BY d;
SELECT d + '0.1'::DECIMAL(2,1), d + 10000 FROM decimals ORDER BY d;
----
0.2 10000.1
0.3 10000.2

# multiplication
query I
SELECT '0.1'::DECIMAL * '10.0'::DECIMAL
SELECT '0.1'::DECIMAL(2,1) * '10.0'::DECIMAL(3,1)
----
1.0

Expand All @@ -124,13 +124,13 @@ SELECT typeof('0.1'::DECIMAL(2,1) * '10.0'::DECIMAL(3,1))
DECIMAL(5, 2)

query I
SELECT '0.1'::DECIMAL * '0.1'::DECIMAL
SELECT '0.1'::DECIMAL(2,1) * '0.1'::DECIMAL(2,1)
----
0.01

# multiplication with non-decimal
query I
SELECT '0.1'::DECIMAL * 10::INTEGER
SELECT '0.1'::DECIMAL(2,1) * 10::INTEGER
----
1.0

Expand All @@ -154,10 +154,10 @@ SELECT ('18.25'::DECIMAL(4,2) * '17.25'::DECIMAL(4,2))::VARCHAR

# different types
query IIII
SELECT '0.001'::DECIMAL * 100::TINYINT,
'0.001'::DECIMAL * 10000::SMALLINT,
'0.001'::DECIMAL * 1000000::INTEGER,
'0.001'::DECIMAL * 100000000::BIGINT
SELECT '0.001'::DECIMAL(4,3) * 100::TINYINT,
'0.001'::DECIMAL(4,3) * 10000::SMALLINT,
'0.001'::DECIMAL(4,3) * 1000000::INTEGER,
'0.001'::DECIMAL(4,3) * 100000000::BIGINT
----
0.100
10.000
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,31 @@ DECIMAL(32767, 0)

# test basic string conversions
query II
SELECT '0.1'::DECIMAL::VARCHAR, '922337203685478.758'::DECIMAL::VARCHAR;
SELECT '0.1'::DECIMAL(2,1)::VARCHAR, '922337203685478.758'::DECIMAL(18,3)::VARCHAR;
----
0.1 922337203685478.758

# negative values
query II
SELECT '-0.1'::DECIMAL::VARCHAR, '-922337203685478.758'::DECIMAL::VARCHAR;
SELECT '-0.1'::DECIMAL(2,1)::VARCHAR, '-922337203685478.758'::DECIMAL(18,3)::VARCHAR;
----
-0.1 -922337203685478.758

# some more difficult string conversions
query III
SELECT ' 7 '::DECIMAL::VARCHAR, '9.'::DECIMAL::VARCHAR, '.1'::DECIMAL::VARCHAR;
SELECT ' 7 '::DECIMAL::VARCHAR, '9.'::DECIMAL::VARCHAR, '.1'::DECIMAL(2,1)::VARCHAR;
----
7 9 0.1

# trailing decimals get truncated
query II
SELECT '0.123456789'::DECIMAL::VARCHAR, '-0.123456789'::DECIMAL::VARCHAR;
SELECT '0.123456789'::DECIMAL(10,9)::VARCHAR, '-0.123456789'::DECIMAL(10,9)::VARCHAR;
----
0.123456789 -0.123456789

# no overflow in conversion
query I
SELECT '9223372036854788.758'::DECIMAL;
SELECT '9223372036854788.758'::DECIMAL(19,3);
----
9223372036854788.758

Expand Down Expand Up @@ -68,7 +68,7 @@ SELECT '-1'::DECIMAL(3, 3)::VARCHAR;

# repeat the same cast many times
query I
select '0.1'::decimal::decimal::decimal;
select '0.1'::decimal(2,1)::decimal(2,1)::decimal(2,1);
----
0.1

Expand Down Expand Up @@ -120,7 +120,7 @@ SELECT '100.100'::DECIMAL(10,0)::VARCHAR
100

query R
SELECT '100.100'::DECIMAL::VARCHAR
SELECT '100.100'::DECIMAL(6,3)::VARCHAR
----
100.100

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ DELETE FROM decimals WHERE d <> d::DECIMAL(9,1)
#
# test ABS function
query III
SELECT ABS('-0.1'::DECIMAL), ABS('0.1'::DECIMAL), ABS(NULL::DECIMAL)
SELECT ABS('-0.1'::DECIMAL(2,1)), ABS('0.1'::DECIMAL(2,1)), ABS(NULL::DECIMAL)
----
0.1 0.1 NULL

Expand All @@ -115,17 +115,17 @@ SELECT ABS('-0.1'::DECIMAL(4,3)), ABS('-0.1'::DECIMAL(9,3)), ABS('-0.1'::DECIMAL

# test CEIL function
query III
SELECT CEIL('0.1'::DECIMAL), CEIL('-0.1'::DECIMAL), CEIL(NULL::DECIMAL)
SELECT CEIL('0.1'::DECIMAL(2,1)), CEIL('-0.1'::DECIMAL(2,1)), CEIL(NULL::DECIMAL)
----
1 0 NULL

query II
SELECT CEIL('100.3'::DECIMAL), CEIL('-127012.3'::DECIMAL)
SELECT CEIL('100.3'::DECIMAL(4,1)), CEIL('-127012.3'::DECIMAL(7,1))
----
101 -127012

query II
SELECT CEIL('10.5'::DECIMAL), CEIL('-10.5'::DECIMAL)
SELECT CEIL('10.5'::DECIMAL(3,1)), CEIL('-10.5'::DECIMAL(3,1))
----
11 -10

Expand All @@ -142,17 +142,17 @@ SELECT CEIL('-999.9'::DECIMAL(4,1)), CEIL('-99999999.9'::DECIMAL(9,1)), CEIL('-9

# test FLOOR function
query III
SELECT FLOOR('0.1'::DECIMAL), FLOOR('-0.1'::DECIMAL), FLOOR(NULL::DECIMAL)
SELECT FLOOR(decimal '0.1'), FLOOR(decimal '-0.1'), FLOOR(NULL::DECIMAL)
----
0 -1 NULL

query II
SELECT FLOOR('100.3'::DECIMAL), FLOOR('-127012.3'::DECIMAL)
SELECT FLOOR(decimal '100.3'), FLOOR(decimal '-127012.3')
----
100 -127013

query II
SELECT FLOOR('10.5'::DECIMAL), FLOOR('-10.5'::DECIMAL)
SELECT FLOOR(decimal '10.5'), FLOOR(decimal '-10.5')
----
10 -11

Expand All @@ -169,17 +169,17 @@ SELECT FLOOR('-999.9'::DECIMAL(4,1)), FLOOR('-99999999.9'::DECIMAL(9,1)), FLOOR(

# test unary ROUND function
query III
SELECT ROUND('0.1'::DECIMAL), ROUND('-0.1'::DECIMAL), ROUND(NULL::DECIMAL)
SELECT ROUND(decimal '0.1'), ROUND(decimal '-0.1'), ROUND(NULL::DECIMAL)
----
0 0 NULL

query II
SELECT ROUND('100.3'::DECIMAL), ROUND('-127012.3'::DECIMAL)
SELECT ROUND(decimal '100.3'), ROUND(decimal '-127012.3')
----
100 -127012

query II
SELECT ROUND('10.5'::DECIMAL), ROUND('-10.5'::DECIMAL)
SELECT ROUND(decimal '10.5'), ROUND(decimal '-10.5')
----
11 -11

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@
import org.apache.calcite.avatica.util.DateTimeUtils;
import org.apache.calcite.linq4j.function.NonDeterministic;
import org.apache.calcite.runtime.SqlFunctions;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.ignite.internal.lang.IgniteStringBuilder;
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.IgniteMath;
import org.apache.ignite.internal.sql.engine.util.TypeUtils;
Expand Down Expand Up @@ -337,16 +335,6 @@ public static BigDecimal toBigDecimal(Number value, int precision, int scale) {
return null;
}

int defaultPrecision = IgniteTypeSystem.INSTANCE.getDefaultPrecision(SqlTypeName.DECIMAL);

if (precision == defaultPrecision) {
BigDecimal dec = convertToBigDecimal(value);
// 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 dec;
}

if (value.longValue() == 0) {
return processFractionData(value, precision, scale);
} else {
Expand Down
Loading

0 comments on commit 55a896a

Please sign in to comment.