[v1] Planner docs and cleanup (#1708)

* Resolve package warnings

* Internal code refactor; remove top-level internal functions

* Add javadoc for public APIs

* Fix planner test warnings

partiql-planner/src/main/kotlin/org/partiql/planner/PartiQLPlanner.kt

@@ -47,9 +47,19 @@ public interface PartiQLPlanner {
 
     public companion object {
 
+        /**
+         * Create a new [PartiQLPlannerBuilder] instance.
+         *
+         * @return
+         */
         @JvmStatic
         public fun builder(): PartiQLPlannerBuilder = PartiQLPlannerBuilder()
 
+        /**
+         * Create a new [PartiQLPlanner] instance with the default configuration.
+         *
+         * @return
+         */
         @JvmStatic
         public fun standard(): PartiQLPlanner = PartiQLPlannerBuilder().build()
     }

partiql-planner/src/main/kotlin/org/partiql/planner/PartiQLPlannerPass.kt

@@ -3,7 +3,16 @@ package org.partiql.planner
 import org.partiql.plan.Plan
 import org.partiql.spi.Context
 
+/**
+ * Interface specifies a pass that can be applied to a [Plan] by the [PartiQLPlanner].
+ */
 public interface PartiQLPlannerPass {
-
+    /**
+     * Applies this pass to the given [Plan] and returns the resulting [Plan].
+     *
+     * @param plan The [Plan] to apply this pass to.
+     * @param ctx The [Context] to use for this pass.
+     * @return The resulting [Plan] after applying this pass.
+     */
     public fun apply(plan: Plan, ctx: Context): Plan
 }

partiql-planner/src/main/kotlin/org/partiql/planner/builder/PartiQLPlannerBuilder.kt

@@ -9,10 +9,12 @@ import org.partiql.planner.internal.SqlPlanner
  * PartiQLPlannerBuilder is used to programmatically construct a [PartiQLPlanner] implementation.
  *
  * Usage:
- *      PartiQLPlanner.builder()
- *                    .signal()
- *                    .addPass(myPass)
- *                    .build()
+ * ```
+ * PartiQLPlanner.builder()
+ *               .signal()
+ *               .addPass(myPass)
+ *               .build()
+ * ```
  */
 public class PartiQLPlannerBuilder {
 
@@ -49,7 +51,7 @@ public class PartiQLPlannerBuilder {
     }
 
     /**
-     * Java style method for setting the planner to signal mode
+     * Java style method for setting the planner to signal mode.
      */
     public fun signal(signal: Boolean = true): PartiQLPlannerBuilder {
         if (signal) {

partiql-planner/src/main/kotlin/org/partiql/planner/internal/CoercionFamily.kt

@@ -6,7 +6,7 @@ import org.partiql.spi.types.PType
  * This represents SQL:1999 Section 4.1.2 "Type conversions and mixing of data types" and breaks down the different
  * coercion groups.
  *
- * TODO: [UNKNOWN] should likely be removed in the future. However, it is needed due to literal nulls and missings.
+ * TODO: [UNKNOWN] should likely be removed in the future. However, it is needed due to literal null and missing.
  * TODO: [DYNAMIC] should likely be removed in the future. This is currently only kept to map function signatures.
  */
 internal enum class CoercionFamily {
@@ -25,10 +25,10 @@ internal enum class CoercionFamily {
     companion object {
 
         /**
-         * Gets the coercion family for the given [PType.Kind].
+         * Gets the coercion family for the given [PType.code].
          *
          * @see CoercionFamily
-         * @see PType.Kind
+         * @see PType.code
          * @see family
          */
         @JvmStatic

partiql-planner/src/main/kotlin/org/partiql/planner/internal/Env.kt

@@ -282,10 +282,9 @@ internal class Env(private val session: Session) {
         for (i in args.indices) {
             val a = args[i]
             val p = parameters[i]
-            val m = p.getMatch(a)
-            when {
-                m == null -> return null
-                m == a -> continue
+            when (val m = p.getMatch(a)) {
+                null -> return null
+                a -> continue
                 else -> mapping[i] = coercion(a, m)
             }
         }

partiql-planner/src/main/kotlin/org/partiql/planner/internal/PlannerFlag.kt

@@ -6,7 +6,7 @@ internal enum class PlannerFlag {
      *
      * If this flag is included:
      *
-     *    The problematic operation will be tracked in problem callback as a error.
+     *    The problematic operation will be tracked in problem callback as an error.
      *
      *    The result plan will turn the problematic operation into an error node.
      *

partiql-planner/src/main/kotlin/org/partiql/planner/internal/SqlPlanner.kt

@@ -7,8 +7,9 @@ import org.partiql.plan.Plan
 import org.partiql.planner.PartiQLPlanner
 import org.partiql.planner.PartiQLPlanner.Result
 import org.partiql.planner.PartiQLPlannerPass
-import org.partiql.planner.internal.normalize.normalize
 import org.partiql.planner.internal.transforms.AstToPlan
+import org.partiql.planner.internal.transforms.NormalizeFromSource
+import org.partiql.planner.internal.transforms.NormalizeGroupBy
 import org.partiql.planner.internal.transforms.PlanTransform
 import org.partiql.planner.internal.typer.PlanTyper
 import org.partiql.spi.Context
@@ -60,6 +61,17 @@ internal class SqlPlanner(
         }
     }
 
+    /**
+     * AST normalization
+     */
+    private fun Statement.normalize(): Statement {
+        // could be a fold, but this is nice for setting breakpoints
+        var ast = this
+        ast = NormalizeFromSource.apply(ast)
+        ast = NormalizeGroupBy.apply(ast)
+        return ast
+    }
+
     /**
      * Create a plan with a query action and error node.
      *

partiql-planner/src/main/kotlin/org/partiql/planner/internal/normalize/AstPass.kt → partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/AstPass.kt

@@ -12,7 +12,7 @@
  *  language governing permissions and limitations under the License.
  */
 
-package org.partiql.planner.internal.normalize
+package org.partiql.planner.internal.transforms
 
 import org.partiql.ast.Statement
 

partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/AstToPlan.kt

@@ -36,7 +36,6 @@ internal object AstToPlan {
     @JvmStatic
     fun apply(statement: AstStatement, env: Env): PlanStatement = statement.accept(ToPlanStatement, env)
 
-    @Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE")
     private object ToPlanStatement : AstVisitor<PlanStatement, Env>() {
 
         override fun defaultReturn(node: AstNode, env: Env) = throw IllegalArgumentException("Unsupported statement")

partiql-planner/src/main/kotlin/org/partiql/planner/internal/normalize/NormalizeFromSource.kt → partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/NormalizeFromSource.kt

@@ -12,7 +12,7 @@
  *  language governing permissions and limitations under the License.
  */
 
-package org.partiql.planner.internal.normalize
+package org.partiql.planner.internal.transforms
 
 import org.partiql.ast.Ast.fromExpr
 import org.partiql.ast.Ast.fromJoin
@@ -26,7 +26,7 @@ import org.partiql.ast.FromType
 import org.partiql.ast.QueryBody
 import org.partiql.ast.Statement
 import org.partiql.ast.expr.Expr
-import org.partiql.planner.internal.helpers.toBinder
+import org.partiql.planner.internal.util.BinderUtils.toBinder
 
 /**
  * Assign aliases to any FROM source which does not have one.

partiql-planner/src/main/kotlin/org/partiql/planner/internal/normalize/NormalizeGroupBy.kt → partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/NormalizeGroupBy.kt

@@ -12,7 +12,7 @@
  *  language governing permissions and limitations under the License.
  */
 
-package org.partiql.planner.internal.normalize
+package org.partiql.planner.internal.transforms
 
 import org.partiql.ast.Ast.groupBy
 import org.partiql.ast.Ast.groupByKey
@@ -21,7 +21,7 @@ import org.partiql.ast.AstRewriter
 import org.partiql.ast.GroupBy
 import org.partiql.ast.Statement
 import org.partiql.ast.expr.Expr
-import org.partiql.planner.internal.helpers.toBinder
+import org.partiql.planner.internal.util.BinderUtils.toBinder
 
 /**
  * Adds a unique binder to each group key.

partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/NormalizeSelect.kt

@@ -47,7 +47,7 @@ import org.partiql.ast.expr.ExprCase
 import org.partiql.ast.expr.ExprQuerySet
 import org.partiql.ast.expr.ExprStruct
 import org.partiql.ast.expr.ExprVarRef
-import org.partiql.planner.internal.helpers.toBinder
+import org.partiql.planner.internal.util.BinderUtils.toBinder
 
 /**
  * Converts SQL-style SELECT to PartiQL SELECT VALUE.
@@ -92,7 +92,7 @@ import org.partiql.planner.internal.helpers.toBinder
  * } FROM A AS x
  * ```
  *
- * NOTE: This does NOT transform subqueries. It operates directly on an [QueryExpr.SFW] -- and that is it. Therefore:
+ * NOTE: This does NOT transform subqueries. It operates directly on an [ExprQuerySet] -- and that is it. Therefore:
  * ```
  * SELECT
  *   (SELECT 1 FROM T AS "T")
@@ -181,7 +181,7 @@ internal object NormalizeSelect {
          */
         private val col = { index: Int -> "_${index + 1}" }
 
-        internal fun visitSFW(node: QueryBody.SFW, ctx: () -> Int): QueryBody.SFW {
+        fun visitSFW(node: QueryBody.SFW, ctx: () -> Int): QueryBody.SFW {
             val sfw = super.visitQueryBodySFW(node, ctx) as QueryBody.SFW
             return when (val select = sfw.select) {
                 is SelectStar -> {
@@ -242,7 +242,7 @@ internal object NormalizeSelect {
         // Helpers
 
         /**
-         * We need to call this from [visitExprSFW] and not override [visitSelectStar] because we need access to the
+         * We need to call this from [visitQueryBodySFW] and not override [visitSelectStar] because we need access to the
          * [From] aliases.
          *
          * Note: We assume that [select] and [from] have already been visited.
@@ -271,7 +271,7 @@ internal object NormalizeSelect {
         }
 
         /**
-         * We need to call this from [visitExprSFW] and not override [visitSelectStar] because we need access to the
+         * We need to call this from [visitQueryBodySFW] and not override [visitSelectStar] because we need access to the
          * [GroupBy] aliases.
          *
          * Note: We assume that [select] and [group] have already been visited.

partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/PlanTransform.kt

@@ -79,7 +79,7 @@ internal class PlanTransform(private val flags: Set<PlannerFlag>) {
             return operators.error(ctx)
         }
 
-        override fun visitRelOpErr(node: org.partiql.planner.internal.ir.Rel.Op.Err, ctx: PType): Any {
+        override fun visitRelOpErr(node: Rel.Op.Err, ctx: PType): Any {
             // Listener should have already received the error. This node is a dud. Registered error listeners should
             // have failed compilation already.
             return operators.scan(operators.error(ctx))

partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RelConverter.kt

@@ -47,7 +47,6 @@ import org.partiql.ast.expr.Expr
 import org.partiql.ast.expr.ExprCall
 import org.partiql.ast.expr.ExprQuerySet
 import org.partiql.planner.internal.Env
-import org.partiql.planner.internal.helpers.toBinder
 import org.partiql.planner.internal.ir.Rel
 import org.partiql.planner.internal.ir.Rex
 import org.partiql.planner.internal.ir.rel
@@ -83,6 +82,7 @@ import org.partiql.planner.internal.ir.rexOpStruct
 import org.partiql.planner.internal.ir.rexOpStructField
 import org.partiql.planner.internal.ir.rexOpVarLocal
 import org.partiql.planner.internal.typer.CompilerType
+import org.partiql.planner.internal.util.BinderUtils.toBinder
 import org.partiql.spi.types.PType
 import org.partiql.spi.value.Datum
 
@@ -156,7 +156,7 @@ internal object RelConverter {
      */
     private fun Expr.toRex(env: Env): Rex = RexConverter.apply(this, env)
 
-    @Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE", "LocalVariableName")
+    @Suppress("LocalVariableName")
     internal class ToRel(private val env: Env) : AstVisitor<Rel, Rel>() {
 
         override fun defaultReturn(node: AstNode, input: Rel): Rel =
@@ -713,7 +713,7 @@ internal object RelConverter {
 
         private fun Identifier.isAggregateCall(): Boolean = identifier.text.lowercase().isAggregateCall()
 
-        override fun defaultReturn(node: AstNode, ctx: Context) = node
+        override fun defaultReturn(node: AstNode, context: Context) = node
     }
 
     private fun syntheticAgg(i: Int) = "\$agg_$i"