[MOREL-208] FromBuilder should remove trivial yield step between two …

…scan steps Fixes #208
hydromatic · Dec 7, 2023 · 0f06f35 · 0f06f35
1 parent e420ca1
commit 0f06f35
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 6 deletions.
diff --git a/src/main/java/net/hydromatic/morel/ast/FromBuilder.java b/src/main/java/net/hydromatic/morel/ast/FromBuilder.java
@@ -105,7 +105,10 @@ private FromBuilder addStep(Core.FromStep step) {
         final Core.Yield yield = (Core.Yield) lastStep;
         if (yield.exp.op == Op.TUPLE) {
           final Core.Tuple tuple = (Core.Tuple) yield.exp;
-          if (tuple.args.size() == 1 && isTrivial(tuple, yield.bindings)) {
+          final Core.FromStep previousStep = steps.get(steps.size() - 2);
+          final List<Binding> previousBindings = previousStep.bindings;
+          if (tuple.args.size() == 1
+              && isTrivial(tuple, previousBindings, yield.bindings)) {
             steps.remove(steps.size() - 1);
           }
         }
@@ -239,7 +242,8 @@ public FromBuilder yield_(boolean uselessIfLast,
     boolean uselessIfNotLast = false;
     switch (exp.op) {
     case TUPLE:
-      final TupleType tupleType = tupleType((Core.Tuple) exp, bindings2);
+      final TupleType tupleType =
+          tupleType((Core.Tuple) exp, bindings, bindings2);
       switch (tupleType) {
       case IDENTITY:
         // A trivial record does not rename, so its only purpose is to change
@@ -285,13 +289,13 @@ public FromBuilder yield_(boolean uselessIfLast,
   }
 
   /** Returns whether tuple is something like "{i = i, j = j}". */
-  private boolean isTrivial(Core.Tuple tuple,
+  private static boolean isTrivial(Core.Tuple tuple, List<Binding> bindings,
       @Nullable List<Binding> bindings2) {
-    return tupleType(tuple, bindings2) == TupleType.IDENTITY;
+    return tupleType(tuple, bindings, bindings2) == TupleType.IDENTITY;
   }
 
   /** Returns whether tuple is something like "{i = i, j = j}". */
-  private TupleType tupleType(Core.Tuple tuple,
+  private static TupleType tupleType(Core.Tuple tuple, List<Binding> bindings,
       @Nullable List<Binding> bindings2) {
     if (tuple.args.size() != bindings.size()) {
       return TupleType.OTHER;
@@ -328,7 +332,7 @@ private Core.Exp build(boolean simplify) {
       final Core.Yield yield = (Core.Yield) getLast(steps);
       assert yield.exp.op == Op.TUPLE
           && ((Core.Tuple) yield.exp).args.size() == 1
-          && isTrivial((Core.Tuple) yield.exp, yield.bindings)
+          && isTrivial((Core.Tuple) yield.exp, bindings, yield.bindings)
           : yield.exp;
       steps.remove(steps.size() - 1);
     }

diff --git a/src/test/java/net/hydromatic/morel/FromBuilderTest.java b/src/test/java/net/hydromatic/morel/FromBuilderTest.java
@@ -145,6 +145,24 @@ Core.Exp record(Core.Id... ids) {
     assertThat(e, is(from));
   }
 
+  @Test void testTrivialYield3() {
+    // from j in [1, 2] yield {j} join i in [3, 4]
+    //   ==>
+    // from j in [1, 2] join i in [3, 4]
+    final Fixture f = new Fixture();
+    final FromBuilder fromBuilder = core.fromBuilder(f.typeSystem);
+    fromBuilder.scan(f.jPat, f.list12)
+        .yield_(f.record(f.jId))
+        .scan(f.iPat, f.list34);
+
+    final Core.From from = fromBuilder.build();
+    final String expected = "from j in [1, 2] "
+        + "join i in [3, 4]";
+    assertThat(from.toString(), is(expected));
+    final Core.Exp e = fromBuilder.buildSimplify();
+    assertThat(e, is(from));
+  }
+
   @Test void testNested() {
     // from i in (from j in [1, 2] where j < 2) where i > 1
     //   ==>