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 join subquery+map+distinct and sortBy+distinct #1462

Merged
merged 1 commit into from Jun 17, 2019

Conversation

deusaquilus
Copy link
Collaborator

@deusaquilus deusaquilus commented Jun 4, 2019

Fixes #1446, #1447 and #1472 as well as #1463.

@fwbrasil I would appreciate it if you could review AST transformation changes. This one seems to be a very simple solution. Am I missing anything?
Alternatively, maybe every single case of DetachableMap in ApplyMap should have a variant with Distinct(DetachableMap(...))?

Problem

Join+Distinct+Map fails:

case class Person(id: Int, name: String, age: Int)
case class Membership(personId: Int, team: Int)

val m = run(
  query[Person]
    .join(
      query[Membership]
        .filter(liftQuery(Set(1,2,3)) contains _.team)
        .map(_.personId)
        .distinct
    )
    .on(_.id == _)
    .map(_._1)
)

Results in:

exception during macro expansion: 
java.lang.IllegalStateException: The monad composition can't be expressed using applicative joins.

Solution

To fix #1446 add the following clause to ApplyMap:

      // a.*join(b.map(c => d).distinct).on((iA, iB) => on)
      //    a.*join(b.distinct).on((iA, c) => on[iB := d]).map(t => (t._1, d[c := t._2]))
      case Join(tpe, a, Distinct(DetachableMap(b, c, d)), iA, iB, on) =>
        val onr = BetaReduction(on, iB -> d)
        val t = Ident("t")
        val t1 = Property(t, "_1")
        val t2 = BetaReduction(d, c -> Property(t, "_2"))
        Some(Map(Join(tpe, a, Distinct(b), iA, c, onr), t, Tuple(List(t1, t2))))

The following is then returned:

SELECT x1.id, x1.name, x1.age, m.person_id FROM person x1 INNER JOIN (SELECT DISTINCT x.person_id FROM membership x) AS m ON x1.id = m.person_id

Same thing with #1447, add the following to ApplyMap:

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

Then you will get:

SELECT x5._1id, x5._1name, x5._1age, x5._2person_id, x5._2team FROM (SELECT DISTINCT x3.age AS _1age, x3.id AS _1id, x3.name AS _1name, x4.team AS _2team, x4.person_id AS _2person_id FROM person x3 INNER JOIN membership x4 ON x3.id = x4.person_id ORDER BY x3.id DESC) AS x5
  • Unit test all changes
  • Update README.md if applicable
  • Add [WIP] to the pull request title if it's work in progress
  • Squash commits that aren't meaningful changes
  • Run sbt scalariformFormat test:scalariformFormat to make sure that the source files are formatted

@getquill/maintainers

@deusaquilus deusaquilus requested a review from fwbrasil June 4, 2019 07:41
@deusaquilus deusaquilus changed the title Fix join subquery+map+distinct issue Fix join subquery+map+distinct and sortBy+distinct Jun 4, 2019
@@ -100,6 +106,15 @@ object ApplyMap {
val t2 = BetaReduction(d, c -> Property(t, "_2"))
Some(Map(Join(tpe, a, b, iA, c, onr), t, Tuple(List(t1, t2))))

// a.*join(b.map(c => d).distinct).on((iA, iB) => on)
// a.*join(b.distinct).on((iA, c) => on[iB := d]).map(t => (t._1, d[c := t._2]))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This transformation doesn't seem valid. In b.map(c => d).distinct, it'll return distinct mapped values, in b.distinct it'll return distinct records considering all columns

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I took this out.

@deusaquilus deusaquilus force-pushed the join_map_distinct_fix branch 2 times, most recently from 3a1365d to 63669e8 Compare June 14, 2019 20:22
@deusaquilus deusaquilus force-pushed the join_map_distinct_fix branch 2 times, most recently from 8a6400c to 7aff8fe Compare June 17, 2019 16:49
@deusaquilus
Copy link
Collaborator Author

The merged implementation also fixed #1463.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compilation error when joining a sub-query with map and distinct
2 participants