Permalink
Browse files

Transitively pull join conditions out in `rewriteJoins`

`rearrangeJoinConditions` already performed the transformation
recursively but did not recognize symbols that need to be pulled
out through multiple Joins.

Test in JoinTest.testJoin. Fixes #1316.
  • Loading branch information...
szeiger committed Oct 21, 2015
1 parent 73a0166 commit 988f87deb44faebcfa43ac9ffa69916f4b7f85f5
@@ -51,6 +51,14 @@ class JoinTest extends AsyncTest[RelationalTestDB] {
_ <- q2.map(p => (p._1, p._2)).result.map(_ shouldBe List((2,1), (3,2), (4,3), (5,2)))
q3 = posts.flatMap(_.withCategory)
_ <- mark("q3", q3.result).map(_ should (_.length == 20))
q4 = (for {
a1 <- categories
a2 <- categories
a3 <- categories
a4 <- categories
if a1.id === a4.id
} yield a1.id).to[Set]
_ <- mark("q4", q4.result).map(_ shouldBe Set(1, 2, 3, 4))
} yield ()
}
@@ -57,7 +57,7 @@ class RewriteJoins extends Phase {
}
val (oj2, invalid) = hoistFilters(oj)
val (oj3, m) = eliminateIllegalRefs(oj2, Set.empty, sn)
val oj4 = rearrangeJoinConditions(oj3)
val oj4 = rearrangeJoinConditions(oj3, Set.empty)
val sel3 = if(m.isEmpty) sel2 else sel2.replace {
case p @ FwdPath(r1 :: rest) if r1 == sn && m.contains(rest) => m(rest)
case r @ Ref(s) if (oj4 ne oj2) && s == sn => r.untyped // Structural expansion may have changed
@@ -155,7 +155,7 @@ class RewriteJoins extends Phase {
}
}
/** Recursively push refs from the right-hand side of a Join to the left-hand side out the join.
/** Recursively push refs from the right-hand side of a Join to the left-hand side out of the join.
* This is only possible when they occur in a a mapping `Bind(_, _, Pure(StructNode))` directly
* at the RHS of a Join. Returns the (possibly transformed) Join and replacements for forward
* paths into it.
@@ -223,11 +223,13 @@ class RewriteJoins extends Phase {
* of `on2` refer to `s1`, merge them into `on1`. Nested joins are processed recursively. The
* same is done in the opposite direction, pushing predicates down into sub-joins if they only
* reference one side of the join. */
def rearrangeJoinConditions(j: Join): Join = j match {
def rearrangeJoinConditions(j: Join, alsoPull: Set[TermSymbol]): Join = j match {
case Join(s1, s2, _, j2a @ Join(_, _, _, _, JoinType.Inner, _), JoinType.Inner, on1) =>
val j2b = rearrangeJoinConditions(j2a)
val (on1Down, on1Keep) = splitConjunctions(on1).partition(p => hasRefTo(p, Set(s2)) && !hasRefTo(p, Set(s1)))
val (on2Up, on2Keep) = splitConjunctions(j2b.on).partition(p => hasRefTo(p, Set(s1)))
logger.debug("Trying to rearrange join conditions (alsoPull: "+alsoPull.mkString(", ")+") in:", j)
val pull = alsoPull + s1
val j2b = rearrangeJoinConditions(j2a, pull)
val (on1Down, on1Keep) = splitConjunctions(on1).partition(p => hasRefTo(p, Set(s2)) && !hasRefTo(p, pull))
val (on2Up, on2Keep) = splitConjunctions(j2b.on).partition(p => hasRefTo(p, pull))
if(on1Down.nonEmpty || on2Up.nonEmpty) {
val refS2 = Ref(s2) :@ j2b.nodeType.asCollectionType.elementType
val on1b = and(on1Keep ++ on2Up.map(_.replace({
@@ -247,6 +249,9 @@ class RewriteJoins extends Phase {
case j => j
}
// binary compatibility shim for 3.1
def rearrangeJoinConditions(j: Join): Join = rearrangeJoinConditions(j, Set.empty)
/** Merge nested mapping operations of the form `Bind(_, Bind(_, _, Pure(StructNode(p1), _)), Pure(StructNode(p2), _))`
* into a single Bind, provided that each element of either p1 or p2 contains not more than one path.
* This transformation is not required for the correctness of join rewriting but it keeps the

0 comments on commit 988f87d

Please sign in to comment.