Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix group by alias conflict and don't apply join(groupby.map) #899

Merged
merged 1 commit into from
Sep 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ target
**/.tmpBin
local.*
quill_test.db
Bug.scala
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ commands += Command.command("checkUnformattedFiles") { st =>
val vcs = Project.extract(st).get(releaseVcs).get
val modified = vcs.cmd("ls-files", "--modified", "--exclude-standard").!!.trim.split('\n').filter(_.contains(".scala"))
if(modified.nonEmpty)
throw new IllegalStateException(s"Please run `sbt scalariformFormat test:scalariformFormat` and resubmit your pull request. Found unformatted files: $modified")
throw new IllegalStateException(s"Please run `sbt scalariformFormat test:scalariformFormat` and resubmit your pull request. Found unformatted files: ${modified.toList}")
st
}

Expand Down
51 changes: 27 additions & 24 deletions quill-core/src/main/scala/io/getquill/norm/ApplyMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,70 +7,73 @@ object ApplyMap {
private def isomorphic(e: Ast, c: Ast, alias: Ident) =
BetaReduction(e, alias -> c) == c

object DetachableMap {
def unapply(ast: Ast): Option[(Ast, Ident, Ast)] =
ast match {
case Map(a: GroupBy, b, c) => None
case Map(a, b, c) => Some((a, b, c))
case _ => None
}
}

def unapply(q: Query): Option[Query] =
q match {

case Map(Map(a: GroupBy, b, c), d, e) => None
case FlatMap(Map(a: GroupBy, b, c), d, e) => None
case Filter(Map(a: GroupBy, b, c), d, e) => None
case SortBy(Map(a: GroupBy, b, c), d, e, f) => None
case Take(Map(a: GroupBy, b, c), d) => None
case Drop(Map(a: GroupBy, b, c), d) => None
case Map(a: GroupBy, b, c) if (b == c) => None
case Map(a: Nested, b, c) if (b == c) => None
case Map(a: GroupBy, b, c) if (b == c) => None
case Map(a: Nested, b, c) if (b == c) => None

// map(i => (i.i, i.l)).distinct.map(x => (x._1, x._2)) =>
// map(i => (i.i, i.l)).distinct
case Map(Distinct(Map(a, b, c)), d, e) if isomorphic(e, c, d) =>
case Map(Distinct(DetachableMap(a, b, c)), d, e) if isomorphic(e, c, d) =>
Some(Distinct(Map(a, b, c)))

// a.map(b => b) =>
// a
case Map(a: Query, b, c) if (b == c) =>
Some(a)

// a.map(b => c).map(d => e) =>
// a.map(b => e[d := c])
case Map(Map(a, b, c), d, e) =>
case Map(DetachableMap(a, b, c), d, e) =>
val er = BetaReduction(e, d -> c)
Some(Map(a, b, er))

// a.map(b => b) =>
// a
case Map(a: Query, b, c) if (b == c) =>
Some(a)

// a.map(b => c).flatMap(d => e) =>
// a.flatMap(b => e[d := c])
case FlatMap(Map(a, b, c), d, e) =>
case FlatMap(DetachableMap(a, b, c), d, e) =>
val er = BetaReduction(e, d -> c)
Some(FlatMap(a, b, er))

// a.map(b => c).filter(d => e) =>
// a.filter(b => e[d := c]).map(b => c)
case Filter(Map(a, b, c), d, e) =>
case Filter(DetachableMap(a, b, c), d, e) =>
val er = BetaReduction(e, d -> c)
Some(Map(Filter(a, b, er), b, c))

// a.map(b => c).sortBy(d => e) =>
// a.sortBy(b => e[d := c]).map(b => c)
case SortBy(Map(a, b, c), d, e, f) =>
case SortBy(DetachableMap(a, b, c), d, e, f) =>
val er = BetaReduction(e, d -> c)
Some(Map(SortBy(a, b, er, f), b, c))

// a.map(b => c).drop(d) =>
// a.drop(d).map(b => c)
case Drop(Map(a, b, c), d) =>
case Drop(DetachableMap(a, b, c), d) =>
Some(Map(Drop(a, d), b, c))

// a.map(b => c).take(d) =>
// a.drop(d).map(b => c)
case Take(Map(a, b, c), d) =>
case Take(DetachableMap(a, b, c), d) =>
Some(Map(Take(a, d), b, c))

// a.map(b => c).nested =>
// a.nested.map(b => c)
case Nested(Map(a, b, c)) =>
case Nested(DetachableMap(a, b, c)) =>
Some(Map(Nested(a), b, c))

// a.map(b => c).*join(d.map(e => f)).on((iA, iB) => on)
// a.*join(d).on((b, e) => on[iA := c, iB := f]).map(t => (c[b := t._1], f[e := t._2]))
case Join(tpe, Map(a, b, c), Map(d, e, f), iA, iB, on) =>
case Join(tpe, DetachableMap(a, b, c), DetachableMap(d, e, f), iA, iB, on) =>
val onr = BetaReduction(on, iA -> c, iB -> f)
val t = Ident("t")
val t1 = BetaReduction(c, b -> Property(t, "_1"))
Expand All @@ -79,7 +82,7 @@ object ApplyMap {

// a.*join(b.map(c => d)).on((iA, iB) => on)
// a.*join(b).on((iA, c) => on[iB := d]).map(t => (t._1, d[c := t._2]))
case Join(tpe, a, Map(b, c, d), iA, iB, on) =>
case Join(tpe, a, DetachableMap(b, c, d), iA, iB, on) =>
val onr = BetaReduction(on, iB -> d)
val t = Ident("t")
val t1 = Property(t, "_1")
Expand All @@ -88,7 +91,7 @@ object ApplyMap {

// a.map(b => c).*join(d).on((iA, iB) => on)
// a.*join(d).on((b, iB) => on[iA := c]).map(t => (c[b := t._1], t._2))
case Join(tpe, Map(a, b, c), d, iA, iB, on) =>
case Join(tpe, DetachableMap(a, b, c), d, iA, iB, on) =>
val onr = BetaReduction(on, iA -> c)
val t = Ident("t")
val t1 = BetaReduction(c, b -> Property(t, "_1"))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package io.getquill.norm.capture

import io.getquill.ast.Ast
import io.getquill.ast._
import io.getquill.ast.Entity
import io.getquill.ast.Filter
import io.getquill.ast.FlatMap
Expand All @@ -12,6 +12,7 @@ import io.getquill.ast.SortBy
import io.getquill.ast.StatefulTransformer
import io.getquill.norm.BetaReduction
import io.getquill.ast.FlatJoin
import io.getquill.ast.GroupBy

private case class AvoidAliasConflict(state: collection.Set[Ident])
extends StatefulTransformer[collection.Set[Ident]] {
Expand All @@ -31,6 +32,9 @@ private case class AvoidAliasConflict(state: collection.Set[Ident])
case SortBy(q: Entity, x, p, o) =>
apply(x, p)(SortBy(q, _, _, o))

case GroupBy(q: Entity, x, p) =>
apply(x, p)(GroupBy(q, _, _))

case Join(t, a, b, iA, iB, o) =>
val (ar, art) = apply(a)
val (br, brt) = art.apply(b)
Expand All @@ -47,7 +51,9 @@ private case class AvoidAliasConflict(state: collection.Set[Ident])
val (orr, orrt) = AvoidAliasConflict(art.state + freshA)(or)
(FlatJoin(t, ar, freshA, orr), orrt)

case other => super.apply(other)
case _: Entity | _: FlatMap | _: Map | _: Filter | _: SortBy | _: GroupBy | _: Aggregation |
_: Take | _: Drop | _: Union | _: UnionAll | _: Distinct | _: Nested =>
super.apply(q)
}

private def apply(x: Ident, p: Ast)(f: (Ident, Ast) => Query): (Query, StatefulTransformer[collection.Set[Ident]]) = {
Expand Down
23 changes: 22 additions & 1 deletion quill-core/src/test/scala/io/getquill/norm/ApplyMapSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ApplyMapSpec extends Spec {
}
"map" in {
val q = quote {
qr1.groupBy(t => t.i).map(y => y._1).map(s => s)
qr1.groupBy(t => t.i).map(y => y._1).filter(i => i == 1)
}
ApplyMap.unapply(q.ast) mustEqual None
}
Expand All @@ -48,12 +48,33 @@ class ApplyMapSpec extends Spec {
}
ApplyMap.unapply(q.ast) mustEqual None
}

"identity map" in {
val q = quote {
qr1.groupBy(t => t.i).map(y => y)
}
ApplyMap.unapply(q.ast) mustEqual None
}
"mapped join" - {
"left" in {
val q = quote {
qr1.groupBy(t => t.i).map(t => t._1).join(qr2).on((a, b) => a == b.i)
}
ApplyMap.unapply(q.ast) mustEqual None
}
"right" in {
val q = quote {
qr1.join(qr2.groupBy(t => t.i).map(t => t._1)).on((a, b) => a.i == b)
}
ApplyMap.unapply(q.ast) mustEqual None
}
"both" in {
val q = quote {
qr1.groupBy(t => t.i).map(t => t._1).join(qr2.groupBy(t => t.i).map(t => t._1)).on((a, b) => a == b)
}
ApplyMap.unapply(q.ast) mustEqual None
}
}
}

"avoids applying the identity map with nested query" in {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ class AvoidAliasConflictSpec extends Spec {
}
AvoidAliasConflict(q.ast) mustEqual n.ast
}
"groupBy" in {
val q = quote {
qr1.flatMap(a => qr2.groupBy(a => a.s))
}
val n = quote {
qr1.flatMap(a => qr2.groupBy(a1 => a1.s))
}
AvoidAliasConflict(q.ast) mustEqual n.ast
}
"outer join" - {
"both sides" in {
val q = quote {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ object ExpandNestedQueries {
case List(SelectValue(i: Ident, _)) =>
SelectValue(Property(i, name))
case other =>
SelectValue(Ident(name))
SelectValue(Ident(name), Some(name))
}
}
}
Expand Down