Combine multiple “from” relations into a single CrossRelation.

This eliminates some duplication where we define an operation on a CrossRelation and then effectively fold the same operation over the top-level list. It also simplifies some cases where we “knew” the list was non-empty and can now show that in the types.

Author: sellout

src/main/scala/slamdata/engine/compiler.scala

@@ -172,21 +172,18 @@ trait Compiler[F[_]] {
       }
     }
 
-    def buildJoinDirectionMap(relations: List[SqlRelation]): Map[String, List[JoinTraverse.Dir]] = {
+    def buildJoinDirectionMap(relations: SqlRelation): Map[String, List[JoinTraverse.Dir]] = {
       def loop(rel: SqlRelation): JoinTraverse = rel match {
         case t @ TableRelationAST(name, aliasOpt) => JoinTraverse.Leaf(t.aliasName)
         case t @ SubqueryRelationAST(subquery, alias) => JoinTraverse.Leaf(t.aliasName) // Leaf????
         case JoinRelation(left, right, tpe, clause) => JoinTraverse.Join(loop(left), loop(right))
         case CrossRelation(left, right) => JoinTraverse.Join(loop(left), loop(right))
       }
 
-      (relations.map(loop _).foldLeft[Option[JoinTraverse]](None) {
-        case (None, traverse) => Some(traverse)
-        case (Some(acc), traverse) => Some(JoinTraverse.Join(acc, traverse))
-      }).map(_.directionMap).getOrElse(Map())
+      loop(relations).directionMap
     }
 
-    def compileTableRefs(joined: Term[LogicalPlan], relations: List[SqlRelation]): Map[String, Term[LogicalPlan]] = {
+    def compileTableRefs(joined: Term[LogicalPlan], relations: SqlRelation): Map[String, Term[LogicalPlan]] = {
       buildJoinDirectionMap(relations).map {
         case (name, dirs) =>
           name -> dirs.foldLeft(joined) {
@@ -198,11 +195,10 @@ trait Compiler[F[_]] {
       }
     }
 
-    def tableContext(joined: Term[LogicalPlan], relations: List[SqlRelation]): TableContext = {
+    def tableContext(joined: Term[LogicalPlan], relations: SqlRelation): TableContext =
       TableContext(Some(joined), compileTableRefs(joined, relations))
-    }
 
-    def step(relations: List[SqlRelation]): (Option[CompilerM[Term[LogicalPlan]]] => CompilerM[Term[LogicalPlan]] => CompilerM[Term[LogicalPlan]]) = { 
+    def step(relations: SqlRelation): (Option[CompilerM[Term[LogicalPlan]]] => CompilerM[Term[LogicalPlan]] => CompilerM[Term[LogicalPlan]]) = { 
         (current: Option[CompilerM[Term[LogicalPlan]]]) => (next: CompilerM[Term[LogicalPlan]]) => 
       current.map { current =>
         for {
@@ -332,90 +328,85 @@ trait Compiler[F[_]] {
 
         val projs = projections.map(_.expr)
 
-        val stepBuilder = step(relations)
-
-        if (relations.length == 0) for {
-          projs <- projs.map(compile0).sequenceU
-        } yield buildSelectRecord(names, projs)
-        else for {
-          joined <- relations.map(compile0).sequenceU
-
-          crossed = Foldable[List].foldLeftM[CompilerM, Term[LogicalPlan], Term[LogicalPlan]](
-            joined.tail, joined.head
-          )((left, right) => emit[Term[LogicalPlan]](LogicalPlan.invoke(Cross, left :: right :: Nil)))
-
-          rez <- stepBuilder(Some(crossed)) {
-            val filtered = filter map { filter =>
-              for {
-                t <- CompilerState.rootTableReq
-                f <- compile0(filter)
-              } yield Filter(t, f)
-            }
-
-            stepBuilder(filtered) {
-              val grouped = groupBy map { groupBy =>
+        relations match {
+          case None => for {
+            projs <- projs.map(compile0).sequenceU
+          } yield buildSelectRecord(names, projs)
+          case Some(relations) => {
+            val stepBuilder = step(relations)
+            stepBuilder(Some(compile0(relations))) {
+              val filtered = filter map { filter =>
                 for {
                   t <- CompilerState.rootTableReq
-                  g <- compileArray(groupBy.keys)
-                } yield GroupBy(t, g)
+                  f <- compile0(filter)
+                } yield Filter(t, f)
               }
 
-              stepBuilder(grouped) {
-                val having = groupBy.flatMap(_.having) map { having =>
+              stepBuilder(filtered) {
+                val grouped = groupBy map { groupBy =>
                   for {
                     t <- CompilerState.rootTableReq
-                    h <- compile0(having)
-                  } yield Filter(t, h)
+                    g <- compileArray(groupBy.keys)
+                  } yield GroupBy(t, g)
                 }
 
-                stepBuilder(having) {
-                  val select = Some {
+                stepBuilder(grouped) {
+                  val having = groupBy.flatMap(_.having) map { having =>
                     for {
-                      projs <- projs.map(compile0).sequenceU
-                    } yield buildSelectRecord(names, projs)
+                      t <- CompilerState.rootTableReq
+                      h <- compile0(having)
+                    } yield Filter(t, h)
                   }
 
-                  stepBuilder(select) {
-                    val sort = orderBy map { orderBy =>
+                  stepBuilder(having) {
+                    val select = Some {
                       for {
-                        t <- CompilerState.rootTableReq
-                        s <- compileArray(orderBy.keys.map(_._1)) // TODO: Ascending / descending
-                      } yield OrderBy(t, s)
+                        projs <- projs.map(compile0).sequenceU
+                      } yield buildSelectRecord(names, projs)
                     }
 
-                    stepBuilder(sort) {
-                      // Note: inspecting the name is not the most awesome imaginable way to identify 
-                      // fields that were introduced to support "oredr by"
-                      val synthetic: Option[String] => Boolean = {
-                        case Some(name) => name.startsWith("__sd__")
-                        case None => false
+                    stepBuilder(select) {
+                      val sort = orderBy map { orderBy =>
+                        for {
+                          t <- CompilerState.rootTableReq
+                          s <- compileArray(orderBy.keys.map(_._1)) // TODO: Ascending / descending
+                        } yield OrderBy(t, s)
                       }
-                      
-                      val pruned = if (names.exists(synthetic)) 
-                        Some {
-                          for {
-                            t <- CompilerState.rootTableReq
-                            ns = names.collect {
-                              case Some(name) if !synthetic(Some(name)) => name
-                            }
-                            ts = ns.map(name => ObjectProject(t, LogicalPlan.constant(Data.Str(name))))
-                          } yield buildSelectRecord(ns.map(name => Some(name)), ts)
-                        } 
-                        else None
-                      
-                      stepBuilder(pruned) {
-                        val drop = offset map { offset =>
-                          for {
-                            t <- CompilerState.rootTableReq
-                          } yield Drop(t, LogicalPlan.constant(Data.Int(offset)))
-                        }
 
-                        stepBuilder(drop) {
-                          (limit map { limit =>
+                      stepBuilder(sort) {
+                        // Note: inspecting the name is not the most awesome imaginable way to identify
+                        // fields that were introduced to support "oredr by"
+                        val synthetic: Option[String] => Boolean = {
+                          case Some(name) => name.startsWith("__sd__")
+                          case None => false
+                        }
+                        
+                        val pruned = if (names.exists(synthetic))
+                          Some {
+                            for {
+                              t <- CompilerState.rootTableReq
+                              ns = names.collect {
+                                case Some(name) if !synthetic(Some(name)) => name
+                              }
+                              ts = ns.map(name => ObjectProject(t, LogicalPlan.constant(Data.Str(name))))
+                            } yield buildSelectRecord(ns.map(name => Some(name)), ts)
+                          }
+                          else None
+                        
+                        stepBuilder(pruned) {
+                          val drop = offset map { offset =>
                             for {
                               t <- CompilerState.rootTableReq
-                            } yield Take(t, LogicalPlan.constant(Data.Int(limit)))
-                          }).getOrElse(CompilerState.rootTableReq)
+                            } yield Drop(t, LogicalPlan.constant(Data.Int(offset)))
+                          }
+
+                          stepBuilder(drop) {
+                            (limit map { limit =>
+                              for {
+                                t <- CompilerState.rootTableReq
+                              } yield Take(t, LogicalPlan.constant(Data.Int(limit)))
+                            }).getOrElse(CompilerState.rootTableReq)
+                          }
                         }
                       }
                     }
@@ -424,7 +415,7 @@ trait Compiler[F[_]] {
               }
             }
           }
-        } yield rez
+        }
 
       case Subselect(select) => compile0(select)
 
@@ -512,7 +503,7 @@ trait Compiler[F[_]] {
 
       case SubqueryRelationAST(subquery, _) => compile0(subquery)
 
-      case JoinRelation(left, right, tpe, clause) =>
+      case JoinRelation(left, right, tpe, clause) => 
         for {
           leftName <- CompilerState.freshName("left")
           rightName <- CompilerState.freshName("right")
@@ -521,8 +512,8 @@ trait Compiler[F[_]] {
           left0 <- compile0(left)
           right0 <- compile0(right)
           join <- CompilerState.contextual(
-            tableContext(leftFree, List(left))
-              ++ tableContext(rightFree, List(right))) {
+            tableContext(leftFree, left) ++ tableContext(rightFree, right)
+          ) {
             for {
               tuple  <- compileJoin(clause)
             } yield LogicalPlan.join(leftFree, rightFree,

src/main/scala/slamdata/engine/semantics.scala

@@ -7,6 +7,7 @@ import scalaz._
 import scalaz.std.map._
 import scalaz.std.string._
 import scalaz.std.list._
+import scalaz.std.option._
 import scalaz.std.set._
 
 import scalaz.syntax.apply._

src/main/scala/slamdata/engine/sql/ast.scala

@@ -18,7 +18,7 @@ trait NodeInstances {
 object Node extends NodeInstances
 
 final case class SelectStmt(projections:  List[Proj],
-                            relations:    List[SqlRelation],
+                            relations:    Option[SqlRelation],
                             filter:       Option[Expr],
                             groupBy:      Option[GroupBy],
                             orderBy:      Option[OrderBy],

src/main/scala/slamdata/engine/sql/parser.scala

@@ -12,6 +12,8 @@ import scala.util.parsing.combinator.token._
 import scala.util.parsing.input.CharArrayReader.EofCh
 
 import scalaz._
+import scalaz.std.option._
+import scalaz.syntax.bind._
 
 case class Query(value: String)
 
@@ -72,7 +74,7 @@ class SQLParser extends StandardTokenParsers {
     keyword("select") ~> projections ~
       opt(relations) ~ opt(filter) ~
       opt(group_by) ~ opt(order_by) ~ opt(limit) ~ opt(offset) <~ opt(op(";")) ^^ {
-    case p ~ r ~ f ~ g ~ o ~ l ~ off => SelectStmt(p, r.getOrElse(Nil), f, g, o, l, off)
+    case p ~ r ~ f ~ g ~ o ~ l ~ off => SelectStmt(p, r.join, f, g, o, l, off)
   }
 
   def projections: Parser[List[Proj]] = repsep(projection, op(",")).map(_.toList)
@@ -206,7 +208,11 @@ class SQLParser extends StandardTokenParsers {
     stringLit ^^ { case s => StringLiteral(s) } |
     keyword("null") ^^^ NullLiteral.apply
 
-  def relations: Parser[List[SqlRelation]] = keyword("from") ~> rep1sep(relation, op(",")).map(_.toList)
+  def relations: Parser[Option[SqlRelation]] =
+    keyword("from") ~> rep1sep(relation, op(",")).map(_.foldLeft[Option[SqlRelation]](None) {
+      case (None, traverse) => Some(traverse)
+      case (Some(acc), traverse) => Some(CrossRelation(acc, traverse))
+    })
 
   def std_join_relation: Parser[SqlRelation => SqlRelation] = 
     opt(join_type) ~ keyword("join") ~ simple_relation ~ keyword("on") ~ expr ^^

src/test/scala/slamdata/engine/compiler.scala

@@ -560,22 +560,22 @@ class CompilerSpec extends Specification with CompilerHelpers {
     "compile simple inner equi-join" in {
       testLogicalPlanCompile(
         "select foo.name, bar.address from foo join bar on foo.id = bar.foo_id",
-        letOne('tmp2,
-               let(Map('left0 -> read("foo"),
-                       'right1 -> read("bar")),
-                 join(free('left0), free('right1),
+        letOne('tmp0,
+               let(Map('left1 -> read("foo"),
+                       'right2 -> read("bar")),
+                 join(free('left1), free('right2),
                       JoinType.Inner, JoinRel.Eq,
-                      ObjectProject(free('left0), constant(Data.Str("id"))),
-                      ObjectProject(free('right1), constant(Data.Str("foo_id"))))),
+                      ObjectProject(free('left1), constant(Data.Str("id"))),
+                      ObjectProject(free('right2), constant(Data.Str("foo_id"))))),
           letOne('tmp3,
                  makeObj(
                    "name" ->
                      ObjectProject(
-                       ObjectProject(free('tmp2), constant(Data.Str("left"))),
+                       ObjectProject(free('tmp0), constant(Data.Str("left"))),
                        constant(Data.Str("name"))),
                    "address" ->
                      ObjectProject(
-                       ObjectProject(free('tmp2), constant(Data.Str("right"))),
+                       ObjectProject(free('tmp0), constant(Data.Str("right"))),
                        constant(Data.Str("address")))),
             free('tmp3))))
     }