Removes PartiQLValue and adds support for VARIANT (partiql#1678)

Removes PartiQLValue from public API Removes PartiQLValueType from public API Removes PartiQLValueExperimental from public API Removes PartiQLValue reader/writer from public API Adds full support for the VARIANT type Adds PType static factory methods with default parameters Adds AST-to-Plan typing logic of integers to the Ion Variant Adds a lowerSafe() internal utility method for reducing code size
johnedquinn · Dec 27, 2024 · 1abe677 · 1abe677
1 parent ea779ee
commit 1abe677
Show file tree

Hide file tree

Showing 104 changed files with 723 additions and 1,429 deletions.
diff --git a/docs/wiki/tutorials/Pluggable Functions Tutorial.md b/docs/wiki/tutorials/Pluggable Functions Tutorial.md
@@ -27,7 +27,7 @@ import org.partiql.types.PartiQLValueType
 import org.partiql.types.function.FunctionParameter
 import org.partiql.types.function.FunctionSignature
 import org.partiql.value.PartiQLValue
-import org.partiql.value.PartiQLValueExperimental
+
 import org.partiql.value.StringValue
 import org.partiql.value.stringValue
 
@@ -43,7 +43,6 @@ object TrimLead : PartiQLFunction {
         description = "Trims leading whitespace of a [str]." // A brief description of your function
     )
 
-    @OptIn(PartiQLValueExperimental::class)
     override operator fun invoke(session: ConnectorSession, arguments: List<PartiQLValue>): PartiQLValue {
         // Implement the function logic here
         val str = (arguments[0] as? StringValue)?.string ?: ""

diff --git a/partiql-cli/build.gradle.kts b/partiql-cli/build.gradle.kts
@@ -26,6 +26,7 @@ dependencies {
     implementation(project(":partiql-planner"))
     implementation(project(":partiql-types"))
     implementation(project(":partiql-spi"))
+    implementation(testFixtures(project(":partiql-spi")))
     implementation(Deps.csv)
     implementation(Deps.awsSdkBom)
     implementation(Deps.awsSdkDynamodb)

diff --git a/partiql-cli/src/main/kotlin/org/partiql/cli/Main.kt b/partiql-cli/src/main/kotlin/org/partiql/cli/Main.kt
@@ -24,8 +24,8 @@ 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.value.PartiQLValueExperimental
-import org.partiql.value.io.PartiQLValueTextWriter
+import org.partiql.spi.value.ValueUtils
+import org.partiql.spi.value.io.PartiQLValueTextWriter
 import picocli.CommandLine
 import java.io.File
 import java.io.InputStream
@@ -187,7 +187,6 @@ internal class MainCommand : Runnable {
         Shell(pipeline, session(), debug).start()
     }
 
-    @OptIn(PartiQLValueExperimental::class)
     private fun run(statement: String) {
         val config = getPipelineConfig()
         val pipeline = when (strict) {
@@ -206,7 +205,8 @@ internal class MainCommand : Runnable {
         // TODO add format support
         checkFormat(format)
         val writer = PartiQLValueTextWriter(System.out)
-        writer.append(result.toPartiQLValue()) // TODO: Create a Datum writer
+        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
@@ -31,8 +31,8 @@ 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.value.PartiQLValueExperimental
-import org.partiql.value.io.PartiQLValueTextWriter
+import org.partiql.spi.value.ValueUtils
+import org.partiql.spi.value.io.PartiQLValueTextWriter
 import java.io.Closeable
 import java.io.PrintStream
 import java.nio.file.Path
@@ -161,7 +161,6 @@ internal class Shell(
         }
     }
 
-    @OptIn(PartiQLValueExperimental::class)
     private fun run(exiting: AtomicBoolean) = TerminalBuilder.builder()
         .name("PartiQL")
         .nativeSignals(true)
@@ -276,7 +275,8 @@ internal class Shell(
                         out.appendLine()
                         out.info("=== RESULT ===")
                         val writer = PartiQLValueTextWriter(out)
-                        writer.append(result.toPartiQLValue()) // TODO: Create a Datum writer
+                        val p = ValueUtils.newPartiQLValue(result)
+                        writer.append(p) // TODO: Create a Datum writer
                         out.appendLine()
                         out.appendLine()
                         out.success("OK!")

diff --git a/partiql-eval/build.gradle.kts b/partiql-eval/build.gradle.kts
@@ -34,6 +34,7 @@ dependencies {
     // Test
     testImplementation(project(":partiql-parser"))
     testImplementation(testFixtures(project(":partiql-types"))) // TODO: Remove use of StaticType
+    testImplementation(testFixtures(project(":partiql-spi")))
     testImplementation(Deps.junit4)
     testImplementation(Deps.junit4Params)
     testImplementation(Deps.junitVintage) // Enables JUnit4

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
@@ -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. The planner/plan 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
@@ -3,13 +3,10 @@ package org.partiql.eval.internal.helpers
 import org.partiql.errors.TypeCheckException
 import org.partiql.spi.value.Datum
 import org.partiql.types.PType
-import org.partiql.value.PartiQLValue
-import org.partiql.value.PartiQLValueExperimental
-import org.partiql.value.PartiQLValueType
 import java.math.BigInteger
 
 /**
- * Holds helper functions for [PartiQLValue].
+ * Holds helper functions for [Datum].
  */
 internal object ValueUtility {
 
@@ -18,12 +15,14 @@ internal object ValueUtility {
      */
     @JvmStatic
     fun Datum.isTrue(): Boolean {
-        return this.type.code() == PType.BOOL && !this.isNull && !this.isMissing && this.boolean
-    }
-
-    @OptIn(PartiQLValueExperimental::class)
-    fun Datum.check(type: PartiQLValueType): Datum {
-        return this.check(type.toPType())
+        if (this.isNull || this.isMissing) {
+            return false
+        }
+        return when (this.type.code()) {
+            PType.VARIANT -> this.lower().isTrue()
+            PType.BOOL -> this.boolean
+            else -> false
+        }
     }
 
     /**
@@ -37,6 +36,9 @@ internal object ValueUtility {
         if (this.type == type) {
             return this
         }
+        if (this.type.code() == PType.VARIANT) {
+            return this.lower().check(type)
+        }
         if (!this.isNull) {
             throw TypeCheckException("Expected type $type but received ${this.type}.")
         }
@@ -51,29 +53,42 @@ internal object ValueUtility {
      */
     fun Datum.getText(): String {
         return when (this.type.code()) {
+            PType.VARIANT -> this.lower().getText()
             PType.STRING, PType.CHAR -> this.string
             else -> throw TypeCheckException("Expected text, but received ${this.type}.")
         }
     }
 
     /**
-     * Takes in a [Datum] that is any integer type ([PartiQLValueType.INT8], [PartiQLValueType.INT8],
-     * [PartiQLValueType.INT8], [PartiQLValueType.INT8], [PartiQLValueType.INT8]) and returns the [BigInteger] (potentially
-     * coerced) that represents the integer.
+     * Converts all number values to [BigInteger]. If the number is [PType.DECIMAL] or [PType.NUMERIC], this asserts that
+     * the scale is zero.
      *
      * INTERNAL NOTE: The PLANNER should be handling the coercion. This function should not be necessary.
      *
+     * TODO: This is used specifically for LIMIT and OFFSET. This makes the conformance tests pass by coercing values
+     *  of type [PType.NUMERIC] and [PType.DECIMAL], but this is unspecified. Do we allow for LIMIT 2.0? Or of
+     *  a value that is greater than [PType.BIGINT]'s MAX value by using a [PType.DECIMAL] with a high precision and scale
+     *  of zero? This hasn't been decided, however, as the conformance tests allow for this, this function coerces
+     *  the value to a [BigInteger] regardless of the number's type.
+     *
      * @throws NullPointerException if the value is null
      * @throws TypeCheckException if type is not an integer type
      */
     fun Datum.getBigIntCoerced(): BigInteger {
         return when (this.type.code()) {
+            PType.VARIANT -> this.lower().getBigIntCoerced()
             PType.TINYINT -> this.byte.toInt().toBigInteger()
             PType.SMALLINT -> this.short.toInt().toBigInteger()
             PType.INTEGER -> this.int.toBigInteger()
             PType.BIGINT -> this.long.toBigInteger()
-            PType.NUMERIC -> this.bigInteger
-            else -> throw TypeCheckException()
+            PType.NUMERIC, PType.DECIMAL -> {
+                val decimal = this.bigDecimal
+                if (decimal.scale() != 0) {
+                    throw TypeCheckException("Expected integer, but received decimal.")
+                }
+                return decimal.toBigInteger()
+            }
+            else -> throw TypeCheckException("Type: ${this.type}")
         }
     }
 
@@ -90,6 +105,7 @@ internal object ValueUtility {
      */
     fun Datum.getInt32Coerced(): Int {
         return when (this.type.code()) {
+            PType.VARIANT -> this.lower().getInt32Coerced()
             PType.TINYINT -> this.byte.toInt()
             PType.SMALLINT -> this.short.toInt()
             PType.INTEGER -> this.int

diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpExclude.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpExclude.kt
@@ -7,8 +7,6 @@ import org.partiql.plan.Exclusion
 import org.partiql.spi.value.Datum
 import org.partiql.spi.value.Field
 import org.partiql.types.PType
-import org.partiql.value.PartiQLValue
-import org.partiql.value.PartiQLValueType
 
 /**
  * Implementation of the EXCLUDE clause; there are good opportunities to tune/optimize this.
@@ -133,8 +131,8 @@ internal class RelOpExclude(
     }
 
     /**
-     * Returns a [PartiQLValue] created from an iterable of [coll]. Requires [type] to be a collection type
-     * (i.e. [PartiQLValueType.LIST] or [PartiQLValueType.BAG].).
+     * Returns a [Datum] created from an iterable of [coll]. Requires [type] to be a collection type
+     * (i.e. [PType.ARRAY] or [PType.BAG].).
      */
     private fun newCollValue(type: PType, coll: Iterable<Datum>): Datum {
         return when (type.code()) {

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
@@ -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,7 +17,7 @@ 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 = when (r.type.code()) {
             PType.BAG -> {

diff --git a/...iql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpIteratePermissive.kt b/...iql-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,7 +17,7 @@ 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 = when (r.type.code()) {
             PType.BAG -> {

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
@@ -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.eval.internal.helpers.RecordValueIterator
 import org.partiql.types.PType
 
@@ -15,7 +16,7 @@ internal class RelOpScan(
     private lateinit var records: Iterator<Row>
 
     override fun open(env: Environment) {
-        val r = expr.eval(env.push(Row()))
+        val r = expr.eval(env.push(Row())).lowerSafe()
         records = when (r.type.code()) {
             PType.ARRAY, PType.BAG -> RecordValueIterator(r.iterator())
             else -> {

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
@@ -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.eval.internal.helpers.RecordValueIterator
 import org.partiql.types.PType
 
@@ -14,7 +15,7 @@ internal class RelOpScanPermissive(
     private lateinit var records: Iterator<Row>
 
     override fun open(env: Environment) {
-        val r = expr.eval(env.push(Row()))
+        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
@@ -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,7 +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()))
+            val v = expr.eval(env.push(Row())).lowerSafe()
             if (v.type.code() != PType.STRUCT && v.type.code() != PType.ROW) {
                 throw TypeCheckException()
             }
@@ -78,7 +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()))
+            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
@@ -29,6 +29,7 @@ import org.partiql.types.PType.TIMESTAMPZ
 import org.partiql.types.PType.TIMEZ
 import org.partiql.types.PType.TINYINT
 import org.partiql.types.PType.VARCHAR
+import org.partiql.types.PType.VARIANT
 import org.partiql.value.datetime.DateTimeValue
 import java.math.BigDecimal
 import java.math.BigInteger
@@ -105,6 +106,7 @@ internal object CastTable {
         registerTimestamp()
         registerDate()
         registerTime()
+        registerVariant()
     }
 
     private fun String.pad(): String {
@@ -493,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
@@ -4,13 +4,13 @@ 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
 import org.partiql.spi.function.Function
 import org.partiql.spi.value.Datum
 import org.partiql.types.PType
-import org.partiql.value.PartiQLValue
 
 /**
  * Implementation of Dynamic Dispatch.
@@ -23,7 +23,7 @@ import org.partiql.value.PartiQLValue
  *  3. Lookup the candidate to dispatch to and invoke.
  *
  * This implementation can evaluate ([eval]) the input [Row], execute and gather the
- * arguments, and pass the [PartiQLValue]s directly to the [Candidate.eval].
+ * arguments, and pass the values directly to the [Candidate.eval].
  *
  * This implementation also caches previously resolved candidates.
  *
@@ -46,7 +46,7 @@ internal class ExprCallDynamic(
     private val candidates: MutableMap<List<PType>, Candidate> = mutableMapOf()
 
     override fun eval(env: Environment): Datum {
-        val actualArgs = args.map { it.eval(env) }.toTypedArray()
+        val actualArgs = args.map { it.eval(env).lowerSafe() }.toTypedArray()
         val actualTypes = actualArgs.map { it.type }
         var candidate = candidates[actualTypes]
         if (candidate == null) {