Skip to content

Conversation

@dwwoelfel
Copy link
Contributor

Ran into this bug while working on cel -> instaql, where we can generate lots of nested ORs.

Three fixes in this PR:

  1. If there's a single AND or OR, then we just strip it out ([{:or [{handle: 'dww'}]}] is the same as {handle: 'dww'})
  2. Collapse the ors properly, so that [{:or [{:or [cond-a, cond-b]}, cond-c]]}] generates {:or [cond-a, cond-b, cond-c]} instead of {:or [[cond-a, cond-b], cond-c]}.
  3. Fix in datalog.clj to handle ORs in the destination path. I simplified the logic to only do recursion for generating the join conds in one place.

There is still a remaining issue where we could potentially include the same cte twice, leading to postgres complaints about ambiguous columns. I can't reproduce it without disabling the or collapsing, so I'm ignoring it for now. I think it might have something to do with how we generate join syms for the different groups of ors and ands. We might need to increment the level?

[{:expected 'non-empty-list?
:in (conj (:in state) :or)
:message "The list of `or` conditions can't be empty."}])))}
(let [conds (->> v
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still overly complicated--I think it would be a lot easier to reason about if we got rid of the spec coercion, but that's a bigger project.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oo, that could be interesting! Would make it nicer too when we print stuff deep inside the codebase

@github-actions
Copy link
Contributor

github-actions bot commented Mar 6, 2025

Copy link
Contributor

@stopachka stopachka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Niice!

@dwwoelfel dwwoelfel merged commit 94a9a49 into main Mar 6, 2025
29 checks passed
@dwwoelfel dwwoelfel deleted the fix-lots-of-nested-ands-and-ors branch March 6, 2025 18:15
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.

3 participants