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

Misc cleanup of errors and docs #1720

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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 @@ -10,6 +10,11 @@ import org.junit.jupiter.params.provider.MethodSource
*/
class DataExceptionTest {

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

@ParameterizedTest
@MethodSource("plusOverflowTests")
@Execution(ExecutionMode.CONCURRENT)
Expand All @@ -34,6 +39,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 All @@ -43,6 +52,14 @@ class DataExceptionTest {
fun negOverflow(tc: FailureTestCase) = tc.run()

companion object {
@JvmStatic
fun sandboxT() = listOf(
// TINYINT
FailureTestCase(
input = "SELECT x + 1 FROM << 1, 2e0, MISSING>> AS x;"
)
)

@JvmStatic
fun plusOverflowTests() = listOf(
// TINYINT
Expand Down Expand Up @@ -184,6 +201,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
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