Permalink
Browse files

Evaluate paths into grouped keys & values when converting GroupBy.

Grouping by a tuple of multiple keys can lead to paths into the
individual components of the tuple in the grouped result. When converting
GroupBy to Comprehension, we need to evaluate these paths completely
instead of only evaluating the first ElementSymbol and leaving Select()s
into ProductNodes alive.

Fixes issue #110. Test in AggregateTest.testGroupBy/q4.
  • Loading branch information...
szeiger committed Mar 25, 2013
1 parent 4671f6d commit 5ed76809d2e93c4939f5cb55db4d48bae9194d83
@@ -106,5 +106,17 @@ class AggregateTest(val tdb: TestDB) extends TestkitTest {
val r3t = r3: List[(Int, Option[Int])]
println(r3)
assertEquals(List((11, Some(6)), (12, Some(8)), (13, Some(10))), r3)
println("=========================================================== q4")
val q4 = (for {
(x, q) <- T.groupBy(t => (t.a, t.b))
} yield (x, q.length)).sortBy(_._1)
println(q4.selectStatement)
val r4 = q4.list
val r4t = r4: List[((Int, Option[Int]), Int)]
println(r4)
assertEquals(List( ((1,Some(1)),1), ((1,Some(2)),1), ((1,Some(3)),1),
((2,Some(1)),1), ((2,Some(2)),1), ((2,Some(5)),1),
((3,Some(1)),1), ((3,Some(9)),1)), r4)
}
}
@@ -101,14 +101,19 @@ class ConvertToComprehensions extends Phase {
/** Convert a GroupBy followed by an aggregating map operation to a Comprehension */
def convertSimpleGrouping(gen: Symbol, fromGen: Symbol, from: Node, by: Node, sel: Node): Node = {
object FwdPath {
def unapply(n: Node): Option[List[Symbol]] =
Path.unapply(n).map(_.reverse)
def toString(path: Seq[Symbol]): String = path.mkString("Path ", ".", "")
}
val newBy = by.replace { case Ref(f) if f == fromGen => Ref(gen) }
val newSel = sel.replace {
case Bind(s1, Select(Ref(gen2), ElementSymbol(2)), Pure(ProductNode(Seq(Select(Ref(s2), field)))))
if (s2 == s1) && (gen2 == gen) => Select(Ref(gen), field)
case Library.CountAll(Select(Ref(gen2), ElementSymbol(2))) if gen2 == gen =>
Library.Count(ConstColumn(1))
case Select(Ref(gen2), ElementSymbol(2)) if gen2 == gen => Ref(gen2)
case Select(Ref(gen2), ElementSymbol(1)) if gen2 == gen => newBy
case FwdPath(gen2 :: ElementSymbol(idx) :: rest) if gen2 == gen && (idx == 1 || idx == 2) =>
Phase.fuseComprehensions.select(rest, if(idx == 2) Ref(gen) else newBy)(0)
}
Comprehension(Seq(gen -> from), groupBy = Some(newBy), select = Some(Pure(newSel)))
}
@@ -328,7 +333,7 @@ class FuseComprehensions extends Phase {
//case (s, Union(l, r, _, _, _)) => select(s, l) ++ select(s, r)
case (Nil, n) => Vector(n)
case ((s: AnonSymbol) :: t, StructNode(ch)) => select(t, ch.find{ case (s2,_) => s == s2 }.get._2)
//case ((s: ElementSymbol) :: t, ProductNode(ch @ _*)) => select(t, ch(s.idx-1))
case ((s: ElementSymbol) :: t, ProductNode(ch)) => select(t, ch(s.idx-1))
case _ => throw new SlickException("Cannot select "+Path.toString(selects.reverse)+" in "+base)
}
}

0 comments on commit 5ed7680

Please sign in to comment.