Address latest PR feedback

partiql · Dec 5, 2024 · 4682c2e · 4682c2e
1 parent 1a0f6a9
commit 4682c2e
Show file tree

Hide file tree

Showing 38 changed files with 75 additions and 53 deletions.
diff --git a/partiql-eval/api/partiql-eval.api b/partiql-eval/api/partiql-eval.api
@@ -69,7 +69,7 @@ public class org/partiql/eval/compiler/Pattern {
 
 public abstract class org/partiql/eval/compiler/Strategy {
 	public fun <init> (Lorg/partiql/eval/compiler/Pattern;)V
-	public abstract fun apply (Lorg/partiql/eval/compiler/Match;Lorg/partiql/eval/compiler/Strategy$Callback;)Lorg/partiql/eval/Expr;
+	public abstract fun apply (Lorg/partiql/eval/compiler/Match;Lorg/partiql/eval/Mode;Lorg/partiql/eval/compiler/Strategy$Callback;)Lorg/partiql/eval/Expr;
 	public fun getPattern ()Lorg/partiql/eval/compiler/Pattern;
 }
 

diff --git a/partiql-eval/src/main/java/org/partiql/eval/compiler/Strategy.java b/partiql-eval/src/main/java/org/partiql/eval/compiler/Strategy.java
@@ -2,6 +2,7 @@
 
 import org.jetbrains.annotations.NotNull;
 import org.partiql.eval.Expr;
+import org.partiql.eval.Mode;
 import org.partiql.plan.Operator;
 
 /**
@@ -34,11 +35,12 @@ public Pattern getPattern() {
      * Applies the strategy to a logical plan operator and returns the physical operation (expr).
      *
      * @param match holds the matched operators
+     * @param mode evaluation mode
      * @param callback for compiling arguments of matched operators
      * @return the physical operation
      */
     @NotNull
-    public abstract Expr apply(@NotNull Match match, @NotNull Callback callback);
+    public abstract Expr apply(@NotNull Match match, @NotNull Mode mode, @NotNull Callback callback);
 
     /**
      * A compilation callback for strategies to compile arguments of matched operators.
@@ -48,6 +50,7 @@ public interface Callback {
         /**
          * @return a physical operator (expr) for the logical operator.
          */
-        Expr apply(Operator operator);
+        @NotNull
+        Expr apply(@NotNull Operator operator);
     }
 }
