From 0f565af4e7623c179be11f0103dfd11e90acffc9 Mon Sep 17 00:00:00 2001 From: John Ed Quinn Date: Fri, 20 Dec 2024 08:25:04 -0800 Subject: [PATCH] Addresses PR comments Adds PType static factory methods with default parameters Renames factory methods for creating Datum and PartiQLValue Adds AST-to-Plan typing logic of integers to the Ion Variant Reverts the IonDatumReader changes to focus on limited scope Adds a lowerSafe() internal utility method for reducing code size Updates some Javadocs & comments --- .../src/main/kotlin/org/partiql/cli/Main.kt | 4 +- .../kotlin/org/partiql/cli/shell/Shell.kt | 4 +- .../eval/internal/helpers/DatumUtils.kt | 20 ++++ .../eval/internal/helpers/ValueUtility.kt | 7 +- .../internal/operator/rel/RelOpIterate.kt | 10 +- .../operator/rel/RelOpIteratePermissive.kt | 10 +- .../eval/internal/operator/rel/RelOpScan.kt | 15 +-- .../operator/rel/RelOpScanPermissive.kt | 12 +- .../internal/operator/rel/RelOpUnpivot.kt | 17 +-- .../eval/internal/operator/rex/CastTable.kt | 11 +- .../internal/operator/rex/ExprCallDynamic.kt | 10 +- .../operator/rex/ExprStructPermissive.kt | 4 +- .../partiql/eval/internal/SuccessTestCase.kt | 4 +- .../partiql/eval/internal/TypingTestCase.kt | 4 +- .../partiql/planner/internal/FnResolver.kt | 4 +- .../planner/internal/casts/CastTable.kt | 1 + .../java/org/partiql/spi/value/Datum.java | 2 +- .../function/builtins/internal/Accumulator.kt | 3 - .../partiql/spi/value/ion/IonDatumReader.kt | 85 +++++--------- .../org/partiql/spi/value/ion/IonVariant.kt | 15 ++- .../spi/value/PQLToPartiQLIterable.java | 2 +- .../partiql/spi/value/PQLToPartiQLStruct.java | 2 +- .../spi/value/PartiQLToPQLIterable.java | 2 +- .../partiql/spi/value/PartiQLToPQLStruct.java | 2 +- .../{DatumUtils.java => ValueUtils.java} | 11 +- partiql-types/api/partiql-types.api | 10 ++ .../main/java/org/partiql/types/PType.java | 106 ++++++++++++++++-- .../org/partiql/lang/randomized/eval/Utils.kt | 9 +- .../partiql/runner/executor/EvalExecutor.kt | 6 +- 29 files changed, 227 insertions(+), 165 deletions(-) create mode 100644 partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/DatumUtils.kt rename partiql-spi/src/testFixtures/java/org/partiql/spi/value/{DatumUtils.java => ValueUtils.java} (96%) diff --git a/partiql-cli/src/main/kotlin/org/partiql/cli/Main.kt b/partiql-cli/src/main/kotlin/org/partiql/cli/Main.kt index bd57d7f7f..cea88193d 100644 --- a/partiql-cli/src/main/kotlin/org/partiql/cli/Main.kt +++ b/partiql-cli/src/main/kotlin/org/partiql/cli/Main.kt @@ -24,7 +24,7 @@ import org.partiql.spi.catalog.Session import org.partiql.spi.catalog.Table import org.partiql.spi.value.Datum import org.partiql.spi.value.DatumReader -import org.partiql.spi.value.DatumUtils +import org.partiql.spi.value.ValueUtils import org.partiql.spi.value.io.PartiQLValueTextWriter import picocli.CommandLine import java.io.File @@ -205,7 +205,7 @@ internal class MainCommand : Runnable { // TODO add format support checkFormat(format) val writer = PartiQLValueTextWriter(System.out) - val p = DatumUtils.toPartiQLValue(result) + val p = ValueUtils.newPartiQLValue(result) writer.append(p) // TODO: Create a Datum writer println() } diff --git a/partiql-cli/src/main/kotlin/org/partiql/cli/shell/Shell.kt b/partiql-cli/src/main/kotlin/org/partiql/cli/shell/Shell.kt index d696d714a..69443dc68 100644 --- a/partiql-cli/src/main/kotlin/org/partiql/cli/shell/Shell.kt +++ b/partiql-cli/src/main/kotlin/org/partiql/cli/shell/Shell.kt @@ -31,7 +31,7 @@ import org.jline.utils.InfoCmp import org.joda.time.Duration import org.partiql.cli.pipeline.Pipeline import org.partiql.spi.catalog.Session -import org.partiql.spi.value.DatumUtils +import org.partiql.spi.value.ValueUtils import org.partiql.spi.value.io.PartiQLValueTextWriter import java.io.Closeable import java.io.PrintStream @@ -275,7 +275,7 @@ internal class Shell( out.appendLine() out.info("=== RESULT ===") val writer = PartiQLValueTextWriter(out) - val p = DatumUtils.toPartiQLValue(result) + val p = ValueUtils.newPartiQLValue(result) writer.append(p) // TODO: Create a Datum writer out.appendLine() out.appendLine() diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/DatumUtils.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/DatumUtils.kt new file mode 100644 index 000000000..3e3cf110d --- /dev/null +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/DatumUtils.kt @@ -0,0 +1,20 @@ +package org.partiql.eval.internal.helpers + +import org.partiql.spi.value.Datum +import org.partiql.types.PType + +internal object DatumUtils { + + /** + * Calls [Datum.lower] if the datum is a variant, otherwise returns the datum. If you don't know whether the value + * is of type [PType.VARIANT], you should use [Datum.lowerSafe] before invoking whatever methods you intend to use. + * This is essentially a workaround for the fact that we currently don't know whether a particular expression will be + * [PType.VARIANT] or not. This can eventually be optimized to accommodate this. + */ + internal fun Datum.lowerSafe(): Datum { + return when (this.type.code()) { + PType.VARIANT -> this.lower() + else -> this + } + } +} diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/ValueUtility.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/ValueUtility.kt index 48923c5e3..031392577 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/ValueUtility.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/helpers/ValueUtility.kt @@ -18,10 +18,11 @@ internal object ValueUtility { if (this.isNull || this.isMissing) { return false } - if (this.type.code() == PType.VARIANT) { - return this.lower().isTrue() + return when (this.type.code()) { + PType.VARIANT -> this.lower().isTrue() + PType.BOOL -> this.boolean + else -> false } - return this.type.code() == PType.BOOL && this.boolean } /** diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIterate.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIterate.kt index b99aecb3d..ad9b2ac37 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIterate.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIterate.kt @@ -5,6 +5,7 @@ import org.partiql.eval.Environment import org.partiql.eval.ExprRelation import org.partiql.eval.ExprValue import org.partiql.eval.Row +import org.partiql.eval.internal.helpers.DatumUtils.lowerSafe import org.partiql.spi.value.Datum import org.partiql.types.PType @@ -16,14 +17,9 @@ internal class RelOpIterate( private var index: Long = 0 override fun open(env: Environment) { - val r = expr.eval(env.push(Row())) + val r = expr.eval(env.push(Row())).lowerSafe() index = 0 - iterator = records(r) - } - - private fun records(r: Datum): Iterator { - return when (r.type.code()) { - PType.VARIANT -> records(r.lower()) + iterator = when (r.type.code()) { PType.BAG -> { close() throw TypeCheckException() diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIteratePermissive.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIteratePermissive.kt index 1f5b5d32b..fbc6a9435 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIteratePermissive.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIteratePermissive.kt @@ -4,6 +4,7 @@ import org.partiql.eval.Environment import org.partiql.eval.ExprRelation import org.partiql.eval.ExprValue import org.partiql.eval.Row +import org.partiql.eval.internal.helpers.DatumUtils.lowerSafe import org.partiql.spi.value.Datum import org.partiql.types.PType @@ -16,14 +17,9 @@ internal class RelOpIteratePermissive( private var isIndexable: Boolean = true override fun open(env: Environment) { - val r = expr.eval(env.push(Row())) + val r = expr.eval(env.push(Row())).lowerSafe() index = 0 - iterator = records(r) - } - - private fun records(r: Datum): Iterator { - return when (r.type.code()) { - PType.VARIANT -> records(r.lower()) + iterator = when (r.type.code()) { PType.BAG -> { isIndexable = false r.iterator() diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScan.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScan.kt index dce1faf77..d91e885a1 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScan.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScan.kt @@ -5,8 +5,8 @@ import org.partiql.eval.Environment import org.partiql.eval.ExprRelation import org.partiql.eval.ExprValue import org.partiql.eval.Row +import org.partiql.eval.internal.helpers.DatumUtils.lowerSafe import org.partiql.eval.internal.helpers.RecordValueIterator -import org.partiql.spi.value.Datum import org.partiql.types.PType internal class RelOpScan( @@ -16,17 +16,12 @@ internal class RelOpScan( private lateinit var records: Iterator override fun open(env: Environment) { - val r = expr.eval(env.push(Row())) - records = r.records() - } - - private fun Datum.records(): RecordValueIterator { - return when (this.type.code()) { - PType.VARIANT -> this.lower().records() - PType.ARRAY, PType.BAG -> RecordValueIterator(this.iterator()) + val r = expr.eval(env.push(Row())).lowerSafe() + records = when (r.type.code()) { + PType.ARRAY, PType.BAG -> RecordValueIterator(r.iterator()) else -> { close() - throw TypeCheckException("Unexpected type for scan: ${this.type}") + throw TypeCheckException("Unexpected type for scan: ${r.type}") } } } diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScanPermissive.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScanPermissive.kt index 5a0597369..126e64c66 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScanPermissive.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpScanPermissive.kt @@ -4,8 +4,8 @@ import org.partiql.eval.Environment import org.partiql.eval.ExprRelation import org.partiql.eval.ExprValue import org.partiql.eval.Row +import org.partiql.eval.internal.helpers.DatumUtils.lowerSafe import org.partiql.eval.internal.helpers.RecordValueIterator -import org.partiql.spi.value.Datum import org.partiql.types.PType internal class RelOpScanPermissive( @@ -15,14 +15,8 @@ internal class RelOpScanPermissive( private lateinit var records: Iterator override fun open(env: Environment) { - val r = expr.eval(env.push(Row())) - records = r.records() - } - - private fun Datum.records(): Iterator { - val r = this - return when (type.code()) { - PType.VARIANT -> r.lower().records() + val r = expr.eval(env.push(Row())).lowerSafe() + records = when (r.type.code()) { PType.BAG, PType.ARRAY -> RecordValueIterator(r.iterator()) else -> iterator { yield(Row(arrayOf(r))) } } diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpUnpivot.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpUnpivot.kt index 68cecaf48..1356a2ac4 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpUnpivot.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpUnpivot.kt @@ -5,6 +5,7 @@ import org.partiql.eval.Environment import org.partiql.eval.ExprRelation import org.partiql.eval.ExprValue import org.partiql.eval.Row +import org.partiql.eval.internal.helpers.DatumUtils.lowerSafe import org.partiql.spi.value.Datum import org.partiql.spi.value.Field import org.partiql.types.PType @@ -58,13 +59,7 @@ internal sealed class RelOpUnpivot : ExprRelation { class Strict(private val expr: ExprValue) : RelOpUnpivot() { override fun struct(): Datum { - val v = expr.eval(env.push(Row())).let { - if (it.type.code() == PType.VARIANT) { - it.lower() - } else { - it - } - } + val v = expr.eval(env.push(Row())).lowerSafe() if (v.type.code() != PType.STRUCT && v.type.code() != PType.ROW) { throw TypeCheckException() } @@ -84,13 +79,7 @@ internal sealed class RelOpUnpivot : ExprRelation { class Permissive(private val expr: ExprValue) : RelOpUnpivot() { override fun struct(): Datum { - val v = expr.eval(env.push(Row())).let { - if (it.type.code() == PType.VARIANT) { - it.lower() - } else { - it - } - } + val v = expr.eval(env.push(Row())).lowerSafe() if (v.isMissing) { return Datum.struct(emptyList()) } diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/CastTable.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/CastTable.kt index 76da4603a..8f994fa1c 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/CastTable.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/CastTable.kt @@ -71,9 +71,6 @@ internal object CastTable { if (target.code() == DYNAMIC) { return source } - if (source.type.code() == VARIANT) { - return cast(source.lower(), target) - } val cast = _table[source.type.code()][target.code()] ?: throw TypeCheckException("CAST(${source.type} AS $target) is not supported.") return try { @@ -109,6 +106,7 @@ internal object CastTable { registerTimestamp() registerDate() registerTime() + registerVariant() } private fun String.pad(): String { @@ -497,6 +495,13 @@ internal object CastTable { register(TIMEZ, TIMESTAMPZ) { x, _ -> Datum.timestamp(DateTimeValue.timestamp(DateTimeValue.date(1970, 1, 1), x.time)) } } + private fun registerVariant() { + PType.codes().forEach { pType -> + register(VARIANT, pType) { x, t -> cast(x.lower(), t) } + } + register(VARIANT, VARIANT) { x, _ -> x } + } + private fun register(source: Int, target: Int, cast: (Datum, PType) -> Datum) { _table[source][target] = cast } 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 236bf457b..7c799a5b7 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 @@ -4,6 +4,7 @@ import org.partiql.errors.TypeCheckException import org.partiql.eval.Environment import org.partiql.eval.ExprValue import org.partiql.eval.Row +import org.partiql.eval.internal.helpers.DatumUtils.lowerSafe import org.partiql.eval.internal.operator.rex.ExprCallDynamic.Candidate import org.partiql.eval.internal.operator.rex.ExprCallDynamic.CoercionFamily.DYNAMIC import org.partiql.eval.internal.operator.rex.ExprCallDynamic.CoercionFamily.UNKNOWN @@ -45,14 +46,7 @@ internal class ExprCallDynamic( private val candidates: MutableMap, Candidate> = mutableMapOf() override fun eval(env: Environment): Datum { - val actualArgs = args.map { - val arg = it.eval(env) - if (arg.type.code() == PType.VARIANT) { - arg.lower() - } else { - arg - } - }.toTypedArray() + val actualArgs = args.map { it.eval(env).lowerSafe() }.toTypedArray() val actualTypes = actualArgs.map { it.type } var candidate = candidates[actualTypes] if (candidate == null) { diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprStructPermissive.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprStructPermissive.kt index abe9711bf..e1b589d18 100644 --- a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprStructPermissive.kt +++ b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rex/ExprStructPermissive.kt @@ -2,6 +2,7 @@ package org.partiql.eval.internal.operator.rex import org.partiql.eval.Environment import org.partiql.eval.ExprValue +import org.partiql.eval.internal.helpers.DatumUtils.lowerSafe import org.partiql.spi.value.Datum import org.partiql.spi.value.Field import org.partiql.types.PType @@ -10,7 +11,7 @@ internal class ExprStructPermissive(private val fields: List) : ExprValue { override fun eval(env: Environment): Datum { val fields = fields.mapNotNull { - val key = it.key.eval(env) + val key = it.key.eval(env).lowerSafe() val keyString = key.getTextOrNull() ?: return@mapNotNull null val value = it.value.eval(env) when (value.isMissing) { @@ -32,7 +33,6 @@ internal class ExprStructPermissive(private val fields: List) : return null } return when (this.type.code()) { - PType.VARIANT -> this.lower().getTextOrNull() PType.STRING, PType.CHAR -> this.string else -> null } diff --git a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/SuccessTestCase.kt b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/SuccessTestCase.kt index fcfeb8986..7ca4245b0 100644 --- a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/SuccessTestCase.kt +++ b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/SuccessTestCase.kt @@ -12,7 +12,7 @@ import org.partiql.spi.catalog.Session import org.partiql.spi.catalog.Table import org.partiql.spi.value.Datum import org.partiql.spi.value.DatumReader -import org.partiql.spi.value.DatumUtils +import org.partiql.spi.value.ValueUtils import org.partiql.types.StaticType import org.partiql.types.fromStaticType import org.partiql.value.PartiQLValue @@ -31,7 +31,7 @@ public class SuccessTestCase( expected: PartiQLValue, mode: Mode = Mode.PERMISSIVE(), globals: List = emptyList(), - ) : this(input, DatumUtils.toDatum(expected), mode, globals) + ) : this(input, ValueUtils.newDatum(expected), mode, globals) private val compiler = PartiQLCompiler.standard() private val parser = PartiQLParser.standard() diff --git a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/TypingTestCase.kt b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/TypingTestCase.kt index 4810731e5..3a09d7342 100644 --- a/partiql-eval/src/test/kotlin/org/partiql/eval/internal/TypingTestCase.kt +++ b/partiql-eval/src/test/kotlin/org/partiql/eval/internal/TypingTestCase.kt @@ -9,7 +9,7 @@ import org.partiql.planner.PartiQLPlanner import org.partiql.spi.catalog.Catalog import org.partiql.spi.catalog.Session import org.partiql.spi.value.Datum -import org.partiql.spi.value.DatumUtils +import org.partiql.spi.value.ValueUtils import org.partiql.spi.value.io.PartiQLValueIonWriterBuilder import org.partiql.types.PType import org.partiql.value.PartiQLValue @@ -29,7 +29,7 @@ public class TypingTestCase( override fun run() { val (permissiveResult, plan) = run(mode = Mode.PERMISSIVE()) - val permissiveResultPValue = DatumUtils.toPartiQLValue(permissiveResult) + val permissiveResultPValue = ValueUtils.newPartiQLValue(permissiveResult) val assertionCondition = try { expectedPermissive == permissiveResultPValue // TODO: Assert using Datum } catch (t: Throwable) { diff --git a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/FnResolver.kt b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/FnResolver.kt index 5a9272901..044b2a539 100644 --- a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/FnResolver.kt +++ b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/FnResolver.kt @@ -27,8 +27,6 @@ internal object FnResolver { /** * Resolution of either a static or dynamic function. * - * TODO: How do we handle DYNAMIC? - * * @param variants * @param args * @return @@ -46,7 +44,7 @@ internal object FnResolver { } } - // 2. If there are DYNAMIC arguments, return all candidates + // 2. If there are DYNAMIC/VARIANT arguments, return all candidates val isDynamic = args.any { it.code() == PType.DYNAMIC || it.code() == PType.VARIANT } if (isDynamic) { val orderedMatches = candidates.sortedWith(FnComparator) diff --git a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/casts/CastTable.kt b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/casts/CastTable.kt index 4e50761cf..bc6a46a19 100644 --- a/partiql-planner/src/main/kotlin/org/partiql/planner/internal/casts/CastTable.kt +++ b/partiql-planner/src/main/kotlin/org/partiql/planner/internal/casts/CastTable.kt @@ -63,6 +63,7 @@ internal class CastTable private constructor( cast(it) } } + // A VARIANT can be cast to all other types. graph[PType.VARIANT] = relationships { PType.codes().map { cast(it) diff --git a/partiql-spi/src/main/java/org/partiql/spi/value/Datum.java b/partiql-spi/src/main/java/org/partiql/spi/value/Datum.java index d754ac85f..9db896e65 100644 --- a/partiql-spi/src/main/java/org/partiql/spi/value/Datum.java +++ b/partiql-spi/src/main/java/org/partiql/spi/value/Datum.java @@ -203,7 +203,7 @@ default short getShort() { * {@link #isNull()} returns false before attempting to invoke this method. */ default int getInt() { - throw new UnsupportedOperationException("Has type: " + getType() + " and class " + this.getClass().getName()); + throw new UnsupportedOperationException(); } /** diff --git a/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/internal/Accumulator.kt b/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/internal/Accumulator.kt index 70ced293f..9e3ca87d3 100644 --- a/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/internal/Accumulator.kt +++ b/partiql-spi/src/main/kotlin/org/partiql/spi/function/builtins/internal/Accumulator.kt @@ -45,9 +45,6 @@ internal fun comparisonAccumulator(comparator: Comparator): (Datum?, Datu } internal fun checkIsNumberType(funcName: String, value: Datum) { - if (value.type.code() == PType.VARIANT) { - return checkIsNumberType(funcName, value.lower()) - } if (!value.type.isNumber()) { throw TypeCheckException("Expected NUMBER but received ${value.type}.") } diff --git a/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonDatumReader.kt b/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonDatumReader.kt index 61f3e00d5..085be8d3e 100644 --- a/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonDatumReader.kt +++ b/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonDatumReader.kt @@ -10,7 +10,6 @@ import org.partiql.spi.value.Datum import org.partiql.spi.value.DatumReader import org.partiql.spi.value.Encoding import org.partiql.spi.value.Field -import org.partiql.types.PType import org.partiql.value.datetime.DateTimeUtil.toBigDecimal import java.io.IOException import java.io.InputStream @@ -67,64 +66,36 @@ internal class IonDatumReader internal constructor( /** * Read without any explicit PartiQL type information. */ - private fun read(): Datum { - if (reader.isNullValue) { - return Datum.nullValue(reader.type.toPTypeDefault()) - } - return when (reader.type) { - IonType.NULL -> Datum.nullValue() - IonType.BOOL -> bool() - IonType.INT -> bigint() - IonType.FLOAT -> double() - IonType.DECIMAL -> decimal0() - IonType.TIMESTAMP -> TODO("timestamp without annotation") - IonType.STRING -> varchar0() - IonType.CLOB -> clob0() - IonType.BLOB -> clob0() - IonType.LIST -> array() - IonType.STRUCT -> struct() - IonType.SYMBOL -> missing() - IonType.SEXP -> { - reader.stepIn() - if (reader.next() == null) { - throw IonDatumException("expected type, was null", null, span()) - } - val method = method() - if (reader.next() == null) { - throw IonDatumException("expected value, was null", null, span()) - } - val value = method() - if (reader.next() != null) { - throw IonDatumException("expected end of s-expression pair", null, span()) - } - reader.stepOut() - value + private fun read(): Datum = when (reader.type) { + IonType.NULL -> Datum.nullValue() + IonType.BOOL -> bool() + IonType.INT -> bigint() + IonType.FLOAT -> double() + IonType.DECIMAL -> decimal0() + IonType.TIMESTAMP -> TODO("timestamp without annotation") + IonType.STRING -> varchar0() + IonType.CLOB -> clob0() + IonType.BLOB -> clob0() + IonType.LIST -> array() + IonType.STRUCT -> struct() + IonType.SYMBOL -> missing() + IonType.SEXP -> { + reader.stepIn() + if (reader.next() == null) { + throw IonDatumException("expected type, was null", null, span()) } - else -> throw IonDatumException("unknown type", null, span()) - } - } - - /** - * Returns the default [PType] for the given [IonType]. - */ - private fun IonType.toPTypeDefault(): PType { - val code = when (this) { - IonType.NULL -> PType.UNKNOWN - IonType.BOOL -> PType.BOOL - IonType.INT -> PType.BIGINT - IonType.FLOAT -> PType.DOUBLE - IonType.DECIMAL -> PType.DECIMAL - IonType.TIMESTAMP -> PType.TIMESTAMP - IonType.STRING -> PType.STRING - IonType.CLOB -> PType.CLOB - IonType.BLOB -> PType.BLOB - IonType.LIST -> PType.ARRAY - IonType.STRUCT -> PType.STRUCT - IonType.SYMBOL -> PType.STRING - IonType.SEXP -> PType.ARRAY - else -> throw IonDatumException("Unknown type", null, span()) + val method = method() + if (reader.next() == null) { + throw IonDatumException("expected value, was null", null, span()) + } + val value = method() + if (reader.next() != null) { + throw IonDatumException("expected end of s-expression pair", null, span()) + } + reader.stepOut() + value } - return PType.of(code) + else -> throw IonDatumException("unknown type", null, span()) } /** diff --git a/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonVariant.kt b/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonVariant.kt index 6183e76f8..23ee66330 100644 --- a/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonVariant.kt +++ b/partiql-spi/src/main/kotlin/org/partiql/spi/value/ion/IonVariant.kt @@ -64,8 +64,14 @@ internal class IonVariant(private var value: AnyElement) : Datum { TIMESTAMP -> Datum.timestamp(DateTimeValue.timestamp(value.timestampValue)) INT -> { val bigInteger = value.bigIntegerValue - Datum.bigint(bigInteger.longValueExact()) - // Datum.decimal(value.bigIntegerValue.toBigDecimal(), 38, 0) + when { + bigInteger < Int.MAX_VALUE.toBigInteger() && bigInteger > Int.MIN_VALUE.toBigInteger() -> Datum.integer(bigInteger.toInt()) + bigInteger < Long.MAX_VALUE.toBigInteger() && bigInteger > Long.MIN_VALUE.toBigInteger() -> Datum.bigint(bigInteger.toLong()) + else -> { + val dec = bigInteger.toBigDecimal() + Datum.decimal(dec, dec.precision(), 0) + } + } } FLOAT -> Datum.doublePrecision(value.doubleValue) DECIMAL -> { @@ -79,6 +85,9 @@ internal class IonVariant(private var value: AnyElement) : Datum { } } + /** + * This returns the [PType] of a null Ion value, given its [ElementType]. + */ private fun ElementType.toPType(): PType { val code = when (this) { SYMBOL, STRING -> PType.STRING @@ -86,7 +95,7 @@ internal class IonVariant(private var value: AnyElement) : Datum { CLOB -> PType.CLOB BLOB -> PType.BLOB TIMESTAMP -> PType.TIMESTAMP - INT -> PType.DECIMAL + INT -> PType.INTEGER FLOAT -> PType.DOUBLE DECIMAL -> PType.DECIMAL LIST, SEXP -> PType.ARRAY diff --git a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLIterable.java b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLIterable.java index 9a7355cf8..d86aec9eb 100644 --- a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLIterable.java +++ b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLIterable.java @@ -25,7 +25,7 @@ public boolean hasNext() { @Override public PartiQLValue next() { - return DatumUtils.toPartiQLValue(iter.next()); + return ValueUtils.newPartiQLValue(iter.next()); } }; } diff --git a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLStruct.java b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLStruct.java index ea29f2d24..f4ad38340 100644 --- a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLStruct.java +++ b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PQLToPartiQLStruct.java @@ -27,7 +27,7 @@ public boolean hasNext() { @Override public Pair next() { Field field = _fields.next(); - return new Pair<>(field.getName(), DatumUtils.toPartiQLValue(field.getValue())); + return new Pair<>(field.getName(), ValueUtils.newPartiQLValue(field.getValue())); } }; } diff --git a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLIterable.java b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLIterable.java index d835e9b4d..a9f0fd586 100644 --- a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLIterable.java +++ b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLIterable.java @@ -31,7 +31,7 @@ public boolean hasNext() { @Override public Datum next() { PartiQLValue value = _value.next(); - return DatumUtils.toDatum(value); + return ValueUtils.newDatum(value); } } } diff --git a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLStruct.java b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLStruct.java index 06613f8c8..58cd79f5d 100644 --- a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLStruct.java +++ b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/PartiQLToPQLStruct.java @@ -42,7 +42,7 @@ public String getName() { @NotNull @Override public Datum getValue() { - return DatumUtils.toDatum(value.getSecond()); + return ValueUtils.newDatum(value.getSecond()); } }; } diff --git a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/DatumUtils.java b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/ValueUtils.java similarity index 96% rename from partiql-spi/src/testFixtures/java/org/partiql/spi/value/DatumUtils.java rename to partiql-spi/src/testFixtures/java/org/partiql/spi/value/ValueUtils.java index 0ab496959..32d40b7b3 100644 --- a/partiql-spi/src/testFixtures/java/org/partiql/spi/value/DatumUtils.java +++ b/partiql-spi/src/testFixtures/java/org/partiql/spi/value/ValueUtils.java @@ -35,14 +35,17 @@ import static org.partiql.types.PType.UNKNOWN; import static org.partiql.types.PType.VARIANT; -public class DatumUtils { +/** + * This internal class contains utility methods pertaining to {@link PartiQLValue} and {@link Datum}. + */ +public class ValueUtils { /** * Converts a {@link Datum} into a {@link PartiQLValue}. * @return the equivalent {@link PartiQLValue} */ @NotNull - public static PartiQLValue toPartiQLValue(@NotNull Datum datum) { + public static PartiQLValue newPartiQLValue(@NotNull Datum datum) { PType type = datum.getType(); if (datum.isMissing()) { return PartiQL.missingValue(); @@ -97,7 +100,7 @@ public static PartiQLValue toPartiQLValue(@NotNull Datum datum) { return PartiQL.missingValue(); } case VARIANT: - return datum.isNull() ? PartiQL.nullValue() : toPartiQLValue(datum.lower()); + return datum.isNull() ? PartiQL.nullValue() : newPartiQLValue(datum.lower()); default: throw new UnsupportedOperationException("Unsupported datum type: " + type); } @@ -109,7 +112,7 @@ public static PartiQLValue toPartiQLValue(@NotNull Datum datum) { * @return the equivalent {@link Datum} */ @NotNull - public static Datum toDatum(PartiQLValue value) { + public static Datum newDatum(PartiQLValue value) { PartiQLValueType type = value.getType(); if (value.isNull()) { return new DatumNull(type.toPType()); diff --git a/partiql-types/api/partiql-types.api b/partiql-types/api/partiql-types.api index cc285661a..1ef003b6d 100644 --- a/partiql-types/api/partiql-types.api +++ b/partiql-types/api/partiql-types.api @@ -45,12 +45,16 @@ public abstract class org/partiql/types/PType : org/partiql/types/Enum { public static fun bag ()Lorg/partiql/types/PType; public static fun bag (Lorg/partiql/types/PType;)Lorg/partiql/types/PType; public static fun bigint ()Lorg/partiql/types/PType; + public static fun blob ()Lorg/partiql/types/PType; public static fun blob (I)Lorg/partiql/types/PType; public static fun bool ()Lorg/partiql/types/PType; + public static fun character ()Lorg/partiql/types/PType; public static fun character (I)Lorg/partiql/types/PType; + public static fun clob ()Lorg/partiql/types/PType; public static fun clob (I)Lorg/partiql/types/PType; public static fun codes ()[I public static fun date ()Lorg/partiql/types/PType; + public static fun decimal ()Lorg/partiql/types/PType; public static fun decimal (II)Lorg/partiql/types/PType; public static fun doublePrecision ()Lorg/partiql/types/PType; public static fun dynamic ()Lorg/partiql/types/PType; @@ -61,6 +65,7 @@ public abstract class org/partiql/types/PType : org/partiql/types/Enum { public fun getTypeParameter ()Lorg/partiql/types/PType; public static fun integer ()Lorg/partiql/types/PType; public fun name ()Ljava/lang/String; + public static fun numeric ()Lorg/partiql/types/PType; public static fun numeric (II)Lorg/partiql/types/PType; public static fun of (I)Lorg/partiql/types/PType; public static fun real ()Lorg/partiql/types/PType; @@ -69,12 +74,17 @@ public abstract class org/partiql/types/PType : org/partiql/types/Enum { public static fun smallint ()Lorg/partiql/types/PType; public static fun string ()Lorg/partiql/types/PType; public static fun struct ()Lorg/partiql/types/PType; + public static fun time ()Lorg/partiql/types/PType; public static fun time (I)Lorg/partiql/types/PType; + public static fun timestamp ()Lorg/partiql/types/PType; public static fun timestamp (I)Lorg/partiql/types/PType; + public static fun timestampz ()Lorg/partiql/types/PType; public static fun timestampz (I)Lorg/partiql/types/PType; + public static fun timez ()Lorg/partiql/types/PType; public static fun timez (I)Lorg/partiql/types/PType; public static fun tinyint ()Lorg/partiql/types/PType; public static fun unknown ()Lorg/partiql/types/PType; + public static fun varchar ()Lorg/partiql/types/PType; public static fun varchar (I)Lorg/partiql/types/PType; public static fun variant (Ljava/lang/String;)Lorg/partiql/types/PType; } diff --git a/partiql-types/src/main/java/org/partiql/types/PType.java b/partiql-types/src/main/java/org/partiql/types/PType.java index e786441f4..89e351b4c 100644 --- a/partiql-types/src/main/java/org/partiql/types/PType.java +++ b/partiql-types/src/main/java/org/partiql/types/PType.java @@ -534,6 +534,14 @@ public static PType numeric(int precision, int scale) { return new PTypeDecimal(NUMERIC, precision, scale); } + /** + * @return a SQL:1999 NUMERIC type with default precision (38) and scale (0). + */ + @NotNull + public static PType numeric() { + return new PTypeDecimal(NUMERIC, 38, 0); + } + /** * @return a PartiQL decimal type */ @@ -542,6 +550,14 @@ public static PType decimal(int precision, int scale) { return new PTypeDecimal(PType.DECIMAL, precision, scale); } + /** + * @return a PartiQL decimal type with default precision (38) and scale (0). + */ + @NotNull + public static PType decimal() { + return new PTypeDecimal(PType.DECIMAL, 38, 0); + } + /** * @return a PartiQL real type. */ @@ -569,7 +585,16 @@ public static PType character(int length) { } /** - * @return a PartiQL char type + * @return a PartiQL char type with a default length of 1 + */ + @NotNull + @SuppressWarnings("unused") + public static PType character() { + return new PTypeWithMaxLength(CHAR, 1); + } + + /** + * @return a PartiQL varchar type */ @NotNull @SuppressWarnings("unused") @@ -577,6 +602,15 @@ public static PType varchar(int length) { return new PTypeWithMaxLength(VARCHAR, length); } + /** + * @return a PartiQL varchar type with a default length of 1 + */ + @NotNull + @SuppressWarnings("unused") + public static PType varchar() { + return new PTypeWithMaxLength(VARCHAR, 1); + } + /** * @return a PartiQL string type */ @@ -594,6 +628,14 @@ public static PType clob(int length) { return new PTypeWithMaxLength(CLOB, length); } + /** + * @return a PartiQL clob type with a default length of {@link Integer#MAX_VALUE}. + */ + @NotNull + public static PType clob() { + return new PTypeWithMaxLength(CLOB, Integer.MAX_VALUE); + } + /** * @return a PartiQL blob type */ @@ -603,6 +645,14 @@ public static PType blob(int length) { return new PTypeWithMaxLength(BLOB, length); } + /** + * @return a PartiQL blob type with a default length of {@link Integer#MAX_VALUE}. + */ + @NotNull + public static PType blob() { + return new PTypeWithMaxLength(BLOB, Integer.MAX_VALUE); + } + /** * @return a PartiQL date type */ @@ -619,6 +669,14 @@ public static PType time(int precision) { return new PTypeWithPrecisionOnly(TIME, precision); } + /** + * @return a PartiQL time without timezone type with a default precision of 6. + */ + @NotNull + public static PType time() { + return new PTypeWithPrecisionOnly(TIME, 6); + } + /** * @return a PartiQL time with timezone type */ @@ -628,6 +686,15 @@ public static PType timez(int precision) { return new PTypeWithPrecisionOnly(TIMEZ, precision); } + /** + * @return a PartiQL time with timezone type with a default precision of 6. + */ + @NotNull + @SuppressWarnings("unused") + public static PType timez() { + return new PTypeWithPrecisionOnly(TIMEZ, 6); + } + /** * @return a PartiQL timestamp without timezone type */ @@ -636,6 +703,14 @@ public static PType timestamp(int precision) { return new PTypeWithPrecisionOnly(TIMESTAMP, precision); } + /** + * @return a PartiQL timestamp without timezone type with a default precision of 6. + */ + @NotNull + public static PType timestamp() { + return new PTypeWithPrecisionOnly(TIMESTAMP, 6); + } + /** * @return a PartiQL timestamp with timezone type */ @@ -645,6 +720,15 @@ public static PType timestampz(int precision) { return new PTypeWithPrecisionOnly(TIMESTAMPZ, precision); } + /** + * @return a PartiQL timestamp with timezone type with a default precision of 6. + */ + @NotNull + @SuppressWarnings("unused") + public static PType timestampz() { + return new PTypeWithPrecisionOnly(TIMESTAMPZ, 6); + } + /** * @return a PartiQL list type with a component type of dynamic */ @@ -745,33 +829,33 @@ public static PType of(int code) throws IllegalArgumentException { case BIGINT: return bigint(); case NUMERIC: - return numeric(38, 0); + return numeric(); case DECIMAL: - return decimal(38, 0); + return decimal(); case REAL: return real(); case DOUBLE: return doublePrecision(); case CHAR: - return character(1); + return character(); case VARCHAR: - return varchar(1); + return varchar(); case STRING: return string(); case CLOB: - return clob(Integer.MAX_VALUE); + return clob(); case BLOB: - return blob(Integer.MAX_VALUE); + return blob(); case DATE: return date(); case TIME: - return time(6); + return time(); case TIMEZ: - return timez(6); + return timez(); case TIMESTAMP: - return timestamp(6); + return timestamp(); case TIMESTAMPZ: - return timestampz(6); + return timestampz(); case ARRAY: return array(); case BAG: diff --git a/test/partiql-randomized-tests/src/test/kotlin/org/partiql/lang/randomized/eval/Utils.kt b/test/partiql-randomized-tests/src/test/kotlin/org/partiql/lang/randomized/eval/Utils.kt index 4eda2b944..43241707d 100644 --- a/test/partiql-randomized-tests/src/test/kotlin/org/partiql/lang/randomized/eval/Utils.kt +++ b/test/partiql-randomized-tests/src/test/kotlin/org/partiql/lang/randomized/eval/Utils.kt @@ -6,8 +6,6 @@ import org.partiql.planner.PartiQLPlanner import org.partiql.spi.catalog.Catalog import org.partiql.spi.catalog.Session import org.partiql.spi.value.Datum -import org.partiql.spi.value.DatumUtils -import org.partiql.value.PartiQLValue import kotlin.test.assertEquals fun runEvaluatorTestCase( @@ -19,7 +17,7 @@ fun runEvaluatorTestCase( assertEquals(expected, result) } -private fun execute(query: String): PartiQLValue { +private fun execute(query: String): Datum { val parser = PartiQLParser.builder().build() val planner = PartiQLPlanner.builder().build() val catalog = object : Catalog { @@ -38,7 +36,8 @@ private fun execute(query: String): PartiQLValue { } fun assertExpression(query: String, value: () -> Datum) { - val expected = DatumUtils.toPartiQLValue(value.invoke()) // TODO: Make the PartiQL Engine return a Datum, not PartiQL Value + val expected = value.invoke() val result = execute(query) - assertEquals(expected, result) + val comparison = Datum.comparator().compare(expected, result) + assert(comparison == 0) { "Expected $expected, got $result" } } 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 768ca7298..a70464bca 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 @@ -26,7 +26,7 @@ import org.partiql.spi.errors.PErrorException import org.partiql.spi.errors.PErrorListener import org.partiql.spi.errors.Severity import org.partiql.spi.value.Datum -import org.partiql.spi.value.DatumUtils +import org.partiql.spi.value.ValueUtils import org.partiql.types.PType import org.partiql.value.PartiQLValue import org.partiql.value.io.DatumIonReaderBuilder @@ -93,13 +93,13 @@ class EvalExecutor( } override fun toIon(value: Datum): IonValue { - val partiql = DatumUtils.toPartiQLValue(value) + val partiql = ValueUtils.newPartiQLValue(value) return partiql.toIon().toIonValue(ION) } // TODO: Use DATUM override fun compare(actual: Datum, expect: Datum): Boolean { - return valueComparison(DatumUtils.toPartiQLValue(actual), DatumUtils.toPartiQLValue(expect)) + return valueComparison(ValueUtils.newPartiQLValue(actual), ValueUtils.newPartiQLValue(expect)) } // Value comparison of PartiQL Value that utilized Ion Hashcode.