Skip to content

Commit

Permalink
[v1] Change integer types to error on overflow; give explicit data ex…
Browse files Browse the repository at this point in the history 
…ception on div/mod by 0
  • Loading branch information
@alancai98
alancai98 committed Jan 14, 2025
1 parent 0868284 commit 34f1aa3
Show file tree
Hide file tree
Showing 10 changed files with 385 additions and 47 deletions.
179 changes: 179 additions & 0 deletions partiql-eval/src/test/kotlin/org/partiql/eval/internal/DataExceptionTest.kt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
package org.partiql.eval.internal

import org.junit.jupiter.api.parallel.Execution
import org.junit.jupiter.api.parallel.ExecutionMode
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource

/**
* E2E evaluation tests that give a data exception.
*/
class DataExceptionTest {

@ParameterizedTest
@MethodSource("plusOverflowTests")
@Execution(ExecutionMode.CONCURRENT)
fun plusOverflow(tc: FailureTestCase) = tc.run()

@ParameterizedTest
@MethodSource("minusOverflowTests")
@Execution(ExecutionMode.CONCURRENT)
fun minusOverflow(tc: FailureTestCase) = tc.run()

@ParameterizedTest
@MethodSource("timesOverflowTests")
@Execution(ExecutionMode.CONCURRENT)
fun timesOverflow(tc: FailureTestCase) = tc.run()

@ParameterizedTest
@MethodSource("divideTests")
@Execution(ExecutionMode.CONCURRENT)
fun divideOverflow(tc: FailureTestCase) = tc.run()

@ParameterizedTest
@MethodSource("divideByZeroTests")
fun divideByZero(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);"
// ),
// SMALLINT
FailureTestCase(
input = "CAST(${Short.MAX_VALUE} AS SMALLINT) + CAST(1 AS SMALLINT);"
),
FailureTestCase(
input = "CAST(${Short.MIN_VALUE} AS SMALLINT) + CAST(-1 AS SMALLINT);"
),
// INT
FailureTestCase(
input = "CAST(${Integer.MAX_VALUE} AS INT) + CAST(1 AS INT);"
),
FailureTestCase(
input = "CAST(${Integer.MIN_VALUE} AS INT) + CAST(-1 AS INT);"
),
// BIGINT
FailureTestCase(
input = "CAST(${Long.MAX_VALUE} AS BIGINT) + CAST(1 AS BIGINT);"
),
FailureTestCase(
input = "CAST(${Long.MIN_VALUE} AS BIGINT) + CAST(-1 AS BIGINT);"
)
)

@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);"
// ),
// SMALLINT
FailureTestCase(
input = "CAST(${Short.MAX_VALUE} AS SMALLINT) - CAST(-1 AS SMALLINT);"
),
FailureTestCase(
input = "CAST(${Short.MIN_VALUE} AS SMALLINT) - CAST(1 AS SMALLINT);"
),
// INT
FailureTestCase(
input = "CAST(${Integer.MAX_VALUE} AS INT) - CAST(-1 AS INT);"
),
FailureTestCase(
input = "CAST(${Integer.MIN_VALUE} AS INT) - CAST(1 AS INT);"
),
// BIGINT
FailureTestCase(
input = "CAST(${Long.MAX_VALUE} AS BIGINT) - CAST(-1 AS BIGINT);"
),
FailureTestCase(
input = "CAST(${Long.MIN_VALUE} AS BIGINT) - CAST(1 AS BIGINT);"
)
)

@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);"
// ),
// SMALLINT
FailureTestCase(
input = "CAST(${Short.MAX_VALUE} AS SMALLINT) * CAST(2 AS SMALLINT);"
),
FailureTestCase(
input = "CAST(${Short.MIN_VALUE} AS SMALLINT) * CAST(2 AS SMALLINT);"
),
// INT
FailureTestCase(
input = "CAST(${Integer.MAX_VALUE} AS INT) * CAST(2 AS INT);"
),
FailureTestCase(
input = "CAST(${Integer.MIN_VALUE} AS INT) * CAST(2 AS INT);"
),
// BIGINT
FailureTestCase(
input = "CAST(${Long.MAX_VALUE} AS BIGINT) * CAST(2 AS BIGINT);"
),
FailureTestCase(
input = "CAST(${Long.MIN_VALUE} AS BIGINT) * CAST(2 AS BIGINT);"
)
)

