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

[v1] Fix ABS integer overflow; add parsing of TINYINT #1716

Merged
merged 2 commits into from
Jan 15, 2025
Merged
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
2 changes: 1 addition & 1 deletion partiql-ast/src/main/java/org/partiql/ast/DataType.java
Original file line number Diff line number Diff line change
@@ -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;
// <numeric type> - <approximate numeric type>
public static final int FLOAT = 28;
public static final int REAL = 29;
Original file line number Diff line number Diff line change
@@ -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()),
Original file line number Diff line number Diff line change
@@ -34,17 +34,20 @@ 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(
// 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);"
@@ -71,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);"
@@ -104,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);"
@@ -137,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)"
@@ -158,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)"
@@ -175,5 +174,25 @@ class DataExceptionTest {
input = "CAST(1 AS BIGINT) / CAST(0 AS BIGINT)"
)
)

@JvmStatic
fun absOverflowTests() = listOf(
// 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))"
)
)
}
}
2 changes: 1 addition & 1 deletion partiql-parser/src/main/antlr/PartiQLParser.g4
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions partiql-parser/src/main/antlr/PartiQLTokens.g4
Original file line number Diff line number Diff line change
@@ -228,6 +228,7 @@ TEMPORARY: 'TEMPORARY';
THEN: 'THEN';
TIME: 'TIME';
TIMESTAMP: 'TIMESTAMP';
TINYINT: 'TINYINT';
TO: 'TO';
TRANSACTION: 'TRANSACTION';
TRANSLATE: 'TRANSLATE';
Original file line number Diff line number Diff line change
@@ -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()
Original file line number Diff line number Diff line change
@@ -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)
// <numeric type> - <approximate numeric type>
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()
// <numeric type> - <approximate numeric type>
DataType.FLOAT -> PType.real()
DataType.REAL -> PType.real()
Original file line number Diff line number Diff line change
@@ -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 <absolute value expression> is specified, then let N be the value of the immediately contained
<numeric value expression>.
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(