Skip to content

poc: extend exists/2 to traverse embedded array attributes#231

Draft
nallwhy wants to merge 4 commits into
ash-project:mainfrom
nallwhy:feat/embedded-array-exists
Draft

poc: extend exists/2 to traverse embedded array attributes#231
nallwhy wants to merge 4 commits into
ash-project:mainfrom
nallwhy:feat/embedded-array-exists

Conversation

@nallwhy
Copy link
Copy Markdown
Contributor

@nallwhy nallwhy commented May 17, 2026

Copilot AI review requested due to automatic review settings May 17, 2026 12:33
@nallwhy nallwhy marked this pull request as draft May 17, 2026 12:33
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Extends AshSql.Expr’s lowering of exists/2 so it can traverse {:array, EmbeddedResource} attributes by translating embedded-array traversal into Postgres jsonb_array_elements(...)-based EXISTS fragments, including support for paths that start with relationships and then enter embedded arrays.

Changes:

  • Refactors relationship-based exists/2 compilation into do_relationship_exists_dynamic/9.
  • Adds embedded-array exists/2 lowering via embedded_array_exists_dynamic/8, fragment construction helpers, and embedded-path splitting (split_at_first_embedded_array/2).
  • Introduces compilation support for embedded-array element field markers (Ash.Query.EmbeddedArrayElementField) to reference opt.elem fields inside the generated EXISTS subquery.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/expr.ex
Comment on lines +3253 to +3260
source_ref = %Ref{
attribute: Ash.Resource.Info.attribute(source_resource, first_segment),
relationship_path: full_at_path,
resource: bindings.resource
}

{source_dyn, acc} =
do_dynamic_expr(query, source_ref, Map.put(bindings, :no_cast?, true), embedded?, acc)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nallwhy looks like copilot is right on this one>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@zachdaniel

I created those PRs purely as an AI-generated proof of concept to see whether the idea in ash-project/ash#2698 was even implementable. I didn’t really review the code at all, and I never intended to merge it as-is. Normally I’m pretty careful even when using AI-assisted code, but in this case I treated it as a disposable experiment.

Since this touches a fairly complex part of the framework, I think you’d probably be the best person to implement it properly. If you’re too busy, though, I can take another pass at it and start over based on what the PoC demonstrated instead of trying to salvage the current PR.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah okay I see 😄 I probably won't have time any time soon but I'll consider it.

I'm game for you to take a stab at it if you like though!

Comment thread lib/expr.ex
[
{:raw, "EXISTS (SELECT 1 FROM jsonb_array_elements("},
{:casted_expr, source_dyn},
{:raw, ") AS outer_0(elem)" <> chain_sql <> " WHERE "},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to be very careful with this chain_sql here. Is there some way to transform this to an array of values somehow? i.e a function that accepts a list of strings and does the same work?

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