Permalink
Browse files

Merge pull request #145 from slick/tmp/issue-141

Expand symbols in the "by" clause of a GroupBy node.
  • Loading branch information...
szeiger committed Apr 29, 2013
2 parents ac50017 + 511cb76 commit ca4b390f991c5ccdf44b0816e2517b2446831293
@@ -119,14 +119,22 @@ class AggregateTest(val tdb: TestDB) extends TestkitTest {
((2,Some(1)),1), ((2,Some(2)),1), ((2,Some(5)),1),
((3,Some(1)),1), ((3,Some(9)),1)), r4)
println("=========================================================== q5")
val q5 = Query(T)
.filter(_.a === 1)
.map(t => (t.a, t.b))
.sortBy(_._2)
.groupBy(x => (x._1, x._2))
.map { case (a, _) => (a._1, a._2) }
assertEquals(Set((1, Some(1)), (1, Some(2)), (1, Some(3))), q5.run.toSet)
U.insert(4)
println("=========================================================== q5")
val q5 = (for {
println("=========================================================== q6")
val q6 = (for {
(u, t) <- U leftJoin T on (_.id === _.a)
} yield (u, t)).groupBy(_._1.id).map {
case (id, q) => (id, q.length, q.map(_._1).length, q.map(_._2).length)
}
assertEquals(Set((1, 3, 3, 3), (2, 3, 3, 3), (3, 2, 2, 2), (4, 1, 1, 0)), q5.run.toSet)
assertEquals(Set((1, 3, 3, 3), (2, 3, 3, 3), (3, 2, 2, 2), (4, 1, 1, 0)), q6.run.toSet)
}
}
@@ -125,7 +125,7 @@ class ExpandRefs extends Phase with ColumnizerUtils {
def expandRefs(n: Node, scope: Scope = Scope.empty, keepRef: Boolean = false): Node = n match {
case p @ Path(psyms) =>
logger.debug("Checking path "+Path.toString(psyms))
logger.debug("Checking path "+Path.toString(psyms)+" (keepRef="+keepRef+")")
psyms.head match {
case f: FieldSymbol => p
case _ if keepRef => p
@@ -149,7 +149,15 @@ class ExpandRefs extends Phase with ColumnizerUtils {
n.mapChildrenWithScope(((_, ch, chsc) => expandRefs(ch, chsc, true)), scope)
case n =>
// Don't expand children in 'from' positions
n.mapChildrenWithScope(((symO, ch, chsc) => expandRefs(ch, chsc, symO.isDefined)), scope)
n.mapChildrenWithScope({ (symO, ch, chsc) =>
val kr = (symO, n) match {
case (None, _) => false
//TODO GroupBy should not need a "by" symbol at all -- without that, this special case would not be necessary
case (Some(sym), GroupBy(_, by, _, _)) => sym != by
case _ => true
}
expandRefs(ch, chsc, kr)},
scope)
}
/** Expand a base path into a given target */

0 comments on commit ca4b390

Please sign in to comment.