Created
August 15, 2019 16:21
-
-
Save deusaquilus/f91b917edfe4cab8909d53a6a067678f to your computer and use it in GitHub Desktop.
Potential Need for PropertySet in ExpandNestedQueries
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
// 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