Enable Eval Test Suites (#1340)

partiql · Jan 25, 2024 · 23961f3 · 23961f3
1 parent 57496a9
commit 23961f3
Show file tree

Hide file tree

Showing 10 changed files with 107 additions and 48 deletions.
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/PartiQLEngine.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/PartiQLEngine.kt
@@ -1,6 +1,9 @@
 package org.partiql.eval
 
 import org.partiql.plan.PartiQLPlan
+import org.partiql.spi.connector.ConnectorBindings
+import org.partiql.spi.function.PartiQLFunction
+import org.partiql.spi.function.PartiQLFunctionExperimental
 
 /**
  * PartiQL's Experimental Engine.
@@ -19,8 +22,9 @@ import org.partiql.plan.PartiQLPlan
  */
 public interface PartiQLEngine {
 
-    public fun prepare(plan: PartiQLPlan): PartiQLStatement<*>
+    public fun prepare(plan: PartiQLPlan, session: Session): PartiQLStatement<*>
 
+    // TODO: Pass session variable during statement execution once we finalize data structure for session.
     public fun execute(statement: PartiQLStatement<*>): PartiQLResult
 
     companion object {
@@ -31,4 +35,9 @@ public interface PartiQLEngine {
         @JvmStatic
         fun default() = PartiQLEngineBuilder().build()
     }
+
+    public class Session @OptIn(PartiQLFunctionExperimental::class) constructor(
+        val bindings: Map<String, ConnectorBindings> = mapOf(),
+        val functions: Map<String, List<PartiQLFunction>> = mapOf()
+    )
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/PartiQLEngineBuilder.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/PartiQLEngineBuilder.kt
@@ -1,36 +1,11 @@
 package org.partiql.eval
 
-import org.partiql.spi.connector.ConnectorBindings
-
 class PartiQLEngineBuilder {
 
-    private var catalogs: MutableMap<String, ConnectorBindings> = mutableMapOf()
-
     /**
      * Build the builder, return an implementation of a [PartiQLEngine]
      *
      * @return
      */
-    public fun build(): PartiQLEngine = PartiQLEngineDefault(catalogs)
-
-    /**
-     * Java style method for assigning a Catalog name to [ConnectorBindings].
-     *
-     * @param catalog
-     * @param bindings
-     * @return
-     */
-    public fun addCatalog(catalog: String, bindings: ConnectorBindings): PartiQLEngineBuilder = this.apply {
-        this.catalogs[catalog] = bindings
-    }
-
-    /**
-     * Kotlin style method for assigning Catalog names to [ConnectorBindings].
-     *
-     * @param catalogs
-     * @return
-     */
-    public fun catalogs(vararg catalogs: Pair<String, ConnectorBindings>): PartiQLEngineBuilder = this.apply {
-        this.catalogs = mutableMapOf(*catalogs)
-    }
+    public fun build(): PartiQLEngine = PartiQLEngineDefault()
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/PartiQLEngineDefault.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/PartiQLEngineDefault.kt
@@ -3,18 +3,15 @@ package org.partiql.eval
 import org.partiql.eval.internal.Compiler
 import org.partiql.eval.internal.Record
 import org.partiql.plan.PartiQLPlan
-import org.partiql.spi.connector.ConnectorBindings
 import org.partiql.value.PartiQLValue
 import org.partiql.value.PartiQLValueExperimental
 
-internal class PartiQLEngineDefault(
-    private val catalogs: Map<String, ConnectorBindings>,
-) : PartiQLEngine {
+internal class PartiQLEngineDefault : PartiQLEngine {
 
     @OptIn(PartiQLValueExperimental::class)
-    override fun prepare(plan: PartiQLPlan): PartiQLStatement<*> {
+    override fun prepare(plan: PartiQLPlan, session: PartiQLEngine.Session): PartiQLStatement<*> {
         try {
-            val compiler = Compiler(plan, catalogs)
+            val compiler = Compiler(plan, session)
             val expression = compiler.compile()
             return object : PartiQLStatement.Query {
                 override fun execute(): PartiQLValue {

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
@@ -1,5 +1,6 @@
 package org.partiql.eval.internal
 
+import org.partiql.eval.PartiQLEngine
 import org.partiql.eval.internal.operator.Operator
 import org.partiql.eval.internal.operator.rel.RelDistinct
 import org.partiql.eval.internal.operator.rel.RelFilter
@@ -10,6 +11,7 @@ import org.partiql.eval.internal.operator.rel.RelJoinRight
 import org.partiql.eval.internal.operator.rel.RelProject
 import org.partiql.eval.internal.operator.rel.RelScan
 import org.partiql.eval.internal.operator.rel.RelScanIndexed
+import org.partiql.eval.internal.operator.rex.ExprCall
 import org.partiql.eval.internal.operator.rex.ExprCase
 import org.partiql.eval.internal.operator.rex.ExprCollection
 import org.partiql.eval.internal.operator.rex.ExprGlobal
@@ -27,15 +29,17 @@ import org.partiql.plan.PlanNode
 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.visitor.PlanBaseVisitor
-import org.partiql.spi.connector.ConnectorBindings
 import org.partiql.spi.connector.ConnectorObjectPath
+import org.partiql.spi.function.PartiQLFunction
+import org.partiql.spi.function.PartiQLFunctionExperimental
 import org.partiql.value.PartiQLValueExperimental
 import java.lang.IllegalStateException
 
 internal class Compiler(
     private val plan: PartiQLPlan,
-    private val catalogs: Map<String, ConnectorBindings>,
+    private val session: PartiQLEngine.Session
 ) : PlanBaseVisitor<Operator, Unit>() {
 
     fun compile(): Operator.Expr {
@@ -47,7 +51,11 @@ internal class Compiler(
     }
 
     override fun visitRexOpErr(node: Rex.Op.Err, ctx: Unit): Operator {
-        throw IllegalStateException(node.message)
+        val message = buildString {
+            this.appendLine(node.message)
+            PlanPrinter.append(this, plan)
+        }
+        throw IllegalStateException(message)
     }
 
     override fun visitRelOpErr(node: Rel.Op.Err, ctx: Unit): Operator {
@@ -101,7 +109,7 @@ internal class Compiler(
         val catalog = plan.catalogs[node.ref.catalog]
         val symbol = catalog.symbols[node.ref.symbol]
         val path = ConnectorObjectPath(symbol.path)
-        val bindings = catalogs[catalog.name]!!
+        val bindings = session.bindings[catalog.name]!!
         return ExprGlobal(path, bindings)
     }
 
@@ -123,8 +131,26 @@ internal class Compiler(
         return ExprPathIndex(root, index)
     }
 
-    // REL
+    @OptIn(PartiQLFunctionExperimental::class)
+    override fun visitRexOpCallStatic(node: Rex.Op.Call.Static, ctx: Unit): Operator {
+        val fn = node.fn.signature
+        val args = node.args.map { visitRex(it, ctx) }
+        val matches = session.functions
+            .flatMap { it.value }
+            .filterIsInstance<PartiQLFunction.Scalar>()
+            .filter { it.signature == fn }
+        return when (matches.size) {
+            0 -> error("no match")
+            1 -> ExprCall(matches.first(), args.toTypedArray())
+            else -> error("multiple math")
+        }
+    }
+
+    override fun visitRexOpCallDynamic(node: Rex.Op.Call.Dynamic, ctx: Unit): Operator {
+        error("call dynamic not yet implemented")
+    }
 
+    // REL
     override fun visitRel(node: Rel, ctx: Unit): Operator.Relation {
         return super.visitRelOp(node.op, ctx) as Operator.Relation
     }

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
@@ -242,7 +242,7 @@ class PartiQLEngineDefaultTest {
             val statement = parser.parse(input).root
             val session = PartiQLPlanner.Session("q", "u")
             val plan = planner.plan(statement, session)
-            val prepared = engine.prepare(plan.plan)
+            val prepared = engine.prepare(plan.plan, PartiQLEngine.Session())
             val result = engine.execute(prepared) as PartiQLResult.Value
             val output = result.value
             assertEquals(expected, output, comparisonString(expected, output))

diff --git a/test/partiql-tests-runner/build.gradle.kts b/test/partiql-tests-runner/build.gradle.kts
@@ -28,6 +28,7 @@ dependencies {
     testImplementation(project(":partiql-lang"))
     testImplementation(project(":partiql-eval"))
     testImplementation(project(":plugins:partiql-memory"))
+    testImplementation(project(":plugins:partiql-plugin"))
 }
 
 val tests = System.getenv()["PARTIQL_TESTS_DATA"] ?: "../partiql-tests/partiql-tests-data"

diff --git a/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/ConformanceTestBase.kt b/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/ConformanceTestBase.kt
@@ -1,5 +1,6 @@
 package org.partiql.runner
 
+import org.junit.jupiter.api.Timeout
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.ArgumentsSource
 import org.partiql.runner.schema.TestCase
@@ -10,6 +11,11 @@ abstract class ConformanceTestBase<T, V> {
     abstract val runner: TestRunner<T, V>
 
     // Tests the eval tests with the Kotlin implementation
+    // Unit is second.
+    // This is not a performance test. This is for stop long-running tests during development process in eval engine.
+    // This number can be smaller, but to account for the cold start time and fluctuation of GitHub runner,
+    // I decided to make this number a bit larger than needed.
+    @Timeout(value = 100, threadMode = Timeout.ThreadMode.SEPARATE_THREAD)
     @ParameterizedTest(name = "{arguments}")
     @ArgumentsSource(TestProvider.Eval::class)
     fun validatePartiQLEvalTestData(tc: TestCase) {
@@ -20,6 +26,7 @@ abstract class ConformanceTestBase<T, V> {
     }
 
     // Tests the eval equivalence tests with the Kotlin implementation
+    @Timeout(value = 100, threadMode = Timeout.ThreadMode.SEPARATE_THREAD)
     @ParameterizedTest(name = "{arguments}")
     @ArgumentsSource(TestProvider.Equiv::class)
     fun validatePartiQLEvalEquivTestData(tc: TestCase) {

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
@@ -2,6 +2,7 @@ package org.partiql.runner.executor
 
 import com.amazon.ion.IonStruct
 import com.amazon.ion.IonValue
+import com.amazon.ionelement.api.ElementType
 import com.amazon.ionelement.api.IonElement
 import com.amazon.ionelement.api.StructElement
 import com.amazon.ionelement.api.toIonElement
@@ -12,12 +13,14 @@ import org.partiql.eval.PartiQLStatement
 import org.partiql.lang.eval.CompileOptions
 import org.partiql.parser.PartiQLParser
 import org.partiql.planner.PartiQLPlanner
+import org.partiql.plugin.PartiQLPlugin
 import org.partiql.plugins.memory.MemoryBindings
 import org.partiql.plugins.memory.MemoryConnector
 import org.partiql.runner.ION
 import org.partiql.runner.test.TestExecutor
 import org.partiql.spi.connector.Connector
 import org.partiql.spi.connector.ConnectorSession
+import org.partiql.spi.function.PartiQLFunctionExperimental
 import org.partiql.types.StaticType
 import org.partiql.value.PartiQLValue
 import org.partiql.value.PartiQLValueExperimental
@@ -26,13 +29,14 @@ import org.partiql.value.toIon
 
 @OptIn(PartiQLValueExperimental::class)
 class EvalExecutor(
-    private val session: PartiQLPlanner.Session,
+    private val plannerSession: PartiQLPlanner.Session,
+    private val evalSession: PartiQLEngine.Session
 ) : TestExecutor<PartiQLStatement<*>, PartiQLResult> {
 
     override fun prepare(statement: String): PartiQLStatement<*> {
         val stmt = parser.parse(statement).root
-        val plan = planner.plan(stmt, session)
-        return engine.prepare(plan.plan)
+        val plan = planner.plan(stmt, plannerSession)
+        return engine.prepare(plan.plan, evalSession)
     }
 
     override fun execute(statement: PartiQLStatement<*>): PartiQLResult {
@@ -54,11 +58,41 @@ class EvalExecutor(
 
     override fun compare(actual: PartiQLResult, expect: PartiQLResult): Boolean {
         if (actual is PartiQLResult.Value && expect is PartiQLResult.Value) {
-            return actual.value == expect.value
+            return valueComparison(actual.value, expect.value)
         }
         error("Cannot compare different types of PartiQLResult")
     }
 
+    // Value comparison of PartiQL Value that utilized Ion Hashcode.
+    // in here, null.bool is considered equivalent to null
+    // missing is considered different from null
+    // annotation::1 is considered different from 1
+    // 1 of type INT is considered the same as 1 of type INT32
+    // we should probably consider adding our own hashcode implementation
+    private fun valueComparison(v1: PartiQLValue, v2: PartiQLValue): Boolean {
+        // Additional check to put on annotation
+        // we want to have
+        // annotation::null.int == annotation::null.bool  <- True
+        // annotation::null.int == other::null.int <- False
+        if (v1.annotations != v2.annotations) {
+            return false
+        }
+        if (v1.isNull && v2.isNull) {
+            return true
+        }
+        if (v1 == v2) {
+            return true
+        }
+        if (v1.toIon().hashCode() == v2.toIon().hashCode()) {
+            return true
+        }
+        // Ion element hash code contains a bug
+        // Hashcode of BigIntIntElementImpl(BigInteger.ONE) is not the same as that of LongIntElementImpl(1)
+        if (v1.toIon().type == ElementType.INT && v2.toIon().type == ElementType.INT) {
+            return v1.toIon().asAnyElement().bigIntegerValue == v2.toIon().asAnyElement().bigIntegerValue
+        }
+        return false
+    }
     companion object {
         val parser = PartiQLParser.default()
         val planner = PartiQLPlanner.default()
@@ -67,6 +101,7 @@ class EvalExecutor(
 
     object Factory : TestExecutor.Factory<PartiQLStatement<*>, PartiQLResult> {
 
+        @OptIn(PartiQLFunctionExperimental::class)
         override fun create(env: IonStruct, options: CompileOptions): TestExecutor<PartiQLStatement<*>, PartiQLResult> {
             val catalog = "default"
             val data = env.toIonElement() as StructElement
@@ -79,13 +114,22 @@ class EvalExecutor(
                 userId = "user",
                 currentCatalog = catalog,
                 catalogs = mapOf(
-                    "test" to connector.getMetadata(object : ConnectorSession {
+                    "default" to connector.getMetadata(object : ConnectorSession {
                         override fun getQueryId(): String = "query"
                         override fun getUserId(): String = "user"
                     })
                 )
             )
-            return EvalExecutor(session)
+
+            val evalSession = PartiQLEngine.Session(
+                bindings = mutableMapOf(
+                    "default" to connector.getBindings()
+                ),
+                functions = mutableMapOf(
+                    "partiql" to PartiQLPlugin.functions
+                )
+            )
+            return EvalExecutor(session, evalSession)
         }
 
         /**

diff --git a/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/report/ReportGenerator.kt b/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/report/ReportGenerator.kt
@@ -24,7 +24,7 @@ class ReportGenerator(val engine: String) : TestWatcher, AfterAllCallback {
     }
 
     override fun afterAll(p0: ExtensionContext?) {
-        val basePath = System.getenv("conformanceReportDir")
+        val basePath = System.getenv("conformanceReportDir") ?: "."
         val dir = Files.createDirectory(Path("$basePath/$engine")).toFile()
         val file = File(dir, "conformance_test_results.ion")
         val outputStream = file.outputStream()

diff --git a/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/test/TestProvider.kt b/test/partiql-tests-runner/src/test/kotlin/org/partiql/runner/test/TestProvider.kt
@@ -5,8 +5,8 @@ import org.junit.jupiter.params.provider.Arguments
 import org.junit.jupiter.params.provider.ArgumentsProvider
 import java.util.stream.Stream
 
-private val PARTIQL_EVAL_TEST_DATA_DIR = System.getenv("PARTIQL_EVAL_TESTS_DATA")
-private val PARTIQL_EVAL_EQUIV_TEST_DATA_DIR = System.getenv("PARTIQL_EVAL_EQUIV_TESTS_DATA")
+private val PARTIQL_EVAL_TEST_DATA_DIR = System.getenv("PARTIQL_EVAL_TESTS_DATA") ?: "../partiql-tests/partiql-tests-data/eval"
+private val PARTIQL_EVAL_EQUIV_TEST_DATA_DIR = System.getenv("PARTIQL_EVAL_EQUIV_TESTS_DATA") ?: "../partiql-tests/partiql-tests-data/eval-equiv"
 
 /**
  * Reduces some of the boilerplate associated with the style of parameterized testing frequently