[MOREL-159] from should not have a singleton record type unless it en…

…ds with a singleton record yield

If a `from` expression has one variable named `i` of type
`int`, should the type of the returned elements be `int` or
`{i: int}`? (We call `int` a scalar, `{i: int}` a singleton
record type, `yield {i = i}` a singleton record yield, and
`yield {i = j, j = k}` a renaming yield.)

After this change, `from` will have a singleton record type
only it ends with a singleton record yield. If it does not
end in yield, the type depends on N, the number of pipeline
variables, and will be a scalar if N = 1 and a record with N
fields if N != 1.

As part of this change, we introduce a new `class FromBuilder`
to safely build `Core.From` pipelines. It performs
micro-optimizations as it goes, such as removing `where true`,
empty `order` and trivial `yield` steps. `FromBuilder` can
also inline nested from expressions:

  from i in (from j in [1, 2, 3]
      where j > 1)
    where i < 3

becomes

  from j in [1, 2, 3]
    where j > 1
    yield {i = j}
    where i < 3

Note the use of `yield {i = j}` to handle the variable name
change caused by inlining.

Ensure that `from d in scott.depts` is not printed as
'<relation>' even if it is optimized to `scott.depts`. If we
write `scott.depts` in the shell, the shell prints
'<relation>' because `depts` may be a large table.
`FromBuilder` now simplies `from d in scott.depts` to
`scott.depts`, but we want the shell to print the rows, not
'<relation>'. Therefore the shell now calls `Code.wrap` to
tag whether an expression would be treated as a relation or
as a query.

Fixes hydromatic#159

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

@@ -1502,7 +1502,6 @@ && getLast(steps) instanceof Ast.Yield) {
         return null;
       }
       Set<Id> fields = ImmutableSet.of();
-      boolean record = true;
       final Set<Id> nextFields = new HashSet<>();
       for (FromStep step : steps) {
         switch (step.op) {
@@ -1517,7 +1516,6 @@ boolean record = true;
             }
           });
           fields = ImmutableSet.copyOf(nextFields);
-          record = nextFields.size() != 1;
           break;
 
         case COMPUTE:
@@ -1535,7 +1533,6 @@ record = nextFields.size() != 1;
           groupExps.forEach(pair -> nextFields.add(pair.left));
           aggregates.forEach(aggregate -> nextFields.add(aggregate.id));
           fields = nextFields;
-          record = fields.size() != 1;
           break;
 
         case YIELD:
@@ -1546,14 +1543,15 @@ record = fields.size() != 1;
                     .stream()
                     .map(label -> ast.id(Pos.ZERO, label))
                     .collect(Collectors.toSet());
-            record = true;
-          } else {
-            record = false;
           }
           break;
         }
       }
-      if (!record) {
+
+      if (fields.size() == 1
+          && (steps.isEmpty()
+          || getLast(steps).op != Op.YIELD
+          || ((Yield) getLast(steps)).exp.op != Op.RECORD)) {
         return Iterables.getOnlyElement(fields);
       } else {
         final SortedMap<String, Exp> map = new TreeMap<>(ORDERING);

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

@@ -482,7 +482,7 @@ public Type type() {
    * value. What would be an {@code Id} in Ast is often a {@link String} in
    * Core; for example, compare {@link Ast.Con0Pat#tyCon}
    * with {@link Con0Pat#tyCon}. */
-  public static class Id extends Exp {
+  public static class Id extends Exp implements Comparable<Id> {
     public final NamedPat idPat;
 
     /** Creates an Id. */
@@ -491,6 +491,10 @@ public static class Id extends Exp {
       this.idPat = requireNonNull(idPat);
     }
 
+    @Override public int compareTo(Id o) {
+      return idPat.compareTo(o.idPat);
+    }
+
     @Override public int hashCode() {
       return idPat.hashCode();
     }
@@ -768,6 +772,17 @@ public static class Tuple extends Exp {
       this.args = ImmutableList.copyOf(args);
     }
 
+    @Override public boolean equals(Object o) {
+      return this == o
+          || o instanceof Tuple
+          && args.equals(((Tuple) o).args)
+          && type.equals(((Tuple) o).type);
+    }
+
+    @Override public int hashCode() {
+      return Objects.hash(args, type);
+    }
+
     @Override public RecordLikeType type() {
       return (RecordLikeType) type;
     }
@@ -977,6 +992,16 @@ public static class From extends Exp {
       this.steps = requireNonNull(steps);
     }
 
+    @Override public boolean equals(Object o) {
+      return this == o
+          || o instanceof From
+          && steps.equals(((From) o).steps);
+    }
+
+    @Override public int hashCode() {
+      return steps.hashCode();
+    }
+
     @Override public ListType type() {
       return (ListType) type;
     }
@@ -1002,7 +1027,7 @@ public static class From extends Exp {
     public Exp copy(TypeSystem typeSystem, List<FromStep> steps) {
       return steps.equals(this.steps)
           ? this
-          : core.from(type(), steps);
+          : core.fromBuilder(typeSystem).addAll(steps).build();
     }
   }
 

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

@@ -336,21 +336,26 @@ public Core.From from(ListType type, List<Core.FromStep> steps) {
   /** Derives the result type, then calls
    * {@link #from(ListType, List)}. */
   public Core.From from(TypeSystem typeSystem, List<Core.FromStep> steps) {
-    final Type elementType;
-    if (!steps.isEmpty() && Iterables.getLast(steps) instanceof Core.Yield) {
-      elementType = ((Core.Yield) Iterables.getLast(steps)).exp.type;
+    final Type elementType = fromElementType(typeSystem, steps);
+    return from(typeSystem.listType(elementType), steps);
+  }
+
+  /** Returns the element type of a {@link Core.From} with the given steps. */
+  static Type fromElementType(TypeSystem typeSystem,
+      List<Core.FromStep> steps) {
+    if (!steps.isEmpty()
+        && Iterables.getLast(steps) instanceof Core.Yield) {
+      return ((Core.Yield) Iterables.getLast(steps)).exp.type;
     } else {
       final List<Binding> lastBindings = core.lastBindings(steps);
       if (lastBindings.size() == 1) {
-        elementType = getOnlyElement(lastBindings).id.type;
-      } else {
-        final SortedMap<String, Type> argNameTypes = new TreeMap<>(ORDERING);
-        lastBindings.forEach(b -> argNameTypes.put(b.id.name, b.id.type));
-        elementType = typeSystem.recordType(argNameTypes);
+        return lastBindings.get(0).id.type;
       }
+      final SortedMap<String, Type> argNameTypes = new TreeMap<>(ORDERING);
+      lastBindings
+          .forEach(b -> argNameTypes.put(b.id.name, b.id.type));
+      return typeSystem.recordType(argNameTypes);
     }
-    final ListType type = typeSystem.listType(elementType);
-    return from(type, ImmutableList.copyOf(steps));
   }
 
   /** Returns what would be the yield expression if we created a
@@ -385,6 +390,11 @@ public List<Binding> lastBindings(List<? extends Core.FromStep> steps) {
         : Iterables.getLast(steps).bindings;
   }
 
+  /** Creates a builder that will create a {@link Core.From}. */
+  public FromBuilder fromBuilder(TypeSystem typeSystem) {
+    return new FromBuilder(typeSystem);
+  }
+
   public Core.Fn fn(FnType type, Core.IdPat idPat, Core.Exp exp) {
     return new Core.Fn(type, idPat, exp);
   }