From e928d29f463ea32efe0018ef4b1a72652ffdb86c Mon Sep 17 00:00:00 2001 From: Alan Cai Date: Tue, 14 Jan 2025 16:52:55 -0800 Subject: [PATCH 1/2] [v1] Fix ABS integer overflow --- .../eval/internal/DataExceptionTest.kt | 25 ++++++++++++ .../partiql/spi/function/builtins/FnAbs.kt | 40 ++++++++++++++++--- 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt index d21387f7b..47c7ab762 100644 --- a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt +++ b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt @@ -34,6 +34,10 @@ class DataExceptionTest { @MethodSource("divideByZeroTests") fun divideByZero(tc: FailureTestCase) = tc.run() + @ParameterizedTest + @MethodSource("absOverflowTests") + fun absOverflow(tc: FailureTestCase) = tc.run() + companion object { @JvmStatic fun plusOverflowTests() = listOf( @@ -175,5 +179,26 @@ class DataExceptionTest { input = "CAST(1 AS BIGINT) / CAST(0 AS BIGINT)" ) ) + + @JvmStatic + fun absOverflowTests() = listOf( + // TINYINT + // TODO add parsing and planning support for TINYINT +// FailureTestCase( +// input = "ABS(CAST(${Byte.MIN_VALUE} AS TINYINT))" +// ), + // SMALLINT + FailureTestCase( + input = "ABS(CAST(${Short.MIN_VALUE} AS SMALLINT))" + ), + // INT + FailureTestCase( + input = "ABS(CAST(${Integer.MIN_VALUE} AS INT))" + ), + // BIGINT + FailureTestCase( + input = "ABS(CAST(${Long.MIN_VALUE} AS BIGINT))" + ) + ) } } diff --git a/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnAbs.kt b/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnAbs.kt index b54ec633d..b379c89e4 100644 --- a/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnAbs.kt +++ b/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnAbs.kt @@ -3,21 +3,36 @@ package org.partiql.spi.function.builtins +import org.partiql.spi.errors.DataException import org.partiql.spi.function.Function import org.partiql.spi.function.Parameter import org.partiql.spi.types.PType import org.partiql.spi.value.Datum import kotlin.math.absoluteValue -// TODO: When negate a negative value, we need to consider overflow +/* +ABS overflow behavior is specified in SQL1999 section 6.17: + 9) If is specified, then let N be the value of the immediately contained + . + Case: + a) If N is the null value, then the result is the null value. + b) If N >= 0, then the result is N. + c) Otherwise, the result is -1 * N. If -1 * N is not representable by the result data type, then + an exception condition is raised: data exception — numeric value out of range + */ + internal val Fn_ABS__INT8__INT8 = Function.static( name = "abs", parameters = arrayOf(Parameter("value", PType.tinyint())), returns = PType.tinyint(), ) { args -> - @Suppress("DEPRECATION") val value = args[0].byte - if (value < 0) Datum.tinyint(value.times(-1).toByte()) else Datum.tinyint(value) + if (value == Byte.MIN_VALUE) { + throw DataException("Resulting value out of range for TINYINT: ABS($value)") + } else { + val result = if (value < 0) (-value).toByte() else value + Datum.tinyint(result) + } } internal val Fn_ABS__INT16__INT16 = Function.static( @@ -26,7 +41,12 @@ internal val Fn_ABS__INT16__INT16 = Function.static( parameters = arrayOf(Parameter("value", PType.smallint())), ) { args -> val value = args[0].short - if (value < 0) Datum.smallint(value.times(-1).toShort()) else Datum.smallint(value) + if (value == Short.MIN_VALUE) { + throw DataException("Resulting value out of range for SMALLINT: ABS($value)") + } else { + val result = if (value < 0) (-value).toShort() else value + Datum.smallint(result) + } } internal val Fn_ABS__INT32__INT32 = Function.static( @@ -35,7 +55,11 @@ internal val Fn_ABS__INT32__INT32 = Function.static( parameters = arrayOf(Parameter("value", PType.integer())), ) { args -> val value = args[0].int - Datum.integer(value.absoluteValue) + if (value == Int.MIN_VALUE) { + throw DataException("Resulting value out of range for INT: ABS($value)") + } else { + Datum.integer(value.absoluteValue) + } } internal val Fn_ABS__INT64__INT64 = Function.static( @@ -44,7 +68,11 @@ internal val Fn_ABS__INT64__INT64 = Function.static( parameters = arrayOf(Parameter("value", PType.bigint())), ) { args -> val value = args[0].long - Datum.bigint(value.absoluteValue) + if (value == Long.MIN_VALUE) { + throw DataException("Resulting value out of range for BIGINT: ABS($value)") + } else { + Datum.bigint(value.absoluteValue) + } } internal val Fn_ABS__NUMERIC__NUMERIC = Function.static( From 3c44dbf0425f1d563a1863667416d6a719fc7443 Mon Sep 17 00:00:00 2001 From: Alan Cai Date: Tue, 14 Jan 2025 17:00:15 -0800 Subject: [PATCH 2/2] Add parse support for TINYINT --- .../main/java/org/partiql/ast/DataType.java | 2 +- .../org/partiql/ast/sql/SqlDialectTest.kt | 1 + .../eval/internal/DataExceptionTest.kt | 60 +++++++++---------- .../src/main/antlr/PartiQLParser.g4 | 2 +- .../src/main/antlr/PartiQLTokens.g4 | 1 + .../parser/internal/PartiQLParserDefault.kt | 1 + .../internal/transforms/RexConverter.kt | 4 +- 7 files changed, 34 insertions(+), 37 deletions(-) diff --git a/partiql-ast/src/main/java/org/partiql/ast/DataType.java b/partiql-ast/src/main/java/org/partiql/ast/DataType.java index 47f8a83b7..e34584f38 100644 --- a/partiql-ast/src/main/java/org/partiql/ast/DataType.java +++ b/partiql-ast/src/main/java/org/partiql/ast/DataType.java @@ -128,7 +128,7 @@ public String getComment() { public static final int INT2 = 24; public static final int INTEGER2 = 25; public static final int SMALLINT = 26; - public static final int TINYINT = 27; // TODO not defined in parser yet + public static final int TINYINT = 27; // - public static final int FLOAT = 28; public static final int REAL = 29; diff --git a/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt b/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt index 95cde0743..93f2becea 100644 --- a/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt +++ b/partiql-ast/src/test/kotlin/org/partiql/ast/sql/SqlDialectTest.kt @@ -240,6 +240,7 @@ class SqlDialectTest { fun types() = listOf( // SQL expect("BOOL", DataType.BOOL()), + expect("TINYINT", DataType.TINYINT()), expect("SMALLINT", DataType.SMALLINT()), expect("INT", DataType.INT()), expect("REAL", DataType.REAL()), diff --git a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt index 47c7ab762..c46f3630e 100644 --- a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt +++ b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt @@ -42,13 +42,12 @@ class DataExceptionTest { @JvmStatic fun plusOverflowTests() = listOf( // TINYINT - // TODO add parsing and planning support for TINYINT -// FailureTestCase( -// input = "CAST(${Byte.MAX_VALUE} AS TINYINT) + CAST(1 AS TINYINT);" -// ), -// FailureTestCase( -// input = "CAST(${Byte.MIN_VALUE} AS TINYINT) + CAST(-1 AS TINYINT);" -// ), + FailureTestCase( + input = "CAST(${Byte.MAX_VALUE} AS TINYINT) + CAST(1 AS TINYINT);" + ), + FailureTestCase( + input = "CAST(${Byte.MIN_VALUE} AS TINYINT) + CAST(-1 AS TINYINT);" + ), // SMALLINT FailureTestCase( input = "CAST(${Short.MAX_VALUE} AS SMALLINT) + CAST(1 AS SMALLINT);" @@ -75,13 +74,12 @@ class DataExceptionTest { @JvmStatic fun minusOverflowTests() = listOf( // TINYINT - // TODO add parsing and planning support for TINYINT -// FailureTestCase( -// input = "CAST(${Byte.MAX_VALUE} AS TINYINT) - CAST(-1 AS TINYINT);" -// ), -// FailureTestCase( -// input = "CAST(${Byte.MIN_VALUE} AS TINYINT) - CAST(1 AS TINYINT);" -// ), + FailureTestCase( + input = "CAST(${Byte.MAX_VALUE} AS TINYINT) - CAST(-1 AS TINYINT);" + ), + FailureTestCase( + input = "CAST(${Byte.MIN_VALUE} AS TINYINT) - CAST(1 AS TINYINT);" + ), // SMALLINT FailureTestCase( input = "CAST(${Short.MAX_VALUE} AS SMALLINT) - CAST(-1 AS SMALLINT);" @@ -108,13 +106,12 @@ class DataExceptionTest { @JvmStatic fun timesOverflowTests() = listOf( // TINYINT - // TODO add parsing and planning support for TINYINT -// FailureTestCase( -// input = "CAST(${Byte.MAX_VALUE} AS TINYINT) * CAST(2 AS TINYINT);" -// ), -// FailureTestCase( -// input = "CAST(${Byte.MIN_VALUE} AS TINYINT) * CAST(2 AS TINYINT);" -// ), + FailureTestCase( + input = "CAST(${Byte.MAX_VALUE} AS TINYINT) * CAST(2 AS TINYINT);" + ), + FailureTestCase( + input = "CAST(${Byte.MIN_VALUE} AS TINYINT) * CAST(2 AS TINYINT);" + ), // SMALLINT FailureTestCase( input = "CAST(${Short.MAX_VALUE} AS SMALLINT) * CAST(2 AS SMALLINT);" @@ -141,10 +138,9 @@ class DataExceptionTest { @JvmStatic fun divideTests() = listOf( // TINYINT - // TODO add parsing and planning support for TINYINT -// FailureTestCase( -// input = "CAST(${Byte.MIN_VALUE} AS TINYINT) / CAST(-1 AS TINYINT)" -// ), + FailureTestCase( + input = "CAST(${Byte.MIN_VALUE} AS TINYINT) / CAST(-1 AS TINYINT)" + ), // SMALLINT FailureTestCase( input = "CAST(${Short.MIN_VALUE} AS SMALLINT) / CAST(-1 AS SMALLINT)" @@ -162,10 +158,9 @@ class DataExceptionTest { @JvmStatic fun divideByZeroTests() = listOf( // TINYINT - // TODO add parsing and planning support for TINYINT -// FailureTestCase( -// input = "CAST(1 AS TINYINT) / CAST(0 AS TINYINT)" -// ), + FailureTestCase( + input = "CAST(1 AS TINYINT) / CAST(0 AS TINYINT)" + ), // SMALLINT FailureTestCase( input = "CAST(1 AS SMALLINT) / CAST(0 AS SMALLINT)" @@ -183,10 +178,9 @@ class DataExceptionTest { @JvmStatic fun absOverflowTests() = listOf( // TINYINT - // TODO add parsing and planning support for TINYINT -// FailureTestCase( -// input = "ABS(CAST(${Byte.MIN_VALUE} AS TINYINT))" -// ), + FailureTestCase( + input = "ABS(CAST(${Byte.MIN_VALUE} AS TINYINT))" + ), // SMALLINT FailureTestCase( input = "ABS(CAST(${Short.MIN_VALUE} AS SMALLINT))" diff --git a/partiql-parser/src/main/antlr/PartiQLParser.g4 b/partiql-parser/src/main/antlr/PartiQLParser.g4 index a94349b44..429818f68 100644 --- a/partiql-parser/src/main/antlr/PartiQLParser.g4 +++ b/partiql-parser/src/main/antlr/PartiQLParser.g4 @@ -941,7 +941,7 @@ literal type : datatype=( - BOOL | BOOLEAN | SMALLINT | INTEGER2 | INT2 | INTEGER | INT | INTEGER4 | INT4 + BOOL | BOOLEAN | TINYINT | SMALLINT | INTEGER2 | INT2 | INTEGER | INT | INTEGER4 | INT4 | INTEGER8 | INT8 | BIGINT | REAL | CHAR | CHARACTER | STRING | SYMBOL | BLOB | CLOB | DATE | ANY ) # TypeAtomic diff --git a/partiql-parser/src/main/antlr/PartiQLTokens.g4 b/partiql-parser/src/main/antlr/PartiQLTokens.g4 index c4c9f6a53..a4d69cd94 100644 --- a/partiql-parser/src/main/antlr/PartiQLTokens.g4 +++ b/partiql-parser/src/main/antlr/PartiQLTokens.g4 @@ -228,6 +228,7 @@ TEMPORARY: 'TEMPORARY'; THEN: 'THEN'; TIME: 'TIME'; TIMESTAMP: 'TIMESTAMP'; +TINYINT: 'TINYINT'; TO: 'TO'; TRANSACTION: 'TRANSACTION'; TRANSLATE: 'TRANSLATE'; diff --git a/partiql-parser/src/main/kotlin/org/partiql/parser/internal/PartiQLParserDefault.kt b/partiql-parser/src/main/kotlin/org/partiql/parser/internal/PartiQLParserDefault.kt index 42ce7fdf0..a612c176f 100644 --- a/partiql-parser/src/main/kotlin/org/partiql/parser/internal/PartiQLParserDefault.kt +++ b/partiql-parser/src/main/kotlin/org/partiql/parser/internal/PartiQLParserDefault.kt @@ -1998,6 +1998,7 @@ internal class PartiQLParserDefault : PartiQLParser { when (ctx.datatype.type) { GeneratedParser.BOOL -> DataType.BOOLEAN() GeneratedParser.BOOLEAN -> DataType.BOOL() + GeneratedParser.TINYINT -> DataType.TINYINT() GeneratedParser.SMALLINT -> DataType.SMALLINT() GeneratedParser.INT2 -> DataType.INT2() GeneratedParser.INTEGER2 -> DataType.INTEGER2() diff --git a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RexConverter.kt b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RexConverter.kt index 2a2eb5c32..03bfbf22d 100644 --- a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RexConverter.kt +++ b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RexConverter.kt @@ -861,7 +861,7 @@ internal object RexConverter { DataType.INT4, DataType.INTEGER4, DataType.INTEGER -> call(FunctionUtils.OP_IS_INT32, arg0) DataType.INT -> call(FunctionUtils.OP_IS_INT, arg0) DataType.INT2, DataType.SMALLINT -> call(FunctionUtils.OP_IS_INT16, arg0) - DataType.TINYINT -> call(FunctionUtils.OP_IS_INT8, arg0) // TODO define in parser + DataType.TINYINT -> call(FunctionUtils.OP_IS_INT8, arg0) // - DataType.FLOAT -> call(FunctionUtils.OP_IS_FLOAT32, arg0) DataType.REAL -> call(FunctionUtils.OP_IS_REAL, arg0) @@ -1088,7 +1088,7 @@ internal object RexConverter { DataType.BIGINT, DataType.INT8, DataType.INTEGER8 -> PType.bigint() DataType.INT4, DataType.INTEGER4, DataType.INTEGER, DataType.INT -> PType.integer() DataType.INT2, DataType.SMALLINT -> PType.smallint() - DataType.TINYINT -> PType.tinyint() // TODO define in parser + DataType.TINYINT -> PType.tinyint() // - DataType.FLOAT -> PType.real() DataType.REAL -> PType.real()