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

Enable Eval Test Suites #1340

Merged
merged 10 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
11 changes: 10 additions & 1 deletion partiql-eval/src/main/kotlin/org/partiql/eval/PartiQLEngine.kt
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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<*>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

making this more akin to the Planner APIs.


// TODO: Pass session variable during statement execution once we finalize data structure for session.
public fun execute(statement: PartiQLStatement<*>): PartiQLResult

companion object {
Expand All @@ -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()
)
}
Original file line number Diff line number Diff line change
@@ -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()
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
36 changes: 31 additions & 5 deletions partiql-eval/src/main/kotlin/org/partiql/eval/internal/Compiler.kt
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions test/partiql-tests-runner/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 {
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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)
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading