fix: compile parent(rel.id) or-filters as EXISTS instead of joining b…

[TravelCurry02]

Contributor checklist

Leave anything that you believe does not apply unchecked.

• I accept the AI Policy, or AI was not used in the creation of this PR.
• Bug fixes include regression tests
• Chores
• Documentation changes
• Features include unit/acceptance tests
• Refactoring
• Update dependencies

What was wrong

Some of the relationships are a union of two lists. The example from the original issue is 'all_tags' it should be "this post's genre tags, or this post's mood tags."

It would be written like this:
no_attributes? true
sort :id
filter expr(parent(genre_tags.id) == id or parent(mood_tags.id) == id)

'no_attributes?' means there is no direct foreign key. Ash needs to use the 'parent( )' filter to make a decision on what rows belong.

The old SQL joined both parent lists. if a post had 2 genre tags and 1 mood tag the join could produce extra combinations of those rows. It would then 'sort :id' added a 'row_number( )' column called 'order'. 'SELECT DISTINCT' also looked at 'order' so because of this, two copies of the same tag could survive due to them looking different. You would end up with 4 tags instead of 3.

The main issue boiled down to a Cartesian product. This shouldn't be a possibility. The fix that I implemented was to turn this into EXISTS, and that is the fix that I used.

The changes

it was done in two separate pieces both in ash_sql:

1. lib/join.ex
   If the filter is "parent(this.id) == id or parent(that.id) == id", we do not join both parent lists. When you would join them is when extra rows would be created.

2. lib/expr.ex
   For 'parent(rel.id)== id' it compiles EXISTS instead of a normal ==. Now EXISTS looks at whether the parent has this related id. This makes it more of a true-or-false per row so the same tag cannot show up twice from two join combinations.

I did testing through local ash_postgres due to there being no Postgres test suite. I added a small 'all_tags' relationship on Post and a test that created three tags that had ids in order of genre, mood, genre. It used to duplicate the 2nd tag, but now the test returns 3 unique rows, so it passed.

[zachdaniel]

The issue with matching on = like this is that there are many other formulations that can have this issue. I think what we need to do is make it so that when constructing a related filter, if it contains the construct that would cause this, change the whole thing to use exists.

[TravelCurry02]

That makes sense, matching on only = handles the one all_tags shape. While other parent filters can still run into the join issue.

I'll look into rewriting the whole related filter to EXISTS when it would join multiple parent paths.

[TravelCurry02]

I rewrote it so it doesn't match on '=' anymore.

Now, if a related filter would join two or more parent to-many paths, it skips those joins and rewrites that filter to 'exists', keeping the 'or' / 'and'. So '== id', '== title', and '>' all go through the same path.

I re-ran the all_tags case in ash_postgres (three unique tags). I also added a title union and a > filter there, both passed. Those tests aren't in this PR because they are in ash_postgres.

[zachdaniel]

👋 sorry it took me a while to get back to this, been busy! I think we need to move this much higher up in the stack where we build a join.

We need to ideally make the decision once, at the point where join.ex currently calls join_all_relationships(lateral_join_source_query, parent_expr(filter)). If parent_expr(filter) reaches a to-many relationship at any hop:

• Don't join the parent paths onto the lateral source query.
• Wrap the entire relationship filter in a correlated subquery over the source resource, roughly: EXISTS (SELECT 1 FROM posts p LEFT JOIN genre_tags … LEFT JOIN mood_tags … WHERE p.pk = <lateral source pk> AND <whole filter>).
• Bind parent() refs to that inner query, which has the joins.

[TravelCurry02]

I moved it up higher in the stack to about where you mentioned. If parent( ) would hit a many-to-many relationship, it doesn't join the paths together onto the source. The whole relationship filter is wrapped in one correlated EXISIT over the source now. The parent join lives inside the subquery, and parent( )also binds there. The tests in ash_postgres related to all_tags still pass.

[zachdaniel]

Pulled this down and ran it against ash_postgres. The lateral load case is fixed, but:

• The whole destination query filter gets wrapped in the exists, not just the relationship filter. Load query filters and policy filters with relationship refs then resolve against the source. load(union_comments: Comment |> filter(author.first_name == "Alice")) silently returns [] because it hits Post.author, and a relates_to_actor_via policy on the destination crashes. Only the relationship filter should go in the exists, and non-parent() refs (including exists/2) need to stay bound to the destination.
• many_to_many_expr_test.exs:149 in ash_postgres fails. The m2m's own join relationship counts as a to-many hop, so parent(credited_posts.role) == :editor gets decorrelated.
• Filtering through the relationship (exists(rel, ...) / rel.field == ...) and aggregates still join the parent paths onto the outer query. Filters are only saved by DISTINCT ON, and aggregates over it return wrong counts. All three should go through the same helper.
• correlate_source_pk needs to be a real field on Ash.Query.Exists, not Map.put onto the struct.
• Drop the typed_binary_operator_expr move in expr.ex. It's a no-op and it's why this conflicts with main.

I just pushed some tests to a branch in ash_postgres. What you want to do is have ash_postgresfolder next to ash_sql and run ASH_SQL_VERSION=local mix test to run those tests against your changes. All existing tests need to pass (they don't currently w/ these changes) as well as the new ones I added. Branch is here: https://github.com/ash-project/ash_postgres/tree/test/parent-union-relationships

[TravelCurry02]

I went through it again. Now the EXISTS wrap is only the cartesian parent( ) part. The load filters and dest exists/2 stay on the destination. authour.first_name and relates_to_actor_via do not resolve against the source. Now a single m2m hop is not treated as a cartiesian product anymore. This makes parent(credited_posts.role) stays correlated. Load, exists(rel) / rel.field, and aggregates now all go through maybe_join_parent_expr. I dropped the typed_binary_operator_expr extract and merged main. The correlate_source_pk is a real field on Ash.Query.Exists instead of Map.put. That was a one line change that i have locally and it isnt in the pr. The tests were ASH_VERSION=local ASH_SQL_VERSION=local mix test against test/parent-union-relationships.

[zachdaniel]

👋 this was very close! I wrapped it up by pushing a commit on my end.

• No more Ash.Query.Exists / correlate_source_pk. expr.ex is back to main, so no ash core change is needed.
• Instead of flipping parent() and the other refs by hand, add_filter_expression now groups every and statement that references parent() when necessary into one EXISTS over the source (parent_source_exists_expr):
  • The EXISTS is built directly in ash_sql: the source query, plus the parent-path joins, tied to the source row by primary key.
  • The filter inside it is compiled with the bindings swapped. Destination refs become parent_as the outer query, and parent() refs become as inside the subquery. The normal ref compiler does the scoping, so every kind of ref works without being listed one by one.
• Relationship filters are hydrated before the cartesian check. The raw filter still has unresolved %Call{} nodes, so the check always returned false.
• Nested paths resolve the right source resource, using refs_at_path/path instead of the root resource.

The previous version skipped the parent joins for the whole filter but only wrapped some subexpressions, so these broke:

• (cartesian or) and parent(author.first_name) == ... crashed.
• parent(a.x) == ... and parent(b.y) == ... crashed. That one works on main today.
• Destination relationship refs inside the wrapped or (e.g. author.first_name) were resolved against Post, so rows were silently dropped.