Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
6 changes: 3 additions & 3 deletions partiql-ast/src/main/java/org/partiql/ast/DataType.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static class StructField extends AstNode {

private final boolean optional;

@Nullable
@NotNull
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(self-review) lists should be @NotNull rather than @Nullable. Taken from #1692

private final List<AttributeConstraint> constraints;

@Nullable
Expand All @@ -48,7 +48,7 @@ public StructField(
@NotNull Identifier.Simple name,
@NotNull DataType type,
boolean optional,
@Nullable List<AttributeConstraint> constraints,
@NotNull List<AttributeConstraint> constraints,
@Nullable String comment) {
this.name = name;
this.type = type;
Expand Down Expand Up @@ -86,7 +86,7 @@ public boolean isOptional() {
return this.optional;
}

@Nullable
@NotNull
public List<AttributeConstraint> getConstraints() {
return this.constraints;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,10 @@ object ErrorMessageFormatter {
*/
private fun divisionByZero(error: PError): String {
val dividendType = error.getOrNull("DIVIDEND_TYPE", PType::class.java)
val dividendTypeStr = prepare(dividendType.toString(), " of ")
val dividendTypeStr = prepare(dividendType.toString(), " of type ")
val dividend = error.getOrNull("DIVIDEND", String::class.java)
val dividendStr = prepare(dividend.toString(), " -- ", " / 0")
return "Division$dividendTypeStr by zero$dividendStr$dividendTypeStr."
val dividendStr = prepare(dividend.toString(), " ")
return "Cannot divide$dividendStr$dividendTypeStr by zero."
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(self-review) error message follows what's suggested from PError. This new error allows for reuse for / and %. Also the 0 may be a decimal or floating point zero.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,12 @@ internal class ExprCallDynamic(
* This implementation assumes that the [eval] input values contains the original arguments for the desired [function].
* It performs the coercions (if necessary) before computing the result.
*
* TODO what about MISSING calls?
*
* @see ExprCallDynamic
*/
private class Candidate(private var function: Function.Instance) {

private var nil = { Datum.nullValue(function.returns) }
private var missing = { Datum.missing(function.returns) }

/**
* Function instance parameters (just types).
Expand All @@ -175,6 +174,9 @@ internal class ExprCallDynamic(
if (function.isNullCall && arg.isNull) {
return nil.invoke()
}
if (function.isMissingCall && arg.isMissing) {
return missing.invoke()
}
val argType = arg.type
val paramType = function.parameters[i]
when (paramType == argType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import org.junit.jupiter.params.provider.MethodSource
* E2E evaluation tests that give a data exception.
*/
class DataExceptionTest {

@ParameterizedTest
@MethodSource("plusOverflowTests")
@Execution(ExecutionMode.CONCURRENT)
Expand All @@ -34,6 +33,10 @@ class DataExceptionTest {
@MethodSource("divideByZeroTests")
fun divideByZero(tc: FailureTestCase) = tc.run()

@ParameterizedTest
@MethodSource("modByZeroTests")
fun modByZero(tc: FailureTestCase) = tc.run()

@ParameterizedTest
@MethodSource("absOverflowTests")
fun absOverflow(tc: FailureTestCase) = tc.run()
Expand Down Expand Up @@ -184,6 +187,58 @@ class DataExceptionTest {
// BIGINT
FailureTestCase(
input = "CAST(1 AS BIGINT) / CAST(0 AS BIGINT)"
),
// DECIMAL
FailureTestCase(
input = "CAST(1.0 AS DECIMAL) / CAST(0.0 AS DECIMAL)"
),
// NUMERIC
FailureTestCase(
input = "CAST(1.0 AS NUMERIC) / CAST(0.0 AS NUMERIC)"
),
// FLOAT
FailureTestCase(
input = "CAST(1.0e0 AS FLOAT) / CAST(0.0 AS FLOAT)"
),
// REAL
FailureTestCase(
input = "CAST(1.0e0 AS DOUBLE PRECISION) / CAST(0.0 AS DOUBLE PRECISION)"
)
)

@JvmStatic
fun modByZeroTests() = listOf(
// 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)"
),
// DECIMAL
FailureTestCase(
input = "CAST(1.0 AS DECIMAL) % CAST(0.0 AS DECIMAL)"
),
// NUMERIC
FailureTestCase(
input = "CAST(1.0 AS NUMERIC) % CAST(0.0 AS NUMERIC)"
),
// FLOAT
FailureTestCase(
input = "CAST(1.0e0 AS FLOAT) % CAST(0.0 AS FLOAT)"
),
// REAL
FailureTestCase(
input = "CAST(1.0e0 AS DOUBLE PRECISION) % CAST(0.0 AS DOUBLE PRECISION)"
)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2044,7 +2044,13 @@ internal class PartiQLParserDefault : PartiQLParser {
GeneratedParser.INT8 -> DataType.INT8()
GeneratedParser.INTEGER8 -> DataType.INTEGER8()
GeneratedParser.FLOAT -> DataType.FLOAT()
GeneratedParser.DOUBLE -> TODO() // not sure if DOUBLE is to be supported
GeneratedParser.DOUBLE -> {
if (ctx.stop.type == GeneratedParser.PRECISION) {
DataType.DOUBLE_PRECISION()
} else {
throw error(ctx, "Unknown atomic type.")
}
} // not sure if DOUBLE is to be supported
GeneratedParser.REAL -> DataType.REAL()
GeneratedParser.TIMESTAMP -> DataType.TIMESTAMP()
GeneratedParser.CHAR -> DataType.CHAR()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ public class Identifier private constructor(
) : Iterable<Identifier.Simple> {

/**
* Returns the unqualified name part.
* Returns the right-most simple identifier of the qualified identifier. For example, for an identifier
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(self-review) copied from AST's Identifier javadoc

* a.b.c this method would return c.
*/
public fun getIdentifier(): Simple = identifier

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package org.partiql.spi.function.builtins

import org.partiql.spi.function.Function
import org.partiql.spi.function.builtins.internal.PErrors
import org.partiql.spi.internal.isZero
import org.partiql.spi.types.PType
import org.partiql.spi.value.Datum
import java.math.RoundingMode
Expand Down Expand Up @@ -72,6 +73,9 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {
return basic(PType.numeric(p, s), numericLhs, numericRhs) { args ->
val arg0 = args[0].bigDecimal
val arg1 = args[1].bigDecimal
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.numeric(p, s))
}
val result = arg0.divide(arg1, s, RoundingMode.HALF_UP)
Datum.numeric(result, p, s)
}
Expand All @@ -82,6 +86,9 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {
return basic(PType.decimal(p, s), decimalLhs, decimalRhs) { args ->
val arg0 = args[0].bigDecimal
val arg1 = args[1].bigDecimal
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.decimal(p, s))
}
val result = arg0.divide(arg1, s, RoundingMode.HALF_UP)
Datum.decimal(result, p, s)
}
Expand All @@ -105,6 +112,9 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {
return basic(PType.real()) { args ->
val arg0 = args[0].float
val arg1 = args[1].float
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.real())
}
Datum.real(arg0 / arg1)
}
}
Expand All @@ -113,6 +123,9 @@ internal object FnDivide : DiadicArithmeticOperator("divide") {
return basic(PType.doublePrecision()) { args ->
val arg0 = args[0].double
val arg1 = args[1].double
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.doublePrecision())
}
Datum.doublePrecision(arg0 / arg1)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package org.partiql.spi.function.builtins

import org.partiql.spi.function.Function
import org.partiql.spi.function.builtins.internal.PErrors
import org.partiql.spi.internal.isZero
import org.partiql.spi.types.PType
import org.partiql.spi.value.Datum

Expand Down Expand Up @@ -67,6 +68,9 @@ internal object FnModulo : DiadicArithmeticOperator("mod", false) {
return basic(PType.numeric(p, s), numericLhs, numericRhs) { args ->
val arg0 = args[0].bigDecimal
val arg1 = args[1].bigDecimal
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.numeric(p, s))
}
Datum.numeric(arg0 % arg1, p, s)
}
}
Expand All @@ -76,6 +80,9 @@ internal object FnModulo : DiadicArithmeticOperator("mod", false) {
return basic(PType.decimal(p, s), decimalLhs, decimalRhs) { args ->
val arg0 = args[0].bigDecimal
val arg1 = args[1].bigDecimal
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.decimal(p, s))
}
Datum.decimal(arg0 % arg1, p, s)
}
}
Expand All @@ -99,6 +106,9 @@ internal object FnModulo : DiadicArithmeticOperator("mod", false) {
return basic(PType.real()) { args ->
val arg0 = args[0].float
val arg1 = args[1].float
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.real())
}
Datum.real(arg0 % arg1)
}
}
Expand All @@ -107,6 +117,9 @@ internal object FnModulo : DiadicArithmeticOperator("mod", false) {
return basic(PType.doublePrecision()) { args ->
val arg0 = args[0].double
val arg1 = args[1].double
if (arg1.isZero()) {
throw PErrors.divisionByZeroException(arg0, PType.doublePrecision())
}
Datum.doublePrecision(arg0 % arg1)
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/partiql-tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that should be done before merging this is that the revert should be approved/merged, and this submodule's commit should be on main.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that was the plan. Planning to get partiql/partiql-tests#132 merged in and then updating this PR.

Submodule partiql-tests updated 47 files
+134 −0 partiql-tests-data/eval/ion/primitives/cast.ion
+146 −0 partiql-tests-data/eval/ion/primitives/functions/abs.ion
+146 −0 partiql-tests-data/eval/ion/primitives/functions/exists.ion
+198 −0 partiql-tests-data/eval/ion/primitives/functions/extract.ion
+530 −0 partiql-tests-data/eval/ion/primitives/null.ion
+38 −0 partiql-tests-data/eval/ion/primitives/operators/concat.ion
+53 −0 partiql-tests-data/eval/ion/primitives/operators/nary-operators.ion
+20 −0 partiql-tests-data/eval/ion/primitives/path.ion
+21 −0 partiql-tests-data/eval/ion/primitives/symbol.ion
+177 −0 partiql-tests-data/eval/ion/query/group-by/group-by.ion
+102 −0 partiql-tests-data/eval/ion/query/order-by.ion
+15 −0 partiql-tests-data/eval/ion/query/select/from-clause.ion
+18 −0 partiql-tests-data/eval/ion/query/select/projection.ion
+2 −2 partiql-tests-data/eval/misc.ion
+0 −129 partiql-tests-data/eval/primitives/cast.ion
+0 −144 partiql-tests-data/eval/primitives/functions/abs.ion
+22 −34 partiql-tests-data/eval/primitives/functions/cardinality.ion
+14 −74 partiql-tests-data/eval/primitives/functions/exists.ion
+0 −196 partiql-tests-data/eval/primitives/functions/extract.ion
+415 −415 partiql-tests-data/eval/primitives/naughty-strings.ion
+0 −528 partiql-tests-data/eval/primitives/null.ion
+48 −72 partiql-tests-data/eval/primitives/operators/case-operator.ion
+0 −36 partiql-tests-data/eval/primitives/operators/concat.ion
+4 −4 partiql-tests-data/eval/primitives/operators/in-operator.ion
+89 −89 partiql-tests-data/eval/primitives/operators/like.ion
+18 −62 partiql-tests-data/eval/primitives/operators/nary-operators.ion
+2 −2 partiql-tests-data/eval/primitives/path.ion
+12 −12 partiql-tests-data/eval/primitives/string.ion
+1 −1 partiql-tests-data/eval/primitives/symbol.ion
+18 −22 partiql-tests-data/eval/query/group-by/group-by.ion
+8 −68 partiql-tests-data/eval/query/order-by.ion
+1 −1 partiql-tests-data/eval/query/pivot.ion
+5 −18 partiql-tests-data/eval/query/select/from-clause.ion
+1 −17 partiql-tests-data/eval/query/select/projection.ion
+2 −2 partiql-tests-data/eval/query/select/select-mysql.ion
+13 −13 partiql-tests-data/eval/query/select/select.ion
+4 −7 partiql-tests-data/eval/query/undefined-variable-behavior.ion
+2 −2 partiql-tests-data/fail/static-analysis/primitives/functions/exists.ion
+13 −13 partiql-tests-data/fail/static-analysis/primitives/functions/extract.ion
+2 −2 partiql-tests-data/fail/static-analysis/primitives/functions/substring.ion
+2 −2 partiql-tests-data/fail/static-analysis/primitives/operator/is-operator.ion
+3 −3 partiql-tests-data/fail/static-analysis/primitives/operator/like-operator.ion
+9 −0 partiql-tests-data/fail/syntax/ion/primitives/date-constructor.ion
+9 −0 partiql-tests-data/fail/syntax/ion/primitives/time-constructor.ion
+0 −8 partiql-tests-data/fail/syntax/primitives/date-constructor.ion
+1 −1 partiql-tests-data/fail/syntax/primitives/path-expression.ion
+0 −8 partiql-tests-data/fail/syntax/primitives/time-constructor.ion
Loading