Browse files

Fix zip join regression on SQL Server

The test fails since #1649 was merged
but we didn’t notice because it went into the 3.1 branch which does not
yet run on what were then the Slick Extensions drivers.

This failure demonstrates that even with the new backend in 3.1 it is
still hard to predict all possible results when it comes to comprehension
fusion. The cause of the regression is not actually the new reordering
from #1649 but the fix for `h2rownum` that was necessary as a result of
it. The extra identity mapping prevented `reorderOperations` from
pushing a `Subquery.BelowRownum` into a `SortBy` (because there is now
an identity mapping in between the two) and then eliminating it entirely
when it hits the `TableNode` below. This led to an unnecessary subquery,
which would have gone unnoticed if Slick was better at propagating
sort order through subqueries.

There is an existing rewrite rule in `reorderOperations` to push
aliasing mappings under subqueries that are not of type `AboveDistinct`.
I added another exception for `BelowRowNumber` and a rule that does
the opposite, i.e. push `Subquery.BelowRowNumber` into aliasing mappings
just like we already push it into `SortBy` and `Filter`. Since the goal
of these rules is to push it down so far that it eventually hits a
`TableNode` (where it can be eliminated), this seems like the right
direction to take.
  • Loading branch information...
szeiger committed Dec 5, 2016
1 parent dbe94de commit 30d9c4134d658f1457e8f92abb0f70a131a0e705
Showing with 5 additions and 1 deletion.
  1. +5 −1 slick/src/main/scala/slick/compiler/ReorderOperations.scala
@@ -58,7 +58,7 @@ class ReorderOperations extends Phase {
Subquery(Drop(from, count), Subquery.AboveDistinct).infer()
// Push any aliasing / literal projection into other Subquery
- case n @ Bind(s, Subquery(from, cond), Pure(StructNode(defs), ts1)) if cond != Subquery.AboveDistinct && isAliasingOrLiteral(s, defs) =>
+ case n @ Bind(s, Subquery(from, cond), Pure(StructNode(defs), ts1)) if cond != Subquery.AboveDistinct && cond != Subquery.BelowRowNumber && isAliasingOrLiteral(s, defs) =>
Subquery(n.copy(from = from), cond).infer()
// If a Filter checks an upper bound of a ROWNUM, push it into the AboveRownum boundary
@@ -79,6 +79,10 @@ class ReorderOperations extends Phase {
case sq @ Subquery(n: Filter, Subquery.BelowRowNumber) =>
n.copy(from = convert1(sq.copy(child = n.from))).infer()
+ // Push a BelowRowNumber boundary into aliasing / literal projection
+ case sq @ Subquery(n @ Bind(s, from, Pure(StructNode(defs), ts1)), Subquery.BelowRowNumber) if isAliasingOrLiteral(s, defs) =>
+ n.copy(from = convert1(sq.copy(child = from))).infer()
case n => n

0 comments on commit 30d9c41

Please sign in to comment.