diff --git a/partiql-eval/src/main/kotlin/org/partiql/eval/internal/compiler/StandardCompiler.kt b/partiql-eval/src/main/kotlin/org/partiql/eval/internal/compiler/StandardCompiler.kt
@@ -124,11 +124,8 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
 
     override fun prepare(plan: Plan, mode: Mode, ctx: Context): Statement {
         try {
-            val visitor = _Operator_Visitor(mode)
-            if (plan.actions.size != 1) {
-                throw IllegalArgumentException("Only single actions are supported")
-            }
-            val operation = plan.actions[0]
+            val visitor = Visitor(mode)
+            val operation = plan.action
             val statement: Statement = when {
                 operation is Action.Query -> visitor.compile(operation)
                 else -> throw IllegalArgumentException("Only query statements are supported")
@@ -146,10 +143,10 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
     /**
      * Transforms plan relation operators into the internal physical operators.
      */
-    @Suppress("ClassName")
-    private inner class _Operator_Visitor(mode: Mode) : OperatorVisitor<Expr, Unit> {
+    private inner class Visitor(mode: Mode) : OperatorVisitor<Expr, Unit> {
 
-        private val mode = mode.code()
+        private val mode = mode
+        private val MODE = mode.code()
 
         /**
          * Compile a query operation to a query statement.
@@ -179,7 +176,7 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
                     // assume single match
                     val operand = Operand.single(operator)
                     val match = Match(operand)
-                    return strategy.apply(match, ::compileWithStrategies)
+                    return strategy.apply(match, mode, ::compileWithStrategies)
                 }
             }
             return operator.accept(this, Unit)
@@ -246,10 +243,10 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
 
         override fun visitIterate(rel: RelIterate, ctx: Unit): ExprRelation {
             val input = compile(rel.getRex(), ctx)
-            return when (mode) {
+            return when (MODE) {
                 Mode.PERMISSIVE -> RelOpIteratePermissive(input)
                 Mode.STRICT -> RelOpIterate(input)
-                else -> throw IllegalStateException("Unsupported execution mode: $mode")
+                else -> throw IllegalStateException("Unsupported execution mode: $MODE")
             }
         }
 
@@ -291,10 +288,10 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
 
         override fun visitScan(rel: RelScan, ctx: Unit): ExprRelation {
             val input = compile(rel.rex, ctx)
-            return when (mode) {
+            return when (MODE) {
                 Mode.PERMISSIVE -> RelOpScanPermissive(input)
                 Mode.STRICT -> RelOpScan(input)
-                else -> throw IllegalStateException("Unsupported execution mode: $mode")
+                else -> throw IllegalStateException("Unsupported execution mode: $MODE")
             }
         }
 
@@ -320,10 +317,10 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
 
         override fun visitUnpivot(rel: RelUnpivot, ctx: Unit): ExprRelation {
             val input = compile(rel.rex, ctx)
-            return when (mode) {
+            return when (MODE) {
                 Mode.PERMISSIVE -> RelOpUnpivot.Permissive(input)
                 Mode.STRICT -> RelOpUnpivot.Strict(input)
-                else -> throw IllegalStateException("Unsupported execution mode: $mode")
+                else -> throw IllegalStateException("Unsupported execution mode: $MODE")
             }
         }
 
@@ -437,10 +434,10 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
             val input = compile(rex.getInput(), ctx)
             val key = compile(rex.getKey(), ctx)
             val value = compile(rex.getValue(), ctx)
-            return when (mode) {
+            return when (MODE) {
                 Mode.PERMISSIVE -> ExprPivotPermissive(input, key, value)
                 Mode.STRICT -> ExprPivot(input, key, value)
-                else -> throw IllegalStateException("Unsupported execution mode: $mode")
+                else -> throw IllegalStateException("Unsupported execution mode: $MODE")
             }
         }
 
@@ -457,10 +454,10 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
                 val v = compile(it.value, ctx).catch()
                 ExprStructField(k, v)
             }
-            return when (mode) {
+            return when (MODE) {
                 Mode.PERMISSIVE -> ExprStructPermissive(fields)
                 Mode.STRICT -> ExprStructStrict(fields)
-                else -> throw IllegalStateException("Unsupported execution mode: $mode")
+                else -> throw IllegalStateException("Unsupported execution mode: $MODE")
             }
         }
 
@@ -503,10 +500,10 @@ internal class StandardCompiler(strategies: List<Strategy>) : PartiQLCompiler {
         /**
          * Some places "catch" an error and return the MISSING value.
          */
-        private fun ExprValue.catch(): ExprValue = when (mode) {
+        private fun ExprValue.catch(): ExprValue = when (MODE) {
             Mode.PERMISSIVE -> ExprPermissive(this)
             Mode.STRICT -> this
-            else -> throw IllegalStateException("Unsupported execution mode: $mode")
+            else -> throw IllegalStateException("Unsupported execution mode: $MODE")
         }
     }
 }
diff --git a/partiql-eval/src/test/kotlin/org/partiql/eval/compiler/StrategyTest.kt b/partiql-eval/src/test/kotlin/org/partiql/eval/compiler/StrategyTest.kt
@@ -34,7 +34,7 @@ public class StrategyTest {
         var trigged = false
         val pattern = Pattern(RelLimit::class.java)
         val strategy = object : Strategy(pattern) {
-            override fun apply(match: Match, callback: Callback): Expr {
+            override fun apply(match: Match, mode: Mode, callback: Callback): Expr {
                 trigged = true
                 return MyLimit()
             }

diff --git a/partiql-plan/api/partiql-plan.api b/partiql-plan/api/partiql-plan.api
@@ -385,7 +385,7 @@ public final class org/partiql/plan/Operators$DefaultImpls {
 }
 
 public abstract interface class org/partiql/plan/Plan {
-	public abstract fun getActions ()Ljava/util/List;
+	public abstract fun getAction ()Lorg/partiql/plan/Action;
 	public fun getVersion ()Lorg/partiql/plan/Version;
 }
 
@@ -409,7 +409,7 @@ public abstract class org/partiql/plan/rel/RelAggregate : org/partiql/plan/rel/R
 	public abstract fun getGroups ()Ljava/util/List;
 	public abstract fun getInput ()Lorg/partiql/plan/rel/Rel;
 	public abstract fun getMeasures ()Ljava/util/List;
-	public static fun measure (Lorg/partiql/spi/function/Aggregation;Ljava/util/List;Ljava/lang/Boolean;)Lorg/partiql/plan/rel/RelAggregate$Measure;
+	public static fun measure (Lorg/partiql/spi/function/Aggregation;Ljava/util/List;Z)Lorg/partiql/plan/rel/RelAggregate$Measure;
 	protected final fun operands ()Ljava/util/List;
 	protected final fun type ()Lorg/partiql/plan/rel/RelType;
 }
@@ -418,7 +418,7 @@ public class org/partiql/plan/rel/RelAggregate$Measure {
 	public fun copy (Ljava/util/List;)Lorg/partiql/plan/rel/RelAggregate$Measure;
 	public fun getAgg ()Lorg/partiql/spi/function/Aggregation;
 	public fun getArgs ()Ljava/util/List;
-	public fun isDistinct ()Ljava/lang/Boolean;
+	public fun isDistinct ()Z
 }
 
 public abstract class org/partiql/plan/rel/RelBase : org/partiql/plan/rel/Rel {

diff --git a/partiql-plan/src/main/java/org/partiql/plan/Plan.java b/partiql-plan/src/main/java/org/partiql/plan/Plan.java
@@ -2,8 +2,6 @@
 
 import org.jetbrains.annotations.NotNull;
 
-import java.util.List;
-
 /**
  * A plan holds operations that can be executed.
  */
@@ -18,8 +16,8 @@ default public Version getVersion() {
     }
 
     /**
-     * @return statement actions to execute.
+     * @return statement action to execute.
      */
     @NotNull
-    public List<Action> getActions();
+    public Action getAction();
 }
diff --git a/partiql-plan/src/main/java/org/partiql/plan/rel/RelAggregate.java b/partiql-plan/src/main/java/org/partiql/plan/rel/RelAggregate.java
@@ -27,7 +27,7 @@ public static RelAggregate create(@NotNull Rel input, @NotNull List<Measure> mea
      * @return new  {@link Measure} instance
      */
     @NotNull
-    public static Measure measure(@NotNull Aggregation agg, @NotNull List<Rex> args, @NotNull  Boolean distinct) {
+    public static Measure measure(@NotNull Aggregation agg, @NotNull List<Rex> args, @NotNull  boolean distinct) {
         return new Measure(agg, args, distinct);
     }
 
@@ -88,9 +88,9 @@ public static class Measure {
 
         private final Aggregation agg;
         private final List<Rex> args;
-        private final Boolean distinct;
+        private final boolean distinct;
 
-        private Measure(Aggregation agg, List<Rex> args, Boolean distinct) {
+        private Measure(Aggregation agg, List<Rex> args, boolean distinct) {
             this.agg = agg;
             this.args = args;
             this.distinct = distinct;
@@ -106,8 +106,7 @@ public List<Rex> getArgs() {
             return args;
         }
 
-        @NotNull
-        public Boolean isDistinct() {
+        public boolean isDistinct() {
             return distinct;
         }
 

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rel/RelBase.java b/partiql-plan/src/main/java/org/partiql/plan/rel/RelBase.java
@@ -61,12 +61,14 @@ public final List<Operand> getOperands() {
      *
      * @return computed type.
      */
+    @NotNull
     protected abstract RelType type();
 
     /**
      * PROTECTED (could also be package private atm).
      *
      * @return computed operands.
      */
+    @NotNull
     protected abstract List<Operand> operands();
 }
diff --git a/partiql-plan/src/main/java/org/partiql/plan/rel/RelJoin.java b/partiql-plan/src/main/java/org/partiql/plan/rel/RelJoin.java
@@ -45,6 +45,7 @@ public static RelJoin create(@NotNull Rel left, @NotNull Rel right, @NotNull Rex
     @NotNull
     public abstract Rex getCondition();
 
+    @NotNull
     @Override
     protected RelType type() {
         throw new UnsupportedOperationException("compute join type");

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexBase.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexBase.java
@@ -61,12 +61,14 @@ public final List<Operand> getOperands() {
      *
      * @return computed type.
      */
+    @NotNull
     protected abstract RexType type();
 
     /**
      * PROTECTED (could also be package private atm).
      *
      * @return computed operands.
      */
+    @NotNull
     protected abstract List<Operand> operands();
 }
diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexCall.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexCall.java
@@ -29,11 +29,13 @@ public static RexCall create(@NotNull Function.Instance function, @NotNull List<
     @NotNull
     public abstract List<Rex> getArgs();
 
+    @NotNull
     @Override
     protected RexType type() {
         return RexType.of(getFunction().returns);
     }
 
+    @NotNull
     @Override
     protected List<Operand> operands() {
         Operand c0 = Operand.vararg(getArgs());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexCase.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexCase.java
@@ -46,6 +46,7 @@ protected final RexType type() {
         throw new UnsupportedOperationException("Derive type is not implemented");
     }
 
+    @NotNull
     @Override
     protected List<Operand> operands() {
         Rex match = getMatch();

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexCast.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexCast.java
@@ -37,6 +37,7 @@ protected final RexType type() {
         return RexType.of(getTarget());
     }
 
+    @NotNull
     @Override
     protected List<Operand> operands() {
         Operand c0 = Operand.single(getOperand());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexCoalesce.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexCoalesce.java
@@ -25,11 +25,13 @@ public static RexCoalesce create(List<Rex> args) {
     @NotNull
     public abstract List<Rex> getArgs();
 
+    @NotNull
     @Override
     protected final RexType type() {
         throw new UnsupportedOperationException("Derive type is not implemented");
     }
 
+    @NotNull
     @Override
     protected final List<Operand> operands() {
         Operand c0 = Operand.vararg(getArgs());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexDispatch.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexDispatch.java
@@ -42,6 +42,7 @@ protected final RexType type() {
         return RexType.of(PType.dynamic());
     }
 
+    @NotNull
     @Override
     protected final List<Operand> operands() {
         Operand c0 = Operand.vararg(getArgs());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexError.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexError.java
@@ -21,12 +21,14 @@ public static RexError create() {
         return new Impl();
     }
 
+    @NotNull
     @Override
     protected RexType type() {
         // TODO SHOULD BE UNKNOWN
         return RexType.of(PType.dynamic());
     }
 
+    @NotNull
     @Override
     protected List<Operand> operands() {
         return List.of();

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexLit.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexLit.java
@@ -29,6 +29,7 @@ protected final RexType type() {
         return RexType.of(getDatum().getType());
     }
 
+    @NotNull
     @Override
     protected List<Operand> operands() {
         return List.of();

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexNullIf.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexNullIf.java
@@ -41,6 +41,7 @@ protected final RexType type() {
         return getV1().getType();
     }
 
+    @NotNull
     @Override
     protected final List<Operand> operands() {
         Operand c0 = Operand.single(getV1());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexPathIndex.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexPathIndex.java
@@ -36,6 +36,7 @@ protected final RexType type() {
         throw new UnsupportedOperationException("Derive type is not implemented");
     }
 
+    @NotNull
     @Override
     protected final List<Operand> operands() {
         Operand c0 = Operand.single(getOperand());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexPathKey.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexPathKey.java
@@ -37,6 +37,7 @@ protected final RexType type() {
         throw new UnsupportedOperationException("Derive type is not implemented");
     }
 
+    @NotNull
     @Override
     protected List<Operand> operands() {
         Operand c0 = Operand.single(getOperand());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexPathSymbol.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexPathSymbol.java
@@ -37,6 +37,7 @@ protected final RexType type() {
         throw new UnsupportedOperationException("Derive type is not implemented");
     }
 
+    @NotNull
     @Override
     protected final List<Operand> operands() {
         Operand c0 = Operand.single(getOperand());

diff --git a/partiql-plan/src/main/java/org/partiql/plan/rex/RexPivot.java b/partiql-plan/src/main/java/org/partiql/plan/rex/RexPivot.java
@@ -45,6 +45,7 @@ protected final RexType type() {
         return RexType.of(PType.struct());
     }
 
+    @NotNull
     @Override
     protected final List<Operand> operands() {
         Operand c0 = Operand.single(getInput());