@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)"
// ),
// SMALLINT
FailureTestCase(
input = "CAST(${Short.MIN_VALUE} AS SMALLINT) / CAST(-1 AS SMALLINT)"
),
// INT
FailureTestCase(
input = "CAST(${Integer.MIN_VALUE} AS INT) / CAST(-1 AS INT)"
),
// BIGINT
FailureTestCase(
input = "CAST(${Long.MIN_VALUE} AS BIGINT) / CAST(-1 AS BIGINT)"
)
)

@JvmStatic
fun divideByZeroTests() = listOf(
// TINYINT
// TODO add parsing and planning support for TINYINT
// FailureTestCase(
// input = "CAST(1 AS TINYINT) / CAST(0 AS TINYINT)"
// ),
// SMALLINT
FailureTestCase(
input = "CAST(1 AS SMALLINT) / CAST(0 AS SMALLINT)"
),
// INT
FailureTestCase(
input = "CAST(1 AS INT) / CAST(0 AS INT)"
),
// BIGINT
FailureTestCase(
input = "CAST(1 AS BIGINT) / CAST(0 AS BIGINT)"
)
)
}
}
8 changes: 4 additions & 4 deletions partiql-eval/src/test/kotlin/org/partiql/eval/internal/PartiQLEvaluatorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ class PartiQLEvaluatorTest {
int64Value(2),
),
globals = listOf(
SuccessTestCase.Global(
Global(
name = "t",
value = """
[
Expand All @@ -150,7 +150,7 @@ class PartiQLEvaluatorTest {
int64Value(2),
),
globals = listOf(
SuccessTestCase.Global(
Global(
name = "t",
value = """
[
Expand All @@ -177,7 +177,7 @@ class PartiQLEvaluatorTest {
),
),
globals = listOf(
SuccessTestCase.Global(
Global(
name = "customers",
value = """
[{id:1, name: "Mary"},
Expand All @@ -186,7 +186,7 @@ class PartiQLEvaluatorTest {
]
"""
),
SuccessTestCase.Global(
Global(
name = "orders",
value = """
[{custId:1, name: "foo"},
Expand Down
2 changes: 1 addition & 1 deletion partiql-eval/src/test/kotlin/org/partiql/eval/internal/PlusTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class PlusTest {
mode = Mode.STRICT(),
expected = Datum.decimal(BigDecimal.valueOf(457023), 14, 7),
globals = listOf(
SuccessTestCase.Global(
Global(
"dynamic_decimal",
"456789.0000000"
)
Expand Down
66 changes: 57 additions & 9 deletions .../partiql/eval/internal/SuccessTestCase.kt → ...in/org/partiql/eval/internal/TestCases.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ import org.partiql.types.fromStaticType
import org.partiql.value.PartiQLValue
import kotlin.test.assertEquals

/**
* @property value is a serialized Ion value.
*/
class Global(
val name: String,
val value: String,
val type: StaticType = StaticType.ANY,
)

public class SuccessTestCase(
val input: String,
val expected: Datum,
Expand All @@ -37,15 +46,6 @@ public class SuccessTestCase(
private val parser = PartiQLParser.standard()
private val planner = PartiQLPlanner.standard()

/**
* @property value is a serialized Ion value.
*/
class Global(
val name: String,
val value: String,
val type: StaticType = StaticType.ANY,
)

override fun run() {
val parseResult = parser.parse(input)
assertEquals(1, parseResult.statements.size)
Expand Down Expand Up @@ -92,3 +92,51 @@ public class SuccessTestCase(
return input
}
}

public class FailureTestCase(
val input: String,
val mode: Mode = Mode.STRICT(), // default to run in STRICT mode
val globals: List<Global> = emptyList(),
) : PTestCase {
private val compiler = PartiQLCompiler.standard()
private val parser = PartiQLParser.standard()
private val planner = PartiQLPlanner.standard()

override fun run() {
val parseResult = parser.parse(input)
assertEquals(1, parseResult.statements.size)
val statement = parseResult.statements[0]
val catalog = Catalog.builder()
.name("memory")
.apply {
globals.forEach {
val table = Table.standard(
name = Name.of(it.name),
schema = fromStaticType(it.type),
datum = DatumReader.ion(it.value.byteInputStream()).next()!!
)
define(table)
}
}
.build()
val session = Session.builder()
.catalog("memory")
.catalogs(catalog)
.build()
var thrown: Throwable? = null
val plan = planner.plan(statement, session).plan
val actual: Datum = try {
DatumMaterialize.materialize(compiler.prepare(plan, mode).execute())
} catch (t: Throwable) {
thrown = t
Datum.nullValue()
}
if (thrown == null) {
val message = buildString {
appendLine("Expected error to be thrown but none was thrown.")
appendLine("Actual Result: $actual")
}
error(message)
}
}
}
25 changes: 23 additions & 2 deletions partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/FnDivide.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

package org.partiql.spi.function.builtins

import org.partiql.spi.errors.DataException
import org.partiql.spi.function.Function
import org.partiql.spi.types.PType
import org.partiql.spi.value.Datum
Expand All @@ -15,8 +16,13 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {

override fun getTinyIntInstance(tinyIntLhs: PType, tinyIntRhs: PType): Function.Instance {
return basic(PType.tinyint()) { args ->
@Suppress("DEPRECATION") val arg0 = args[0].byte
@Suppress("DEPRECATION") val arg1 = args[1].byte
val arg0 = args[0].byte
val arg1 = args[1].byte
if (arg1 == 0.toByte()) {
throw DataException("Division by zero for TINYINT: $arg0 / $arg1")
} else if (arg0 == Byte.MIN_VALUE && arg1.toInt() == -1) {
throw DataException("Overflow for TINYINT: $arg0 / $arg1")
}
Datum.tinyint((arg0 / arg1).toByte())
}
}
Expand All @@ -25,6 +31,11 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {
return basic(PType.smallint()) { args ->
val arg0 = args[0].short
val arg1 = args[1].short
if (arg1 == 0.toShort()) {
throw DataException("Division by zero for SMALLINT: $arg0 / $arg1")
} else if (arg0 == Short.MIN_VALUE && arg1.toInt() == -1) {
throw DataException("Overflow for SMALLINT: $arg0 / $arg1")
}
Datum.smallint((arg0 / arg1).toShort())
}
}
Expand All @@ -33,6 +44,11 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {
return basic(PType.integer()) { args ->
val arg0 = args[0].int
val arg1 = args[1].int
if (arg1 == 0) {
throw DataException("Division by zero for INT: $arg0 / $arg1")
} else if (arg0 == Int.MIN_VALUE && arg1 == -1) {
throw DataException("Overflow for INT: $arg0 / $arg1")
}
Datum.integer(arg0 / arg1)
}
}
Expand All @@ -41,6 +57,11 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {
return basic(PType.bigint()) { args ->
val arg0 = args[0].long
val arg1 = args[1].long
if (arg1 == 0L) {
throw DataException("Division by zero for BIGINT: $arg0 / $arg1")
} else if (arg0 == Long.MIN_VALUE && arg1 == -1L) {
throw DataException("Overflow for BIGINT: $arg0 / $arg1")
}
Datum.bigint(arg0 / arg1)
}
}
Expand Down
Loading

0 comments on commit 34f1aa3

Please sign in to comment.