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

Fixing lift-ids duplication issue in dynamic query lists #520

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

deusaquilus
Copy link
Contributor

@deusaquilus deusaquilus commented Oct 29, 2024

Fixes all known cases of #318
More holistic approach than #398

This issue really bothered me throughout the latter half of 2023 and I feared that it was a fundemental flaw of the approach in ProtoQuill that I had decided use for runtime lifts (i.e. generating UIDs at runtime in the call-site of the lift macro). I reasoned through multiple approaches such as adding secondary runtime-specific UUIDs as well as embedding lifts directly into the Ast, a step which I was very hesitant to take because it would mean that the tree can no-longer be serialized (and would be harder to cache). Recently however, I realized that the solution was far simpler than I had thought because there are two fundemental constraints to the problem:

  1. The only situation in which a runtime lift UUID is duplicated is in a runtime quotation (specifically in a QuotationVase)
  2. Before any quotation sitting in a QuotationVase is merged back into the final composite query
    (e.g. (quotationVaseQuotationA || quotationVaseQuotationB).filter(...)) it is fully available in the parent quotation runtimeLifts list as a separate item.

The consequence of these two constraints is that we can trivially know where duplicate lifts are and can physically inspect and dudupe them. Furthermore, it is not even necessary to find out what the duplicate lifts are. We can merely regenerate the UUIDs for all the lifts in each QuotationVase individually. That means that when they are merged back into the final query, they will have unique UUIDs. (We can also do this to the UUIDs of the QuotationVase/QuotationTag themselves to ensure that they are unique as well - since the whole quotation might have been inserted from a dynamic list).

Therefore in dynamic situations that collect and then combine multiple queries e.g:

List("a", "b").map(a => quote { query[Person].filter(p => p.name == lift(a)) }).reduce(_ ++ _)

since the lift(a) UID is determined at compile-time the trees of both "a", and "b" variants will have the same value for it resulting in a tree that looks like this (before being flattened into a single filter which happens during query compilation):

Quotation(
  ast = UnionAll(QuotationTag(A), QuotationTag(B)),
  runtimeLifts = List(
    QuotationVase(A, Filter(Entity("Person"), BinaryOperation(p.name, ==, ScalarTag("SOME_UUID")))
    QuotationVase(B, Filter(Entity("Person"), BinaryOperation(p.name, ==, ScalarTag("SOME_UUID")))
  )
)

@deusaquilus deusaquilus mentioned this pull request Oct 29, 2024
@deusaquilus deusaquilus merged commit 1a18e5b into master Oct 29, 2024
3 of 4 checks passed
@deusaquilus deusaquilus deleted the dynamic-dup-fix branch October 29, 2024 14:25
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