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

Switch to disable query sub-expansion #2380

Closed
wants to merge 1 commit into from

Conversation

deusaquilus
Copy link
Collaborator

@deusaquilus deusaquilus commented Jan 16, 2022

Mitigates: #2340

Query sub-expansion in astTokenizer adds column AS col suffixes via the additional steps RemoveExtraAlias(strategy)(ExpandNestedQueries(SqlQuery(a))) but these are not subject to the naming strategy as pointed out by #2340.

Unfortunately this cannot be easily remedied by surrounding the query a with a CaseClass that has fields from it's quat because it is already surrounded by a tuple due to the Elaboration in materializeQueryMeta that happens before query expansion even starts. The in the astTokenizer, the variable a has the following value:

querySchema("TestEntity").filter(t => t.s == "a").map(t => (t.s, t.i, t.l, t.o, t.b))

Note that the end of it is wrapped into a tuple which we would probably need to peel off before accessing the Product-Quat in the actual query querySchema("TestEntity").filter(t => t.s == "a"). We could forcibly do this anyway just for this use-case but that feels hacky.

Now in #2381 we are exploring the idea of removing tuple-elaboration completely. Tuple elaboration was originally introduced before Quats existed due to the limitations of ExpandNestedQuery because selection from arbitrary Identifiers select x could not be elaborated into a set of columns select x.name, x.age since before Quats existed we had no idea that x was a product type with the fields name and age. Now that Quats exist this is no longer necessary.

(Also note that in ProtoQuill there is also a elaboration mechanism but it elaborates into case-classes and not tuples. If this experiment succeeds and we can remove Tuple elaboration in favor of quats then we should probably do the same thing in protoquill).

@deusaquilus deusaquilus deleted the disable-subexpand-switch branch January 19, 2022 03:34
@deusaquilus
Copy link
Collaborator Author

This has been superseded by #2383

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.

1 participant