[MOREL-216] Allow comma-separated scans in join, and on in the `f…

…rom` clause

The following is now legal:
  from a in [1, 2],
      b in [3, 4, 5] on a + b = 6
  where b < 5
  join c in [6, 7] on b + c = 10,
      d in [7, 8]

An `on` after the first scan, `from a in [1, 2]`, was and
remains illegal.

Remove Op.INNER_JOIN, because its semantics is identical to Op.SCAN.
(We will add operators back if and when we support `left join`.)

Fixes #216

docs/reference.md

@@ -139,18 +139,20 @@ In Standard ML but not in Morel:
                                 conditional
     | <b>case</b> <i>exp</i> <b>of</b> <i>match</i>         case analysis
     | <b>fn</b> <i>match</i>                  function
-    | <b>from</b> [ <i>scan<sub>1</sub></i>  <b>,</b> ... <b>,</b> <i>scan<sub>s</sub></i> ] <i>step</i>*
+    | <b>from</b> [ <i>scan<sub>1</sub></i> <b>,</b> ... <b>,</b> <i>scan<sub>s</sub></i> ] <i>step</i>*
                                 relational expression (<i>s</i> &ge; 0)
 <i>exprow</i> &rarr; <i>exprowItem</i> [<b>,</b> <i>exprowItem</i> ]*
                                 expression row
 <i>exprowItem</i> &rarr; [ <i>lab</i> <b>=</b> ] <i>exp</i>
 <i>match</i> &rarr; <i>matchItem</i> [ '<b>|</b>' <i>matchItem</i> ]*
                                 match
 <i>matchItem</i> &rarr; <i>pat</i> <b>=&gt;</b> <i>exp</i>
-<i>scan</i> &rarr; <i>pat</i> [ <b>in</b> | <b>=</b> ] <i>exp</i>
+<i>scan</i> &rarr; <i>pat</i> <b>in</b> <i>exp</i> [ <b>on</b> <i>exp</i> ]    iteration
+    | <i>pat</i> <b>=</b> <i>exp</i> [ <b>on</b> <i>exp</i> ]      single iteration
     | <i>var</i>                       unbounded variable
 <i>step</i> &rarr; <b>where</b> <i>exp</i>                filter clause
-    | <b>join</b> <i>scan</i> [ <b>on</b> <i>exp</i> ]      join clause
+    | <b>join</b> <i>scan<sub>1</sub></i> [ <b>,</b> ... <b>,</b> <i>scan<sub>s</sub></i> ]
+                                join clause
     | <b>group</b> <i>groupKey<sub>1</sub></i> <b>,</b> ... <b>,</b> <i>groupKey<sub>g</sub></i>
       [ <b>compute</b> <i>agg<sub>1</sub></i> <b>,</b> ... <b>,</b> <i>agg<sub>a</sub></i> ]
                                 group clause (<i>g</i> &ge; 0, <i>a</i> &ge; 1)

src/main/java/net/hydromatic/morel/ast/Ast.java

