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

Invalid SQL generated when loading an aggregate that has a sort, for a resource that also has a sort #217

Closed
sevenseacat opened this issue Feb 29, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@sevenseacat
Copy link
Contributor

sevenseacat commented Feb 29, 2024

Probably kind of a niche edge case, but one I just came across!

Describe the bug

  • I have an Artist resource that has_many Albums: has_many :albums, Tunez.Music.Album
  • Album has a year_released attribute: attribute :year_released, :integer, allow_nil?: false
  • The Artist resource defines an aggregate to calculate when the most recent album was released:
    first :latest_album_year_released, :albums, :year_released do
      sort year_released: :desc
    end
  • Say I decide to now order albums by name alphabetically, in a global preparation:
  preparations do
    prepare build(sort: [name: :desc])
  end

Now trying to load an Artist with the aggregate generates an SQL error:

iex> Tunez.Music.get(Tunez.Music.Artist, "faedafed-fee1-4f32-b42f-b8fb95d68f75", 
...> load: [:latest_album_year_released])
[debug] QUERY ERROR source="artists" db=1.4ms queue=3.9ms idle=1648.4ms
SELECT a0."id", a0."name", a0."biography", a0."previous_names", a0."inserted_at", a0."updated_at", 
a0."created_by_id", a0."updated_by_id", s1."latest_album_year_released"::bigint::bigint FROM 
"artists" AS a0 LEFT OUTER JOIN LATERAL (SELECT (array_agg(sa0."year_released"::bigint ORDER BY  
sa0."name" DESC  sa0."year_released" DESC ))[1]::bigint AS "latest_album_year_released", 
sa0."artist_id" AS "artist_id" FROM "public"."albums" AS sa0 WHERE (a0."id" = sa0."artist_id") 
GROUP BY sa0."artist_id") AS s1 ON TRUE WHERE (a0."id"::uuid = $1::uuid) LIMIT $2 
["faedafed-fee1-4f32-b42f-b8fb95d68f75", 2]
↳ anonymous fn/3 in AshPostgres.DataLayer.run_query/2, at: lib/data_layer.ex:686
{:error,
 %Ash.Error.Unknown{
   errors: [
     %Ash.Error.Unknown.UnknownError{
       error: "** (Postgrex.Error) ERROR 42601 (syntax_error) syntax error at or near 
\"sa0\"\n\n    query: SELECT a0.\"id\", a0.\"name\", a0.\"biography\", a0.\"previous_names\", 
a0.\"inserted_at\", a0.\"updated_at\", a0.\"created_by_id\", a0.\"updated_by_id\", 
s1.\"latest_album_year_released\"::bigint::bigint FROM \"artists\" AS a0 LEFT OUTER JOIN 
LATERAL (SELECT...

The SQL for the order by is invalid - ORDER BY sa0."name" DESC sa0."year_released" DESC (missing a comma).

Come to think of it this might actually be an Ecto issue, not an Ash issue? Does the query even make sense? Should the order on the aggregate come before the default ordering?

Expected behavior

Correct SQL to be generated!

Runtime

  • Elixir version 1.15.7-otp-26
  • Erlang version 26.1.2
  • OS MacOS somethingthelatest
  • PostgreSQL 15.5
  • Ash version main at e238804b464ecd7922af522c319894bbc773292b
  • AshPostgres 1.5.9
@sevenseacat sevenseacat added bug Something isn't working needs review labels Feb 29, 2024
@zachdaniel
Copy link
Contributor

I think that if the aggregate has a sort, it should replace the destination action's sort. And also yeah the syntax issue is our issue 🤦

@zachdaniel
Copy link
Contributor

Fixed in 1.5.13

@zachdaniel
Copy link
Contributor

But will adjust Ash to make an aggregate sort replace a the destination action's sort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants