Self-review cleanup

CHANGELOG.md

@@ -29,24 +29,28 @@ Thank you to all who have contributed!
 
 ### Added
 - Added constrained decimal as valid parameter type to functions that take in numeric parameters. 
-- Added async version of physical plan evaluator `PartiQLCompilerAsync`. The following related async APIs have been added:
-  - `org.partiql.lang.compiler` -- `PartiQLCompilerAsync`, `PartiQLCompilerAsyncBuilder`, `PartiQLCompilerAsyncDefault`, `PartiQLCompilerPipelineAsync`
-  - `org.partiql.lang.eval` -- `PartiQLStatementAsync`
-  - `org.partiql.lang.eval.physical` -- `VariableBindingAsync`
-  - `org.partiql.lang.eval.physical.operators` -- `AggregateOperatorFactoryAsync`, `CompiledGroupKeyAsync`, `CompiledAggregateFunctionAsync`, `FilterRelationalOperatorFactoryAsync`, `JoinRelationalOperatorFactoryAsync`, `LetRelationalOperatorFactoryAsync`, `LimitRelationalOperatorFactoryAsync`, `OffsetRelationalOperatorFactoryAsync`, `ProjectRelationalOperatorFactoryAsync`, `RelationExpressionAsync`, `ScanRelationalOperatorFactoryAsync`, `SortOperatorFactoryAsync`, `CompiledSortKeyAsync`, `UnpivotOperatorFactoryAsync`, `ValueExpressionAsync`, `WindowRelationalOperatorFactoryAsync`, `CompiledWindowFunctionAsync`
-  - `org.partiql.lang.eval.physical.window` -- `NavigationWindowFunctionAsync`, `WindowFunctionAsync`
+- Added async version of physical plan evaluator `PartiQLCompilerAsync`. 
+  - The following related async APIs have been added:
+    - `org.partiql.lang.compiler` -- `PartiQLCompilerAsync`, `PartiQLCompilerAsyncBuilder`, `PartiQLCompilerAsyncDefault`, `PartiQLCompilerPipelineAsync`
+    - `org.partiql.lang.eval` -- `PartiQLStatementAsync`
+    - `org.partiql.lang.eval.physical` -- `VariableBindingAsync`
+    - `org.partiql.lang.eval.physical.operators` -- `AggregateOperatorFactoryAsync`, `CompiledGroupKeyAsync`, `CompiledAggregateFunctionAsync`, `FilterRelationalOperatorFactoryAsync`, `JoinRelationalOperatorFactoryAsync`, `LetRelationalOperatorFactoryAsync`, `LimitRelationalOperatorFactoryAsync`, `OffsetRelationalOperatorFactoryAsync`, `ProjectRelationalOperatorFactoryAsync`, `RelationExpressionAsync`, `ScanRelationalOperatorFactoryAsync`, `SortOperatorFactoryAsync`, `CompiledSortKeyAsync`, `UnpivotOperatorFactoryAsync`, `ValueExpressionAsync`, `WindowRelationalOperatorFactoryAsync`, `CompiledWindowFunctionAsync`
+    - `org.partiql.lang.eval.physical.window` -- `NavigationWindowFunctionAsync`, `WindowFunctionAsync`
+  - Overall, we see about a 10-20% performance decline in running a single query on the synchronous vs async evaluator
+    - JMH benchmarks added to partiql-lang: `PartiQLCompilerPipelineBenchmark` and `PartiQLCompilerPipelineAsyncBenchmark`
 
 ### Changed
 - Function resolution logic: Now the function resolver would match all possible candidate(based on if the argument can be coerced to the Signature parameter type). If there are multiple match it will first attempt to pick the one requires the least cast, then pick the function with the highest precedence. 
 - partiql-cli -- experimental version of CLI now uses the async physical plan evaluator
 
 ### Deprecated
-- As part of the additions to make an async physical plan evaluator, the synchronous physical plan evaluator `PartiQLCompiler` has been deprecated. The following related APIs have been deprecated
-  - `org.partiql.lang.compiler` -- `PartiQLCompiler`, `PartiQLCompilerBuilder`, `PartiQLCompilerDefault`, `PartiQLCompilerPipeline`
-  - `org.partiql.lang.eval` -- `PartiQLStatement`
-  - `org.partiql.lang.eval.physical` -- `VariableBinding`
-  - `org.partiql.lang.eval.physical.operators` -- `AggregateOperatorFactory`, `CompiledGroupKey`, `CompiledAggregateFunction`, `FilterRelationalOperatorFactory`, `JoinRelationalOperatorFactory`, `LetRelationalOperatorFactory`, `LimitRelationalOperatorFactory`, `OffsetRelationalOperatorFactory`, `ProjectRelationalOperatorFactory`, `RelationExpression`, `ScanRelationalOperatorFactory`, `SortOperatorFactory`, `CompiledSortKey`, `UnpivotOperatorFactory`, `ValueExpression`, `WindowRelationalOperatorFactory`, `CompiledWindowFunction`
-  - `org.partiql.lang.eval.physical.window` -- `NavigationWindowFunction`, `WindowFunction`
+- As part of the additions to make an async physical plan evaluator, the synchronous physical plan evaluator `PartiQLCompiler` has been deprecated.
+  - The following related APIs have been deprecated
+    - `org.partiql.lang.compiler` -- `PartiQLCompiler`, `PartiQLCompilerBuilder`, `PartiQLCompilerDefault`, `PartiQLCompilerPipeline`
+    - `org.partiql.lang.eval` -- `PartiQLStatement`
+    - `org.partiql.lang.eval.physical` -- `VariableBinding`
+    - `org.partiql.lang.eval.physical.operators` -- `AggregateOperatorFactory`, `CompiledGroupKey`, `CompiledAggregateFunction`, `FilterRelationalOperatorFactory`, `JoinRelationalOperatorFactory`, `LetRelationalOperatorFactory`, `LimitRelationalOperatorFactory`, `OffsetRelationalOperatorFactory`, `ProjectRelationalOperatorFactory`, `RelationExpression`, `ScanRelationalOperatorFactory`, `SortOperatorFactory`, `CompiledSortKey`, `UnpivotOperatorFactory`, `ValueExpression`, `WindowRelationalOperatorFactory`, `CompiledWindowFunction`
+    - `org.partiql.lang.eval.physical.window` -- `NavigationWindowFunction`, `WindowFunction`
 
 ### Fixed
 

examples/src/main/java/org/partiql/examples/PartiQLCompilerPipelineAsyncJavaExample.java

