Work on removing tuple elaboration #2381
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since we can expand a row from a quat we might not need tuple-elaboration in the first place. This would allow us to implement a more complete solution to #2340.
Query Expansion before Quats
In previous version of quill where AST had no typing information (i.e. the Quats), a query that looked like this:
Could be translated to this:
The problem is that the tree does not know what the type of the identifier
p
is so which columns need to be selected from it is not known. Of course we could always dop.*
but if the columns in thePerson
case-class are note the same ones as in the Person table the decoding of the query would fail. ThereforematerializeQueryMeta
was introduced to expand the query into:This then provided enough data to fully expand the query into:
It worked something like this:
Issues
This solution had several notable flaws. Firstly, since the actual object being decoded to is a Tuple2 (i.e. TupleX depending on the object), the real query that is generate would be:
Normally this isn't a problem because the Quill decoders use column index as opposed to column name for decoding. However, for the
Context.prepare
function that returns a ResultSet as well as for ProtoQuill coproduct decodings (which rely on column names), this approach would not work. Also the _1 and _2 present an oddity for the user that is not aware of this query expansion mechanism. For this reason, a transformation toSqlQuery
was introduced in order to remove all top-level aliases (the RemoveExtraAlias class used to do that). Still a better technique was desired, one that returnsname
andage
columns for a given object as opposed to _1 and _2.A Better Solution
Ultimately, Quats were introduced and this added type information to every query. That meant that every identifier now had information as to whether this field was a product or value, and what the fields inside it are if it were a product. That means that given this:
It is known what fields are inside the
p
variable. The expansion goes something like this:This mechanism lives in ExpandNestedQueries (and
ExpandSelection
) where aSelectValue(Id("x", QP))
is expanded intoList(SelectValue(Prop(Id("x", QP), "name")), Prop(Id("x", QP), "age")))
. It was introduced in #1920 in order to correctly expand identifiers that materializeQueryMeta would not expand (e.g. identifiers in nested queries). This functionality existed before in some form but until #1920 it was not reliable.This PR removes the
materializeQueryMeta
mechanism and relies on Quats to do the full identifier expansion as shown inFig .1
.