korlov42 commented on code in PR #4422:
URL: https://github.com/apache/ignite-3/pull/4422#discussion_r1772795573
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientSqlTest.java:
##########
@@ -398,18 +398,18 @@ void testResultSetMapping(boolean useStatement) {
@ValueSource(booleans = {true, false})
void testResultSetMappingAsync(boolean useStatement) {
IgniteSql sql = client().sql();
- String query = "select 1 as num, concat('hello ', ?) as str";
+ String query = "select 1 + ? as num, concat('hello ', ?) as str";
Review Comment:
why did you change this test?
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java:
##########
@@ -606,64 +604,51 @@ public void testDecimalCastsFromNumeric(RelDataType
inputType, Object input,
@ParameterizedTest(name = "{1} {2}")
@MethodSource("decimalOverflows")
public void testCalcOpOverflow(SqlTypeName type, String expr, Object
param) {
- if (param == EMPTY_PARAM) {
- assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr));
- } else {
- assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr, param));
- }
+ assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr, param));
}
private static Stream<Arguments> decimalOverflows() {
return Stream.of(
// BIGINT
- arguments(SqlTypeName.BIGINT, "SELECT 9223372036854775807 +
1", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT 9223372036854775807 *
2", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT -9223372036854775808 -
1", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT -(-9223372036854775807 -
1)", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT
-CAST(-9223372036854775808 AS BIGINT)", EMPTY_PARAM),
arguments(SqlTypeName.BIGINT, "SELECT -(?)",
-9223372036854775808L),
- arguments(SqlTypeName.BIGINT, "SELECT
-9223372036854775808/-1", EMPTY_PARAM),
+ arguments(SqlTypeName.BIGINT, "SELECT -CAST(? AS BIGINT)",
-9223372036854775808L),
// INTEGER
- arguments(SqlTypeName.INTEGER, "SELECT 2147483647 + 1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT CAST(CAST(2147483648 AS
BIGINT) AS INTEGER)", EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT 2147483647 * 2",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -2147483648 - 1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -(-2147483647 - 1)",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -CAST(-2147483648 AS
INTEGER)", EMPTY_PARAM),
arguments(SqlTypeName.INTEGER, "SELECT -(?)", -2147483648),
- arguments(SqlTypeName.INTEGER, "SELECT -2147483648/-1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "select
CAST(9223372036854775807.5 + 9223372036854775807.5 AS INTEGER)",
- EMPTY_PARAM),
+ arguments(SqlTypeName.INTEGER, "SELECT -CAST(? AS INTEGER)",
-2147483648),
// SMALLINT
Review Comment:
does it make sense to have similar test cases for all 4 types ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeSystem.java:
##########
@@ -82,6 +86,84 @@ public int getDefaultPrecision(SqlTypeName typeName) {
}
}
+ /**
+ * Extend passed type to the nearest bigger.
+ * TINYINT -> SMALLINT
+ * SMALLINT -> INTEGER
+ * INTEGER -> BIGINT
+ * BIGINT -> DECIMAL
+ *
+ * @param typeFactory Type factory.
+ * @param type Type need to be extended.
+ * @param nullable Nullability flag.
+ * @return Extended type.
+ */
+ private static RelDataType extendType(RelDataTypeFactory typeFactory,
RelDataType type, boolean nullable) {
+ switch (type.getSqlTypeName()) {
+ case TINYINT:
+ return
typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.SMALLINT),
+ nullable);
+ case SMALLINT:
+ return
typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.INTEGER),
+ nullable);
+ case INTEGER:
+ return
typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.BIGINT),
+ nullable);
+ case BIGINT:
+ return typeFactory.createTypeWithNullability(
+ typeFactory.createSqlType(
+ SqlTypeName.DECIMAL,
+
typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.DECIMAL),
Review Comment:
does it make sense to use minimal yet sufficient precision rather then
maximum possible value?
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java:
##########
@@ -606,64 +604,51 @@ public void testDecimalCastsFromNumeric(RelDataType
inputType, Object input,
@ParameterizedTest(name = "{1} {2}")
@MethodSource("decimalOverflows")
public void testCalcOpOverflow(SqlTypeName type, String expr, Object
param) {
- if (param == EMPTY_PARAM) {
- assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr));
- } else {
- assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr, param));
- }
+ assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr, param));
}
private static Stream<Arguments> decimalOverflows() {
return Stream.of(
// BIGINT
- arguments(SqlTypeName.BIGINT, "SELECT 9223372036854775807 +
1", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT 9223372036854775807 *
2", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT -9223372036854775808 -
1", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT -(-9223372036854775807 -
1)", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT
-CAST(-9223372036854775808 AS BIGINT)", EMPTY_PARAM),
arguments(SqlTypeName.BIGINT, "SELECT -(?)",
-9223372036854775808L),
- arguments(SqlTypeName.BIGINT, "SELECT
-9223372036854775808/-1", EMPTY_PARAM),
+ arguments(SqlTypeName.BIGINT, "SELECT -CAST(? AS BIGINT)",
-9223372036854775808L),
// INTEGER
- arguments(SqlTypeName.INTEGER, "SELECT 2147483647 + 1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT CAST(CAST(2147483648 AS
BIGINT) AS INTEGER)", EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT 2147483647 * 2",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -2147483648 - 1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -(-2147483647 - 1)",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -CAST(-2147483648 AS
INTEGER)", EMPTY_PARAM),
arguments(SqlTypeName.INTEGER, "SELECT -(?)", -2147483648),
- arguments(SqlTypeName.INTEGER, "SELECT -2147483648/-1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "select
CAST(9223372036854775807.5 + 9223372036854775807.5 AS INTEGER)",
- EMPTY_PARAM),
+ arguments(SqlTypeName.INTEGER, "SELECT -CAST(? AS INTEGER)",
-2147483648),
// SMALLINT
- arguments(SqlTypeName.SMALLINT, "SELECT 32000::SMALLINT +
1000::SMALLINT", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "select
CAST(9223372036854775807.5 + 9223372036854775807.5 AS SMALLINT)",
- EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST(CAST(33000 AS
BIGINT) AS SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST(CAST(33000 AS
FLOAT) AS SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST(CAST(33000 + 1 AS
FLOAT) AS SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT 17000::SMALLINT *
2::SMALLINT", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT -32000::SMALLINT -
1000::SMALLINT", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT -(-32767::SMALLINT -
1::SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT -CAST(-32768 AS
SMALLINT)", EMPTY_PARAM),
arguments(SqlTypeName.SMALLINT, "SELECT -CAST(? AS SMALLINT)",
-32768),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST (-32768 AS
SMALLINT)/-1::SMALLINT", EMPTY_PARAM),
// TINYINT
- arguments(SqlTypeName.TINYINT, "SELECT 2::TINYINT +
127::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "select
CAST(9223372036854775807.5 + 9223372036854775807.5 AS TINYINT)",
- EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 AS
BIGINT) AS TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 AS FLOAT)
AS TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 + 1 AS
FLOAT) AS TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT 2::TINYINT *
127::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -2::TINYINT -
127::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -(-127::TINYINT -
1::TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -CAST(-128 AS
TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -CAST(? AS TINYINT)",
-128),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(-128 AS
TINYINT)/-1::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 + 1 AS
FLOAT) AS TINYINT)", EMPTY_PARAM)
+ arguments(SqlTypeName.TINYINT, "SELECT -CAST(? AS TINYINT)",
-128)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("decimalOpTypeExtension")
+ public void testCalcOpDynParamOverflow(String expr, String expect, Object
param) {
Review Comment:
why did you name this test 'overflow' when no overflow is expected?
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItDataTypesTest.java:
##########
@@ -606,64 +604,51 @@ public void testDecimalCastsFromNumeric(RelDataType
inputType, Object input,
@ParameterizedTest(name = "{1} {2}")
@MethodSource("decimalOverflows")
public void testCalcOpOverflow(SqlTypeName type, String expr, Object
param) {
- if (param == EMPTY_PARAM) {
- assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr));
- } else {
- assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr, param));
- }
+ assertThrowsSqlException(RUNTIME_ERR, type.getName() + " out of
range", () -> sql(expr, param));
}
private static Stream<Arguments> decimalOverflows() {
return Stream.of(
// BIGINT
- arguments(SqlTypeName.BIGINT, "SELECT 9223372036854775807 +
1", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT 9223372036854775807 *
2", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT -9223372036854775808 -
1", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT -(-9223372036854775807 -
1)", EMPTY_PARAM),
- arguments(SqlTypeName.BIGINT, "SELECT
-CAST(-9223372036854775808 AS BIGINT)", EMPTY_PARAM),
arguments(SqlTypeName.BIGINT, "SELECT -(?)",
-9223372036854775808L),
- arguments(SqlTypeName.BIGINT, "SELECT
-9223372036854775808/-1", EMPTY_PARAM),
+ arguments(SqlTypeName.BIGINT, "SELECT -CAST(? AS BIGINT)",
-9223372036854775808L),
// INTEGER
- arguments(SqlTypeName.INTEGER, "SELECT 2147483647 + 1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT CAST(CAST(2147483648 AS
BIGINT) AS INTEGER)", EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT 2147483647 * 2",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -2147483648 - 1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -(-2147483647 - 1)",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "SELECT -CAST(-2147483648 AS
INTEGER)", EMPTY_PARAM),
arguments(SqlTypeName.INTEGER, "SELECT -(?)", -2147483648),
- arguments(SqlTypeName.INTEGER, "SELECT -2147483648/-1",
EMPTY_PARAM),
- arguments(SqlTypeName.INTEGER, "select
CAST(9223372036854775807.5 + 9223372036854775807.5 AS INTEGER)",
- EMPTY_PARAM),
+ arguments(SqlTypeName.INTEGER, "SELECT -CAST(? AS INTEGER)",
-2147483648),
// SMALLINT
- arguments(SqlTypeName.SMALLINT, "SELECT 32000::SMALLINT +
1000::SMALLINT", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "select
CAST(9223372036854775807.5 + 9223372036854775807.5 AS SMALLINT)",
- EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST(CAST(33000 AS
BIGINT) AS SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST(CAST(33000 AS
FLOAT) AS SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST(CAST(33000 + 1 AS
FLOAT) AS SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT 17000::SMALLINT *
2::SMALLINT", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT -32000::SMALLINT -
1000::SMALLINT", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT -(-32767::SMALLINT -
1::SMALLINT)", EMPTY_PARAM),
- arguments(SqlTypeName.SMALLINT, "SELECT -CAST(-32768 AS
SMALLINT)", EMPTY_PARAM),
arguments(SqlTypeName.SMALLINT, "SELECT -CAST(? AS SMALLINT)",
-32768),
- arguments(SqlTypeName.SMALLINT, "SELECT CAST (-32768 AS
SMALLINT)/-1::SMALLINT", EMPTY_PARAM),
// TINYINT
- arguments(SqlTypeName.TINYINT, "SELECT 2::TINYINT +
127::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "select
CAST(9223372036854775807.5 + 9223372036854775807.5 AS TINYINT)",
- EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 AS
BIGINT) AS TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 AS FLOAT)
AS TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 + 1 AS
FLOAT) AS TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT 2::TINYINT *
127::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -2::TINYINT -
127::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -(-127::TINYINT -
1::TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -CAST(-128 AS
TINYINT)", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT -CAST(? AS TINYINT)",
-128),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(-128 AS
TINYINT)/-1::TINYINT", EMPTY_PARAM),
- arguments(SqlTypeName.TINYINT, "SELECT CAST(CAST(200 + 1 AS
FLOAT) AS TINYINT)", EMPTY_PARAM)
+ arguments(SqlTypeName.TINYINT, "SELECT -CAST(? AS TINYINT)",
-128)
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("decimalOpTypeExtension")
+ public void testCalcOpDynParamOverflow(String expr, String expect, Object
param) {
+ assertEquals(sql(expr, param).get(0).get(0).toString(), expect);
+ }
+
+ private static Stream<Arguments> decimalOpTypeExtension() {
+ return Stream.of(
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-23232
+ // arguments("SELECT -9223372036854775808::BIGINT/-1::BIGINT",
"9223372036854775808"),
+ // arguments("SELECT -?::BIGINT/-1::BIGINT",
"9223372036854775808", "9223372036854775808"),
+ // arguments("SELECT -2147483648::INTEGER/-1::INTEGER",
"2147483648")/*,
+ // arguments("SELECT -32768::SMALLINT/-1::SMALLINT", "32768"),
+ // arguments("SELECT -128::TINYINT/-1::TINYINT", "128")
+
+ arguments("SELECT CAST(-? AS BIGINT)/-1",
"9223372036854775808", "9223372036854775808"),
+ arguments("SELECT CAST(-? AS INTEGER)/-1", "2147483648",
"2147483648"),
+ arguments("SELECT CAST(-? AS SMALLINT)/-1", "32768", "32768"),
+ arguments("SELECT CAST(-? AS TINYINT)/-1", "128", "128"),
+
+ arguments("SELECT CAST(-? AS BIGINT) * -1",
"9223372036854775808", "9223372036854775808"),
+ arguments("SELECT CAST(-? AS INTEGER) * -1", "2147483648",
"2147483648"),
+ arguments("SELECT CAST(-? AS SMALLINT) * -1", "32768",
"32768"),
+ arguments("SELECT CAST(-? AS TINYINT) * -1", "128", "128")
Review Comment:
does it make sense to add test cases not only with cast from string?
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/type/IgniteTypeSystemTest.java:
##########
@@ -168,5 +169,126 @@ private static Stream<Arguments> deriveAvgTypeArguments()
{
native2relationalType(typeFactory, NativeTypes.DOUBLE)
)
);
- }
+ }
+
+ @ParameterizedTest(name = "op={0} {3} {1}")
+ @MethodSource("deriveTypeArguments")
+ void deriveType(NativeType type1, NativeType type2, NativeType expected,
ArithmeticOp op) {
+ RelDataType relType1 = native2relationalType(Commons.typeFactory(),
type1);
+ RelDataType relType2 = native2relationalType(Commons.typeFactory(),
type2);
+ RelDataType relExpected = native2relationalType(Commons.typeFactory(),
expected);
+
+ switch (op) {
+ case DIV: {
+ RelDataType actual =
typeSystem.deriveDecimalDivideType(Commons.typeFactory(), relType1, relType2);
+ assertThat(actual, Matchers.equalTo(relExpected));
+ break;
+ }
+ case MULT: {
+ RelDataType actual =
typeSystem.deriveDecimalMultiplyType(Commons.typeFactory(), relType1, relType2);
+ assertThat(actual, Matchers.equalTo(relExpected));
+ break;
+ }
+ case ADD: {
+ RelDataType actual =
typeSystem.deriveDecimalPlusType(Commons.typeFactory(), relType1, relType2);
+ assertThat(actual, Matchers.equalTo(relExpected));
+ break;
+ }
+ default:
+ throw new IllegalArgumentException("Unexpected operation type:
" + op);
+ }
Review Comment:
let's split this on 3 different methods with 3 different set of arguments
##########
modules/platforms/dotnet/Apache.Ignite.Tests/Sql/SqlTests.cs:
##########
@@ -519,7 +519,7 @@ public async Task TestCustomDecimalScale()
await using var resultSet = await Client.Sql.ExecuteAsync(null,
"select cast((10 / ?) as decimal(20, 5))", 3m);
IIgniteTuple res = await resultSet.SingleAsync();
- Assert.AreEqual(3.33333m, res[0]);
+ Assert.AreEqual(3.00000m, res[0]);
Review Comment:
can you explain why the result has changed?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
