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
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
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
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);"
Expand All @@ -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);"
Expand All @@ -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);"
Expand All @@ -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)"
Expand All @@ -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)"
Expand All @@ -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
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions partiql-parser/src/main/antlr/PartiQLTokens.g4
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ TEMPORARY: 'TEMPORARY';
THEN: 'THEN';
TIME: 'TIME';
TIMESTAMP: 'TIMESTAMP';
TINYINT: 'TINYINT';
TO: 'TO';
TRANSACTION: 'TRANSACTION';
TRANSLATE: 'TRANSLATE';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
Loading