@@ -35,7 +35,7 @@
 /**
  * This is an example of using PartiQLCompilerPipelineAsync in Java.
  * It is an experimental feature and is marked as such, with @OptIn, in this example.
- * Unfortunately, it seems like the Java does not recognize the Optin annotation specified in Kotlin.
+ * Unfortunately, it seems like the Java does not recognize the OptIn annotation specified in Kotlin.
  * Java users will be able to access the experimental APIs freely, and not be warned at all.
  */
 public class PartiQLCompilerPipelineAsyncJavaExample extends Example {
@@ -57,10 +57,7 @@ public void run() {
                 "{name: \"mary\", age: 19}" +
                 "]";
 
-        final Bindings<ExprValue> globalVariables = Bindings.<ExprValue>lazyBindingsBuilder().addBinding("myTable", () -> {
-            ExprValue exprValue = ExprValue.of(ion.singleValue(myTable));
-            return exprValue;
-        }).build();
+        final Bindings<ExprValue> globalVariables = Bindings.<ExprValue>lazyBindingsBuilder().addBinding("myTable", () -> ExprValue.of(ion.singleValue(myTable))).build();
 
         final EvaluationSession session = EvaluationSession.builder()
                 .globals(globalVariables)
@@ -97,6 +94,11 @@ public void run() {
         String query = "SELECT t.name FROM myTable AS t WHERE t.age > 20";
 
         print("PartiQL query:", query);
+
+        // Calling Kotlin coroutines from Java requires some additional libraries from `kotlinx.coroutines.future`
+        // to return a `java.util.concurrent.CompletableFuture`. If a use case arises to call the
+        // `PartiQLCompilerPipelineAsync` APIs directly from Java, we can add Kotlin functions that directly return
+        // Java's async libraries (e.g. in https://stackoverflow.com/a/52887677).
         CompletableFuture<PartiQLStatementAsync> statementFuture = FutureKt.future(
                 CoroutineScopeKt.CoroutineScope(EmptyCoroutineContext.INSTANCE),
                 EmptyCoroutineContext.INSTANCE,

partiql-lang/build.gradle.kts

@@ -40,7 +40,6 @@ dependencies {
     implementation(Deps.csv)
     implementation(Deps.kotlinReflect)
     implementation(Deps.kotlinxCoroutines)
-    implementation(Deps.kotlinxCoroutinesJdk8)
 
     testImplementation(testFixtures(project(":partiql-planner")))
     testImplementation(project(":plugins:partiql-memory"))

partiql-lang/src/jmh/kotlin/org/partiql/jmh/benchmarks/PartiQLCompilerPipelineAsyncBenchmark.kt

@@ -1,5 +1,6 @@
 package org.partiql.jmh.benchmarks
 
+import com.amazon.ion.IonSystem
 import com.amazon.ion.system.IonSystemBuilder
 import kotlinx.coroutines.runBlocking
 import org.openjdk.jmh.annotations.Benchmark
@@ -41,10 +42,10 @@ open class PartiQLCompilerPipelineAsyncBenchmark {
     @State(Scope.Thread)
     @OptIn(ExperimentalPartiQLCompilerPipeline::class)
     open class MyState {
-        val parser = PartiQLParserBuilder.standard().build()
-        val myIonSystem = IonSystemBuilder.standard().build()
+        private val parser = PartiQLParserBuilder.standard().build()
+        private val myIonSystem: IonSystem = IonSystemBuilder.standard().build()
 
-        fun tableWithRows(numRows: Int): ExprValue {
+        private fun tableWithRows(numRows: Int): ExprValue {
             val allRows = (1..numRows).joinToString { index ->
                 """
                     {
@@ -62,7 +63,7 @@ open class PartiQLCompilerPipelineAsyncBenchmark {
             )
         }
 
-        val bindings = Bindings.ofMap(
+        private val bindings = Bindings.ofMap(
             mapOf(
                 "t1" to tableWithRows(1),
                 "t10" to tableWithRows(10),
@@ -73,7 +74,7 @@ open class PartiQLCompilerPipelineAsyncBenchmark {
             )
         )
 
-        val parameters = listOf(
+        private val parameters = listOf(
             ExprValue.newInt(5), // WHERE `id` > 5
             ExprValue.newInt(1000000), // LIMIT 1000000
             ExprValue.newInt(3), // OFFSET 3 * 2

partiql-lang/src/jmh/kotlin/org/partiql/jmh/benchmarks/PartiQLCompilerPipelineBenchmark.kt

@@ -1,5 +1,6 @@
 package org.partiql.jmh.benchmarks
 
+import com.amazon.ion.IonSystem
 import com.amazon.ion.system.IonSystemBuilder
 import kotlinx.coroutines.runBlocking
 import org.openjdk.jmh.annotations.Benchmark
@@ -42,10 +43,10 @@ open class PartiQLCompilerPipelineBenchmark {
     @State(Scope.Thread)
     @OptIn(ExperimentalPartiQLCompilerPipeline::class)
     open class MyState {
-        val parser = PartiQLParserBuilder.standard().build()
-        val myIonSystem = IonSystemBuilder.standard().build()
+        private val parser = PartiQLParserBuilder.standard().build()
+        private val myIonSystem: IonSystem = IonSystemBuilder.standard().build()
 
-        fun tableWithRows(numRows: Int): ExprValue {
+        private fun tableWithRows(numRows: Int): ExprValue {
             val allRows = (1..numRows).joinToString { index ->
                 """
                     {
@@ -63,7 +64,7 @@ open class PartiQLCompilerPipelineBenchmark {
             )
         }
 
-        val bindings = Bindings.ofMap(
+        private val bindings = Bindings.ofMap(
             mapOf(
                 "t1" to tableWithRows(1),
                 "t10" to tableWithRows(10),
@@ -74,7 +75,7 @@ open class PartiQLCompilerPipelineBenchmark {
             )
         )
 
-        val parameters = listOf(
+        private val parameters = listOf(
             ExprValue.newInt(5), // WHERE `id` > 5
             ExprValue.newInt(1000000), // LIMIT 1000000
             ExprValue.newInt(3), // OFFSET 3 * 2
@@ -133,7 +134,7 @@ open class PartiQLCompilerPipelineBenchmark {
             OFFSET ? * ?
             """.trimIndent()
         )
-        val query6 = parser.parseAstStatement(
+        private val query6 = parser.parseAstStatement(
             """
             SELECT *
             FROM t100000
@@ -143,7 +144,7 @@ open class PartiQLCompilerPipelineBenchmark {
             OFFSET ? * ?
             """.trimIndent()
         )
-        val query7 = parser.parseAstStatement(
+        private val query7 = parser.parseAstStatement(
             """
             SELECT *
             FROM t10000
@@ -153,7 +154,7 @@ open class PartiQLCompilerPipelineBenchmark {
             OFFSET ? * ?
             """.trimIndent()
         )
-        val query8 = parser.parseAstStatement(
+        private val query8 = parser.parseAstStatement(
             """
             SELECT *
             FROM t1000
@@ -163,7 +164,7 @@ open class PartiQLCompilerPipelineBenchmark {
             OFFSET ? * ?
             """.trimIndent()
         )
-        val query9 = parser.parseAstStatement(
+        private val query9 = parser.parseAstStatement(
             """
             SELECT *
             FROM t100
@@ -173,7 +174,7 @@ open class PartiQLCompilerPipelineBenchmark {
             OFFSET ? * ?
             """.trimIndent()
         )
-        val query10 = parser.parseAstStatement(
+        private val query10 = parser.parseAstStatement(
             """
             SELECT *
             FROM t10
@@ -183,7 +184,7 @@ open class PartiQLCompilerPipelineBenchmark {
             OFFSET ? * ?
             """.trimIndent()
         )
-        val query11 = parser.parseAstStatement(
+        private val query11 = parser.parseAstStatement(
             """
             SELECT *
             FROM t1

partiql-lang/src/main/kotlin/org/partiql/lang/compiler/PartiQLCompilerAsyncBuilder.kt

@@ -137,8 +137,7 @@ class PartiQLCompilerAsyncBuilder private constructor() {
     private fun allFunctions(typingMode: TypingMode): List<ExprFunction> {
         val definitionalBuiltins = definitionalBuiltins(typingMode)
         val builtins = SCALAR_BUILTINS_DEFAULT
-        val allFunctions = definitionalBuiltins + builtins + customFunctions + DynamicLookupExprFunction()
-        return allFunctions
+        return definitionalBuiltins + builtins + customFunctions + DynamicLookupExprFunction()
     }
 
     private fun allOperatorFactories() = (DEFAULT_RELATIONAL_OPERATOR_FACTORIES + customOperatorFactories).apply {

partiql-lang/src/main/kotlin/org/partiql/lang/compiler/PartiQLCompilerAsyncDefault.kt

@@ -36,11 +36,11 @@ import org.partiql.lang.types.TypedOpParameter
 
 @ExperimentalPartiQLCompilerPipeline
 internal class PartiQLCompilerAsyncDefault(
-    private val evaluatorOptions: EvaluatorOptions,
-    private val customTypedOpParameters: Map<String, TypedOpParameter>,
-    private val functions: List<ExprFunction>,
-    private val procedures: Map<String, StoredProcedure>,
-    private val operatorFactories: Map<RelationalOperatorFactoryKey, RelationalOperatorFactory>
+    evaluatorOptions: EvaluatorOptions,
+    customTypedOpParameters: Map<String, TypedOpParameter>,
+    functions: List<ExprFunction>,
+    procedures: Map<String, StoredProcedure>,
+    operatorFactories: Map<RelationalOperatorFactoryKey, RelationalOperatorFactory>
 ) : PartiQLCompilerAsync {
 
     private lateinit var exprConverter: PhysicalPlanCompilerAsyncImpl
@@ -113,7 +113,7 @@ internal class PartiQLCompilerAsyncDefault(
 
     private fun compileExplainDomain(statement: PartiqlPhysical.ExplainTarget.Domain, details: PartiQLPlanner.PlanningDetails): PartiQLResult.Explain.Domain {
         val format = statement.format?.text
-        val type = statement.type?.text?.toUpperCase() ?: ExplainDomains.AST.name
+        val type = statement.type?.text?.uppercase() ?: ExplainDomains.AST.name
         val domain = try {
             ExplainDomains.valueOf(type)
         } catch (ex: IllegalArgumentException) {

partiql-lang/src/main/kotlin/org/partiql/lang/eval/PartiQLStatementAsync.kt

@@ -15,7 +15,7 @@
 package org.partiql.lang.eval
 
 /**
- * A compiled PartiQL statement
+ * A compiled PartiQL statement intended to be evaluated from a Kotlin coroutine.
  */
 fun interface PartiQLStatementAsync {
 

partiql-lang/src/main/kotlin/org/partiql/lang/eval/Thunk.kt

@@ -107,6 +107,25 @@ data class ThunkOptions private constructor(
     }
 }
 
+internal val DEFAULT_EXCEPTION_HANDLER_FOR_LEGACY_MODE: ThunkExceptionHandlerForLegacyMode = { e, sourceLocation ->
+    val message = e.message ?: "<NO MESSAGE>"
+    throw EvaluationException(
+        "Internal error, $message",
+        errorCode = (e as? EvaluationException)?.errorCode ?: ErrorCode.EVALUATOR_GENERIC_EXCEPTION,
+        errorContext = errorContextFrom(sourceLocation),
+        cause = e,
+        internal = true
+    )
+}
+
+internal val DEFAULT_EXCEPTION_HANDLER_FOR_PERMISSIVE_MODE: ThunkExceptionHandlerForPermissiveMode = { e, _ ->
+    when (e) {
+        is InterruptedException -> { throw e }
+        is StackOverflowError -> { throw e }
+        else -> {}
+    }
+}
+
 /**
  * An extension method for creating [ThunkFactory] based on the type of [TypingMode]
  *  - when [TypingMode] is [TypingMode.LEGACY], creates [LegacyThunkFactory]

partiql-lang/src/main/kotlin/org/partiql/lang/eval/ThunkAsync.kt

@@ -46,46 +46,6 @@ internal typealias ThunkAsync<TEnv> = suspend (TEnv) -> ExprValue
  */
 internal typealias ThunkValueAsync<TEnv, TArg> = suspend (TEnv, TArg) -> ExprValue
 
-/**
- * A type alias for an exception handler which always throws(primarily used for [TypingMode.LEGACY]).
- */
-internal typealias ThunkExceptionHandlerForLegacyModeAsync = (Throwable, SourceLocationMeta?) -> Nothing
-
-/**
- * A type alias for an exception handler which does not always throw(primarily used for [TypingMode.PERMISSIVE]).
- */
-internal typealias ThunkExceptionHandlerForPermissiveModeAsync = (Throwable, SourceLocationMeta?) -> Unit
-
-/**
- * Options for thunk construction.
- *
- *  - [ThunkOptions.handleExceptionForLegacyMode] will be called when in [TypingMode.LEGACY] mode
- *  - [ThunkOptions.handleExceptionForPermissiveMode] will be called when in [TypingMode.PERMISSIVE] mode
- *  - [ThunkOptions.thunkReturnTypeAssertions] is intended for testing only, and ensures that the return value of every expression
- *  conforms to its `StaticType` meta.  This has negative performance implications so should be avoided in production
- *  environments.  This only be used for testing and diagnostic purposes only.
- * The default exception handler wraps any [Throwable] exception and throws [EvaluationException]
- */
-
-internal val DEFAULT_EXCEPTION_HANDLER_FOR_LEGACY_MODE: ThunkExceptionHandlerForLegacyModeAsync = { e, sourceLocation ->
-    val message = e.message ?: "<NO MESSAGE>"
-    throw EvaluationException(
-        "Internal error, $message",
-        errorCode = (e as? EvaluationException)?.errorCode ?: ErrorCode.EVALUATOR_GENERIC_EXCEPTION,
-        errorContext = errorContextFrom(sourceLocation),
-        cause = e,
-        internal = true
-    )
-}
-
-internal val DEFAULT_EXCEPTION_HANDLER_FOR_PERMISSIVE_MODE: ThunkExceptionHandlerForPermissiveModeAsync = { e, _ ->
-    when (e) {
-        is InterruptedException -> { throw e }
-        is StackOverflowError -> { throw e }
-        else -> {}
-    }
-}
-
 /**
  * An extension method for creating [ThunkFactoryAsync] based on the type of [TypingMode]
  *  - when [TypingMode] is [TypingMode.LEGACY], creates [LegacyThunkFactoryAsync]
@@ -348,7 +308,7 @@ internal abstract class ThunkFactoryAsync<TEnv>(
     }
 
     /**
-     * Handles exceptions appropriately for a run-time [Thunk<TEnv>].
+     * Handles exceptions appropriately for a run-time [ThunkAsync<TEnv>].
      *
      * - The [SourceLocationMeta] will be extracted from [MetaContainer] and included in any [EvaluationException] that
      * is thrown, if present.
@@ -479,7 +439,7 @@ internal class LegacyThunkFactoryAsync<TEnv>(
     }
 
     /**
-     * Handles exceptions appropriately for a run-time [Thunk<TEnv>] respecting [TypingMode.LEGACY] behaviour.
+     * Handles exceptions appropriately for a run-time [ThunkAsync<TEnv>] respecting [TypingMode.LEGACY] behaviour.
      *
      * - The [SourceLocationMeta] will be extracted from [MetaContainer] and included in any [EvaluationException] that
      * is thrown, if present.
@@ -630,7 +590,7 @@ internal class PermissiveThunkFactoryAsync<TEnv>(
     /**
      * Handles exceptions appropriately for a run-time [Thunk<TEnv>] respecting [TypingMode.PERMISSIVE] behaviour.
      *
-     * - Exceptions thrown by [block] that are [EvaluationException] are caught and [MissingExprValue] is returned.
+     * - Exceptions thrown by [block] that are [EvaluationException] are caught and [ExprValue.missingValue] is returned.
      * - Exceptions thrown by [block] that are not an [EvaluationException] cause an [EvaluationException] to be thrown
      * with the original exception as the cause.
      */