Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IGNITE-23172 Sql. Lack of bounds validation in CAST(1 as DECIMAL(MAX_PREC, MAX_SCALE)) #4566

Merged
merged 7 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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