Skip to content

Instantly share code, notes, and snippets.

@deusaquilus
Created August 15, 2019 16:21
Show Gist options
  • Save deusaquilus/f91b917edfe4cab8909d53a6a067678f to your computer and use it in GitHub Desktop.
Save deusaquilus/f91b917edfe4cab8909d53a6a067678f to your computer and use it in GitHub Desktop.
Potential Need for PropertySet in ExpandNestedQueries
// When doing a query like this (this is in the unit tests)
val q = quote {
qr1.leftJoin(qr2).on {
(a, b) =>
a.i == b.i
}.filter {
ab => {
val (a, b) = ab
b.map(bv => bv.l).contains(3L)
}
}.leftJoin(qr3).on {
(ab, c) => {
val (a, b) = ab
b.map(bv => bv.i).contains(a.i) && b.map(bv => bv.i).contains(c.i)
}}
}
// This kind of result is achieved:
// SELECT ab._1s, ab._1i, ab._1l, ab._1o, ab._2s, ab._2i, ab._2l, ab._2o, c.s, c.i, c.l, c.o FROM (
// SELECT a.s AS _1s, a.o AS _1o, a.l AS _1l, a.i AS _1i, b.l AS _2l, b.i AS _2i, b.o AS _2o, b.s AS _2s FROM TestEntity a LEFT JOIN TestEntity2 b ON a.i = b.i WHERE b.l = 3) AS ab LEFT JOIN TestEntity3 c ON ab._2i = ab._1i AND ab._2i = c.i
// However, when Properties are set as renameable = Internal (no longer exists) or potentially Fixed in the AST Transformations,
// ExpandNestedQueries can fail to dedupe properties. Therefore, this results:
// SELECT ab._1s, ab._1i, ab._1l, ab._1o, ab._2s, ab._2i, ab._2l, ab._2o, c.s, c.i, c.l, c.o FROM (
// SELECT a.s AS _1s, a.i AS _1i, a.l AS _1l, a.o AS _1o, b.s AS _2s, b.i AS _2i, b.l AS _2l, b.o AS _2o, b.i AS _2i, a.i AS _1i FROM TestEntity a LEFT JOIN TestEntity2 b ON a.i = b.i WHERE b.l = 3) AS ab LEFT JOIN TestEntity3 c ON ab._2i = ab._1i AND ab._2i = c.i
// Note how a.i AS _1i and b.i AS _2i are repated. The 2nd copy should show up as a.i AS `_1`/*Fixed*/i and b.i AS `_2`/*Fixed*/i
// if properties where renameable = Fixed are stringified with `` and /*Fixed*/ at the end.
// In order to solve this problem we need to introduce a PropertySet into ExpandNestedQueries which does de-duping of properties
// that have different Opinions. For Renameable however, if one is Fixed and the other is renameable ByStrategy, the net result needs
// to be Fixed since otherwise I think the wrong behavior may result.
// Check out the branch rename-fix-expandnestedqueries-propertyset as a starting point but that does not actually
// do a comparison to see if one property was Fixed and other other was ByStrategy and took the Fixed variant.
// The reason this functionality was not included in #1560 is because renameable: Internal was ultimately removed
// so there was no need for it.
// However, once rename=Fixed for aliases is introduced this may once again become necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment