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

Using column references in json_extract_path doesn't work #646

Open
KirillKayumov opened this issue Nov 2, 2024 · 2 comments
Open

Using column references in json_extract_path doesn't work #646

KirillKayumov opened this issue Nov 2, 2024 · 2 comments
Labels

Comments

@KirillKayumov
Copy link

Elixir version

1.16.2

Database and Version

PostgreSQL 14.9

Ecto Versions

ecto: 3.11.2, ecto_sql: 3.11.3

Database Adapter and Versions (postgrex, myxql, etc)

postgrex: 0.18.0

Current behavior

json_extract_path or [] variant doesn't support referencing columns from other tables. The Ecto query below fails to compile:

create table(:test_posts) do
  add(:text, :text)
  add(:user_id, references(:users))
end

create table(:test_users) do
  add(:post_stats, :jsonb)
end

from(u in TestUser, join: p in assoc(u, :posts), select: %{stat: u.post_stats[p.id]}) |> Repo.all

Expected behavior

It would be very convenient if we could pass references to other columns in json_extract_path. I'm not sure about other DBs but for PostgreSQL I can see that json_extract_path transforms to this SQL:

users.post_stats #> '{"my", "path", "inside", "json"}'

Because it uses such syntax of array (as a '{}' string) it's clear why it doesn't work today. However, PostgreSQL supports another syntax where it's possible to reference columns:

users.post_stats #> array['my', 'path', test_posts.id, 'inside', 'json']::text[]

I'm wondering if anything stops us from using the array[] syntax and allow referencing columns from other tables? If no, I can try to work on the fix.

@greg-rychlewski
Copy link
Member

greg-rychlewski commented Nov 2, 2024

Hi,

I think there is a chance we can do this. The main things I can think of that we'd need to be careful about:

  1. Does it work for both json and jsonb. Right now json_extract_path is agnostic between those types and we probably want to keep it that way.
  2. Does it work for the same postgres versions as the array string.
  3. We do some validation in Ecto to make sure the extraction path is valid by comparing against the fields in the embedded schemas. So we probably have to make some adjustments to ignore the validation if a column is provided.
  4. Do we take any significant performance hit making the change. It seems a bit wasteful to do it even if there are no column references. So maybe we have to track whether a column was provided and change the syntax.

@greg-rychlewski
Copy link
Member

One more thing we should probably think about is dynamic paths.

Since the query bindings are compile-time constructs I don't think we can allow them in runtime evaluated paths unless we use dynamic. We might want to evaluate how large a change that would be.

For example if it was prohibitively large then we'd need to decide if we want to allow bindings in compile-time paths only even though users would probably really want them in runtime paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants