From 923c8a0b939163b285d206ac51b9e17733e450d8 Mon Sep 17 00:00:00 2001 From: John Ed Quinn Date: Wed, 24 Jul 2024 16:04:16 -0700 Subject: [PATCH] Fixes function invocation for missing arguments Fixes evaluation of Rex.Op.Missing --- .../java/org/partiql/eval/value/Datum.java | 3 +++ .../org/partiql/eval/internal/Compiler.kt | 12 +----------- .../internal/operator/rex/ExprCallDynamic.kt | 18 +++++++++++------- .../internal/operator/rex/ExprCallStatic.kt | 8 ++++++-- .../eval/internal/operator/rex/ExprMissing.kt | 6 +++--- .../eval/internal/PartiQLEngineDefaultTest.kt | 19 ++++++++++++++----- .../partiql/runner/executor/EvalExecutor.kt | 3 +++ 7 files changed, 41 insertions(+), 28 deletions(-) diff --git a/partiql-eval/src/main/java/org/partiql/eval/value/Datum.java b/partiql-eval/src/main/java/org/partiql/eval/value/Datum.java index 2b068b5f4e..57af2b9299 100644 --- a/partiql-eval/src/main/java/org/partiql/eval/value/Datum.java +++ b/partiql-eval/src/main/java/org/partiql/eval/value/Datum.java @@ -345,6 +345,9 @@ default Datum getInsensitive(@NotNull String name) { @Deprecated default PartiQLValue toPartiQLValue() { PType type = this.getType(); + if (this.isMissing()) { + return PartiQL.missingValue(); + } switch (type.getKind()) { case BOOL: return this.isNull() ? PartiQL.boolValue(null) : PartiQL.boolValue(this.getBoolean()); diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/Compiler.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/Compiler.kt index a2245c1dce..2460f2d2aa 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/Compiler.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/Compiler.kt @@ -55,7 +55,6 @@ import org.partiql.plan.Rel import org.partiql.plan.Rex import org.partiql.plan.Statement import org.partiql.plan.debug.PlanPrinter -import org.partiql.plan.rexOpErr import org.partiql.plan.visitor.PlanBaseVisitor import org.partiql.spi.fn.Agg import org.partiql.spi.fn.FnExperimental @@ -267,16 +266,7 @@ internal class Compiler( } override fun visitRexOpMissing(node: Rex.Op.Missing, ctx: PType?): Operator { - return when (session.mode) { - PartiQLEngine.Mode.PERMISSIVE -> { - // Make a runtime TypeCheckException. - ExprMissing(node.message) - } - PartiQLEngine.Mode.STRICT -> { - // Promote to error. - visitRexOpErr(rexOpErr(node.message, node.causes), null) - } - } + return ExprMissing(ctx ?: PType.typeUnknown()) // TODO: Pass a type } // REL diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallDynamic.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallDynamic.kt index 337f28224a..f95a73073e 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallDynamic.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallDynamic.kt @@ -34,7 +34,7 @@ internal class ExprCallDynamic( val actualTypes = actualArgs.map { it.type } candidateIndex.get(actualTypes)?.let { val transformedArgs = Array(actualArgs.size) { - actualArgs[it].toPartiQLValue() + actualArgs[it] } return it.eval(transformedArgs, env) } @@ -63,15 +63,19 @@ internal class ExprCallDynamic( */ private val nil = { Datum.nullValue(fn.signature.returns) } - fun eval(originalArgs: Array, env: Environment): Datum { + /** + * Memoize creation of missing values + */ + private val missing = { Datum.missingValue(fn.signature.returns) } + + fun eval(originalArgs: Array, env: Environment): Datum { val args = originalArgs.mapIndexed { i, arg -> - if (arg.isNull && fn.signature.isNullCall) { - return nil.invoke() - } + if (arg.isNull && fn.signature.isNullCall) return nil.invoke() + if (arg.isMissing && fn.signature.isMissingCall) return missing.invoke() when (val c = coercions[i]) { null -> arg - else -> ExprCast(ExprLiteral(Datum.of(arg)), c).eval(env).toPartiQLValue() - } + else -> ExprCast(ExprLiteral(arg), c).eval(env) + }.toPartiQLValue() }.toTypedArray() return Datum.of(fn.invoke(args)) } diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallStatic.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallStatic.kt index 6d7e66d545..cdf30858cd 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallStatic.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprCallStatic.kt @@ -1,6 +1,5 @@ package org.partiql.eval.internal.operator.rex -import org.partiql.errors.TypeCheckException import org.partiql.eval.internal.Environment import org.partiql.eval.internal.operator.Operator import org.partiql.eval.value.Datum @@ -19,12 +18,17 @@ internal class ExprCallStatic( */ private val nil = { Datum.nullValue(fn.signature.returns) } + /** + * Memoize creation of missing values + */ + private val missing = { Datum.missingValue(fn.signature.returns) } + override fun eval(env: Environment): Datum { // Evaluate arguments val args = inputs.map { input -> val r = input.eval(env) if (r.isNull && fn.signature.isNullCall) return nil.invoke() - if (r.isMissing && fn.signature.isMissingCall) throw TypeCheckException() + if (r.isMissing && fn.signature.isMissingCall) return missing.invoke() r.toPartiQLValue() }.toTypedArray() return Datum.of(fn.invoke(args)) diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprMissing.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprMissing.kt index 6133fba611..ad55f9e5dd 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprMissing.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprMissing.kt @@ -1,15 +1,15 @@ package org.partiql.eval.internal.operator.rex -import org.partiql.errors.TypeCheckException import org.partiql.eval.internal.Environment import org.partiql.eval.internal.operator.Operator import org.partiql.eval.value.Datum +import org.partiql.types.PType internal class ExprMissing( - private val message: String, + private val type: PType ) : Operator.Expr { override fun eval(env: Environment): Datum { - throw TypeCheckException(message) + return Datum.missingValue(type) } } diff --git a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/PartiQLEngineDefaultTest.kt b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/PartiQLEngineDefaultTest.kt index 13a9e34156..9827ec0cb3 100644 --- a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/PartiQLEngineDefaultTest.kt +++ b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/PartiQLEngineDefaultTest.kt @@ -1073,6 +1073,20 @@ class PartiQLEngineDefaultTest { structValue("a" to int32Value(1), "b" to int32Value(2)) ) ), + // PartiQL Specification Section 7.1 -- Inputs with wrong types Example 28 (1) + // According to the Specification, in permissive mode, functions/operators return missing when one of + // the parameters is missing. + SuccessTestCase( + input = "SELECT VALUE 5 + v FROM <<1, MISSING>> AS v;", + expected = bagValue(int32Value(6), missingValue()) + ), + // PartiQL Specification Section 7.1 -- Inputs with wrong types Example 28 (1) + // See https://github.com/partiql/partiql-tests/pull/118 for more information. + SuccessTestCase( + input = "SELECT VALUE 5 + v FROM <<1, MISSING>> AS v;", + expected = bagValue(int32Value(6), missingValue()), + mode = PartiQLEngine.Mode.STRICT + ), ) @JvmStatic @@ -1194,11 +1208,6 @@ class PartiQLEngineDefaultTest { "amzn" to decimalValue(BigDecimal.valueOf(840.05)) ) ), - TypingTestCase( - name = "PartiQL Specification Section 7.1 -- Inputs with wrong types Example 28 (1)", - input = "SELECT VALUE 5 + v FROM <<1, MISSING>> AS v;", - expectedPermissive = bagValue(int32Value(6), missingValue()) - ), TypingTestCase( name = "PartiQL Specification Section 7.1 -- Inputs with wrong types Example 28 (3)", input = "SELECT VALUE NOT v FROM << false, {'a':1} >> AS v;", diff --git a/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/executor/EvalExecutor.kt b/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/executor/EvalExecutor.kt index eab8096dab..c12aa11493 100644 --- a/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/executor/EvalExecutor.kt +++ b/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/executor/EvalExecutor.kt @@ -65,6 +65,9 @@ class EvalExecutor( if (actual is PartiQLResult.Value && expect is PartiQLResult.Value) { return valueComparison(actual.value, expect.value) } + if (actual is PartiQLResult.Error) { + throw actual.cause + } val errorMessage = buildString { appendLine("Cannot compare different types of PartiQLResult.") appendLine(" - Expected : $expect")