@@ -1509,7 +1509,6 @@ && getLast(steps) instanceof Ast.Yield) {
       final Set<Id> nextFields = new HashSet<>();
       for (FromStep step : steps) {
         switch (step.op) {
-        case INNER_JOIN:
         case SCAN:
           final Scan scan = (Scan) step;
           nextFields.clear();
@@ -1578,8 +1577,12 @@ public Exp accept(Shuttle shuttle) {
       } else {
         w.append("from");
         forEachIndexed(steps, (step, i) -> {
-          if (step.op == Op.SCAN && i > 0 && steps.get(i - 1).op == Op.SCAN) {
-            w.append(",");
+          if (step.op == Op.SCAN && i > 0) {
+            if (steps.get(i - 1).op == Op.SCAN) {
+              w.append(",");
+            } else {
+              w.append(" join");
+            }
           }
           step.unparse(w, 0, 0);
         });
@@ -1620,17 +1623,8 @@ public static class Scan extends FromStep {
     public final @Nullable Exp exp;
     public final @Nullable Exp condition;
 
-    Scan(Pos pos, Op op, Pat pat, @Nullable Exp exp, @Nullable Exp condition) {
-      super(pos, op);
-      switch (op) {
-      case INNER_JOIN:
-        break;
-      case SCAN:
-        checkArgument(condition == null);
-        break;
-      default:
-        throw new AssertionError("not a join type " + op);
-      }
+    Scan(Pos pos, Pat pat, @Nullable Exp exp, @Nullable Exp condition) {
+      super(pos, Op.SCAN);
       this.pat = pat;
       this.exp = exp;
       this.condition = condition;
@@ -1668,7 +1662,7 @@ public Scan copy(Pat pat, @Nullable Exp exp, @Nullable Exp condition) {
           && Objects.equals(this.exp, exp)
           && Objects.equals(this.condition, condition)
           ? this
-          : new Scan(pos, op, pat, exp, condition);
+          : new Scan(pos, pat, exp, condition);
     }
   }
 

src/main/java/net/hydromatic/morel/ast/AstBuilder.java

@@ -457,9 +457,9 @@ public Ast.Exp map(Pos pos, Ast.Exp e1, Ast.Exp e2) {
     return apply(apply(ref(pos, BuiltIn.LIST_MAP), e1), e2);
   }
 
-  public Ast.Scan scan(Pos pos, Op op, Ast.Pat pat, Ast.Exp exp,
+  public Ast.Scan scan(Pos pos, Ast.Pat pat, Ast.Exp exp,
       @Nullable Ast.Exp condition) {
-    return new Ast.Scan(pos, op, pat, exp, condition);
+    return new Ast.Scan(pos, pat, exp, condition);
   }
 
   public Ast.Order order(Pos pos, Iterable<Ast.OrderItem> orderItems) {

src/main/java/net/hydromatic/morel/ast/Core.java

@@ -1187,13 +1187,9 @@ public static class Scan extends FromStep {
     public final Exp exp;
     public final Exp condition;
 
-    Scan(Op op, ImmutableList<Binding> bindings, Pat pat, Exp exp,
+    Scan(ImmutableList<Binding> bindings, Pat pat, Exp exp,
         Exp condition) {
-      super(op, bindings);
-      if (op != Op.INNER_JOIN) {
-        // SCAN and CROSS_JOIN are valid in ast, not core.
-        throw new IllegalArgumentException("not a join type " + op);
-      }
+      super(Op.SCAN, bindings);
       this.pat = requireNonNull(pat, "pat");
       this.exp = requireNonNull(exp, "exp");
       this.condition = requireNonNull(condition, "condition");
@@ -1224,7 +1220,7 @@ private static boolean canAssign(Type fromType, Type toType) {
 
     @Override protected AstWriter unparse(AstWriter w, From from, int ordinal,
         int left, int right) {
-      w.append(ordinal == 0 ? " " : op.padded)
+      w.append(ordinal == 0 ? " " : " join ")
           // for these purposes 'in' has same precedence as '='
           .append(pat, 0, Op.EQ.left);
       if (Extents.isInfinite(exp)) {
@@ -1252,7 +1248,7 @@ public Scan copy(List<Binding> bindings, Pat pat, Exp exp, Exp condition) {
           && condition == this.condition
           && bindings.equals(this.bindings)
           ? this
-          : core.scan(op, bindings, pat, exp, condition);
+          : core.scan(bindings, pat, exp, condition);
     }
   }
 

src/main/java/net/hydromatic/morel/ast/CoreBuilder.java

@@ -415,8 +415,7 @@ public Core.Exp implicitYieldExp(TypeSystem typeSystem,
     }
   }
 
-  public static List<Binding> lastBindings(
-      List<? extends Core.FromStep> steps) {
+  public List<Binding> lastBindings(List<? extends Core.FromStep> steps) {
     return steps.isEmpty()
         ? ImmutableList.of()
         : Iterables.getLast(steps).bindings;
@@ -492,10 +491,9 @@ public Core.DatatypeDecl datatypeDecl(Iterable<DataType> dataTypes) {
     return new Core.DatatypeDecl(ImmutableList.copyOf(dataTypes));
   }
 
-  public Core.Scan scan(Op op, List<Binding> bindings, Core.Pat pat,
+  public Core.Scan scan(List<Binding> bindings, Core.Pat pat,
       Core.Exp exp, Core.Exp condition) {
-    return new Core.Scan(op, ImmutableList.copyOf(bindings), pat, exp,
-        condition);
+    return new Core.Scan(ImmutableList.copyOf(bindings), pat, exp, condition);
   }
 
   public Core.Aggregate aggregate(Type type, Core.Exp aggregate,

src/main/java/net/hydromatic/morel/ast/FromBuilder.java

@@ -201,7 +201,7 @@ && getLast(((Core.From) exp).steps).bindings.size() == 1
       return yield_(uselessIfLast, bindings, core.record(typeSystem, nameExps));
     }
     Compiles.acceptBinding(typeSystem, pat, bindings);
-    return addStep(core.scan(Op.INNER_JOIN, bindings, pat, exp, condition));
+    return addStep(core.scan(bindings, pat, exp, condition));
   }
 
   public FromBuilder addAll(Iterable<? extends Core.FromStep> steps) {
@@ -367,7 +367,7 @@ private Core.Exp build(boolean simplify) {
     }
     if (simplify
         && steps.size() == 1
-        && steps.get(0).op == Op.INNER_JOIN) {
+        && steps.get(0).op == Op.SCAN) {
       final Core.Scan scan = (Core.Scan) steps.get(0);
       if (scan.pat.op == Op.ID_PAT) {
         return scan.exp;

src/main/java/net/hydromatic/morel/ast/Op.java

@@ -130,7 +130,6 @@ public enum Op {
   CASE,
   FROM,
   SCAN(" "),
-  INNER_JOIN(" join "),
   WHERE,
   GROUP,
   COMPUTE,

src/main/java/net/hydromatic/morel/ast/Shuttle.java

@@ -231,7 +231,7 @@ protected AstNode visit(Ast.OrderItem orderItem) {
   }
 
   protected Ast.Scan visit(Ast.Scan scan) {
-    return ast.scan(scan.pos, scan.op, scan.pat.accept(this),
+    return ast.scan(scan.pos, scan.pat.accept(this),
         scan.exp.accept(this),
         scan.condition == null ? null : scan.condition.accept(this));
   }

src/main/java/net/hydromatic/morel/compile/CalciteCompiler.java

@@ -404,7 +404,7 @@ private static void harmonizeRowTypes(RelBuilder relBuilder, int inputCount) {
 
       private RelContext step(RelContext cx, int i, Core.FromStep fromStep) {
         switch (fromStep.op) {
-        case INNER_JOIN:
+        case SCAN:
           return join(cx, i, (Core.Scan) fromStep);
         case WHERE:
           return where(cx, (Core.Where) fromStep);
@@ -687,7 +687,7 @@ private RelContext join(RelContext cx, int i, Core.Scan scan) {
 
   private static JoinRelType joinRelType(Op op) {
     switch (op) {
-    case INNER_JOIN:
+    case SCAN:
       return JoinRelType.INNER;
     default:
       throw new AssertionError(op);

src/main/java/net/hydromatic/morel/compile/Compiler.java

@@ -329,7 +329,7 @@ && getOnlyElement(bindings).id.type.equals(elementType)) {
         createRowSinkFactory(cx, firstStep.bindings, skip(steps),
             elementType);
     switch (firstStep.op) {
-    case INNER_JOIN:
+    case SCAN:
       final Core.Scan scan = (Core.Scan) firstStep;
       final Code code = compile(cx, scan.exp);
       final Code conditionCode = compile(cx, scan.condition);

src/main/java/net/hydromatic/morel/compile/Relationalizer.java

@@ -108,7 +108,7 @@ private Core.From toFrom(Core.Exp exp) {
       final List<Binding> bindings = new ArrayList<>();
       Compiles.acceptBinding(typeSystem, id, bindings);
       final Core.Scan scan =
-          core.scan(Op.INNER_JOIN, bindings, id, exp, core.boolLiteral(true));
+          core.scan(bindings, id, exp, core.boolLiteral(true));
       return core.from(typeSystem, ImmutableList.of(scan));
     }
   }

src/main/java/net/hydromatic/morel/compile/SuchThatShuttle.java

@@ -110,7 +110,6 @@ Core.From visit(Core.From from) {
         final Core.FromStep step = steps.get(i);
         switch (step.op) {
         case SCAN:
-        case INNER_JOIN:
           final Core.Scan scan = (Core.Scan) step;
           if (Extents.isInfinite(scan.exp)) {
             final int idPatCount = idPats.size();

src/main/java/net/hydromatic/morel/compile/TypeResolver.java

@@ -480,7 +480,6 @@ private Pair<TypeEnv, Unifier.Variable> deduceStepType(TypeEnv env,
       Map<Ast.Id, Unifier.Variable> fieldVars, List<Ast.FromStep> fromSteps) {
     switch (step.op) {
     case SCAN:
-    case INNER_JOIN:
       final Ast.Scan scan = (Ast.Scan) step;
       final Ast.Exp scanExp;
       final boolean eq;

src/main/java/net/hydromatic/morel/eval/Codes.java

@@ -3092,7 +3092,7 @@ static class ScanRowSink extends BaseRowSink {
     ScanRowSink(Op op, Core.Pat pat, Code code, Code conditionCode,
         RowSink rowSink) {
       super(rowSink);
-      checkArgument(op == Op.INNER_JOIN);
+      checkArgument(op == Op.SCAN);
       this.op = op;
       this.pat = pat;
       this.code = code;
@@ -3101,8 +3101,7 @@ static class ScanRowSink extends BaseRowSink {
 
     @Override public Describer describe(Describer describer) {
       return describer.start("join", d ->
-          d.arg("op", op.padded.trim())
-              .arg("pat", pat)
+          d.arg("pat", pat)
               .arg("exp", code)
               .argIf("condition", conditionCode, !isConstantTrue(conditionCode))
               .arg("sink", rowSink));

src/main/javacc/MorelParser.jj

@@ -330,16 +330,16 @@ Exp from() :
 }
 {
   <FROM> { span = Span.of(pos()); }
-  [ fromFirstStep(steps)
-    ( <COMMA> fromFirstStep(steps) )*
+  [ fromFirstScan(steps)
+    ( <COMMA> fromScan(steps) )*
   ]
   ( fromStep(steps) )*
   {
     return ast.from(span.end(this), steps);
   }
 }
 
-void fromFirstStep(List<FromStep> steps) :
+void fromFirstScan(List<FromStep> steps) :
 {
   final Pair<Pat, Exp> patExp;
 }
@@ -349,41 +349,49 @@ void fromFirstStep(List<FromStep> steps) :
         patExp.right != null
           ? Span.of(patExp.left, patExp.right)
           : Span.of(patExp.left);
-    steps.add(ast.scan(span.pos(), Op.SCAN, patExp.left, patExp.right, null));
+    steps.add(ast.scan(span.pos(), patExp.left, patExp.right, null));
   }
 }
 
-void fromStep(List<FromStep> steps) :
+void fromScan(List<FromStep> steps) :
 {
-  final Span span;
-  final Op op;
   final Pair<Pat, Exp> patExp;
   final Exp condition;
-  final Exp filterExp;
-  final Exp skipExp;
-  final Exp takeExp;
-  final Exp yieldExp;
-  final PairList<Id, Exp> groupExps;
-  final List<Aggregate> aggregates;
-  final List<OrderItem> orderItems;
 }
 {
-  <JOIN> {
-  span = Span.of(pos());
-  op = Op.INNER_JOIN;
-}
-  patExp = fromSource()
+  patExp = fromSource() {
+    final Span span =
+        patExp.right != null
+          ? Span.of(patExp.left, patExp.right)
+          : Span.of(patExp.left);
+  }
   (
     LOOKAHEAD(2)
     <ON> condition = expression()
   |
     { condition = null; }
   )
   {
-    steps.add(
-        ast.scan(span.end(this), op, patExp.left, patExp.right,
-            condition));
+    steps.add(ast.scan(span.end(this), patExp.left, patExp.right, condition));
   }
+}
+
+void fromStep(List<FromStep> steps) :
+{
+  final Span span;
+  final Op op;
+  final Exp filterExp;
+  final Exp skipExp;
+  final Exp takeExp;
+  final Exp yieldExp;
+  final PairList<Id, Exp> groupExps;
+  final List<Aggregate> aggregates;
+  final List<OrderItem> orderItems;
+}
+{
+  <JOIN>
+  fromScan(steps)
+  ( <COMMA> fromScan(steps) )*
 |
   <WHERE> { span = Span.of(pos()); } filterExp = expression() {
     steps.add(ast.where(span.end(this), filterExp));