elixir-sqlite / exqlite

An SQLite3 driver for Elixir
https://hexdocs.pm/exqlite
MIT License
208 stars 47 forks source link

No such column error on update query with joins and aggregates #254

Closed Zinggi closed 1 year ago

Zinggi commented 1 year ago

Hi there :wave: Sorry for the long code samples, I haven't had the time to shrink it down to the essentials. I'll try to get it down to a much smaller example, I'll add that tomorrow, promised :crossed_fingers:

The code:

    aggregates_query =
      from(a in Album,
        left_join: t in assoc(a, :tracks),
        group_by: a.id,
        select: %{id: a.id, duration: sum(t.duration), n_tracks: count(t), mtime: max(t.mtime)}
      )

    from(a in Album,
      join: s in subquery(aggregates_query),
      on: s.id == a.id,
      update: [set: [duration: s.duration, n_tracks: s.n_tracks, mtime: s.mtime]]
    )
    |> Repo.update_all([])

Full error:

     ** (Exqlite.Error) no such column: st1
     UPDATE "albums" AS a0 SET "duration" = s1."duration", "n_tracks" = s1."n_tracks", "mtime" = s1."mtime" FROM (SELECT sa0."id" AS "id", sum(st1."duration") AS "duration", count(st1) AS "n_tracks", max(st1."mtime") AS "mtime" FROM "albums" AS sa0 LEFT OUTER JOIN "tracks" AS st1 ON st1."album_id" = sa0."id" GROUP BY sa0."id") AS s1 WHERE (s1."id" = a0."id")

Schemas:

  schema "albums" do
    field(:name, :string)
    field(:thumbnail, :string)
    field(:date, :string)

    field(:duration, :float)
    field(:n_tracks, :integer)
    field(:mtime, :naive_datetime)

    belongs_to(:artist, Artist)
    has_many(:tracks, Track)
  end

  schema "tracks" do
    field(:title, :string)
    field(:url, :string)
    field(:thumbnail, :string)
    field(:date, :string)
    field(:duration, :float)
    field(:genre, :string)
    field(:track, :integer)
    field(:disc, :integer)
    field(:mtime, :naive_datetime)
    field(:duplicates, {:array, :string})

    belongs_to(:artist, Artist)
    belongs_to(:album, Album)
  end

The exact same query works with postgres.

warmwaffles commented 1 year ago

That's weird, it's right there in the LEFT OUTER JOIN

UPDATE "albums" AS a0 
SET "duration" = s1."duration", 
    "n_tracks" = s1."n_tracks", 
    "mtime" = s1."mtime" 
FROM (
    SELECT 
        sa0."id" AS "id", 
        sum(st1."duration") AS "duration", 
        count(st1) AS "n_tracks", 
        max(st1."mtime") AS "mtime" 
    FROM "albums" AS sa0 
    -- st1 defined right here   vvv
    LEFT OUTER JOIN "tracks" AS st1 ON st1."album_id" = sa0."id" 
    GROUP BY sa0."id"
) AS s1 
WHERE (s1."id" = a0."id")
Zinggi commented 1 year ago

That's weird, it's right there in the LEFT OUTER JOIN

Yeah, looks correct to me too. But I don't really know sqlite, so :shrug:


As promised, I managed to reduce it down a bit (but still using the same bloated schema):

Here is the updated sample:

    aggregates_query =
      from(a in Album,
        left_join: t in assoc(a, :tracks),
        group_by: a.id,
        select: %{id: a.id, duration: count(t)}
      )

    from(a in Album,
      join: s in subquery(aggregates_query),
      on: s.id == a.id
    )
    |> Repo.all()

New error:

     ** (Exqlite.Error) no such column: st1
     SELECT a0."id", a0."name", a0."thumbnail", a0."date", a0."duration", a0."n_tracks", a0."mtime", a0."artist_id" FROM "albums" AS a0 INNER JOIN (SELECT sa0."id" AS "id", sum(st1) AS "duration" FROM "albums" AS sa0 LEFT OUTER JOIN "tracks" AS st1 ON st1."album_id" = sa0."id" GROUP BY sa0."id") AS s1 ON s1."id" = a0."id"

The same query still works with postgres.


I also looked at the generated SLQ with Repo.to_sql, which is the same for both postgres and sqlite, so I'm assuming the issue is not in https://github.com/elixir-sqlite/ecto_sqlite3, correct?

warmwaffles commented 1 year ago

What's the rendered output for postgres?

Zinggi commented 1 year ago

What's the rendered output for postgres?

What do you mean? The output of to_sql is the same as in the error message above. Or do mean the result? Its just a [%Album{..}, ..]. Or something else?

warmwaffles commented 1 year ago

The rendered sql output. I just want to compare the generated SQL. I suspect with how the left outer join is specified we generated the wrong statement.

Zinggi commented 1 year ago

I still don't understand what "the rendered sql output" means. Maybe the one from the debug logs? Is that different to the to_sql output?

Debug log:

20:31:52.713 [debug] QUERY OK source="albums" db=0.3ms
SELECT a0."id", a0."name", a0."thumbnail", a0."date", a0."duration", a0."n_tracks", a0."mtime", a0."artist_id" FROM "albums" AS a0 INNER JOIN (SELECT sa0."id" AS "id", count(st1) AS "n_tracks" FROM "albums" AS sa0 LEFT OUTER JOIN "tracks" AS st1 ON st1."album_id" = sa0."id" GROUP BY sa0."id") AS s1 ON s1."id" = a0."id" []
Zinggi commented 1 year ago

That was postgres, here is the one from sqlite with the rollback:

20:36:01.624 [debug] QUERY ERROR source="albums" db=0.0ms
SELECT a0."id", a0."name", a0."thumbnail", a0."date", a0."duration", a0."n_tracks", a0."mtime", a0."artist_id" FROM "albums" AS a0 INNER JOIN (SELECT sa0."id" AS "id", count(st1) AS "n_tracks" FROM "albums" AS sa0 LEFT OUTER JOIN "tracks" AS st1 ON st1."album_id" = sa0."id" GROUP BY sa0."id") AS s1 ON s1."id" = a0."id" []
20:36:01.624 [debug] QUERY OK db=0.0ms
rollback []
Zinggi commented 1 year ago

I didn't had debug log on before, hopefully that helps

warmwaffles commented 1 year ago

Sorry @Zinggi the "rendered sql output" refers to that exact debug output you put there. The to_sql turns the Ecto.Queryable into the literal string that will be provided to the database.

Thanks for providing that info. I'll get to it tonight.

If you could also provide the create table bits for albums and tracks I should be able to make an integration test to figure it out

Zinggi commented 1 year ago

Ok, great, thanks for looking into it!

Here are the relevant bits from the migrations:

create table(:albums) do
  add :duration, :float
end

create table(:tracks) do
  add :duration, :float
  add :album_id, references(:albums, on_delete: :delete_all)
end

I tried to condense it a bit this time, I think this should be all that matters. I've also got some unique_indexes on my table, but I don't think they are relevant for this issue here.

warmwaffles commented 1 year ago

One work around I think should work here is to change the count from

select: %{id: a.id, duration: sum(t.duration), n_tracks: count(t), mtime: max(t.mtime)}

to

select: %{id: a.id, duration: sum(t.duration), n_tracks: count(a), mtime: max(t.mtime)}
warmwaffles commented 1 year ago

OH it's complaining about a column. I should have read that more closely. You need to specify which column on t to count. You could do t.id or I believe count(1). Postgres auto expands this IIRC but Sqlite does not. You have to be explicit.

Zinggi commented 1 year ago

Oh wow, that does work :tada: Thank you so much for spotting that :+1:

Both count(1) and count(t.id) do indeed work.

count(a) did not work either, with a similar message as for count(t).


So I guess this isn't a bug really, just a bad error message maybe? Or should the query be generated differently for sqlite?

warmwaffles commented 1 year ago

The error message is directly from SQLite. Had I been paying attention this could be resolved faster.

As for it generating differently. I don't think that would be wise, although having it raise an exception would probably be a good idea to let the caller know that a column or value must be specified.

Zinggi commented 1 year ago

having it raise an exception would probably be a good idea to let the caller know that a column or value must be specified.

Yeah, I think this would be great :+1:


I'm currently porting an application over to sqlite to be able to run it on android and other error messages have been great so far and made it quite straight forward. For instance, I was using push in an update query and it gave me this nice error message:

** (Ecto.QueryError) Arrays are not supported for SQLite3 in query:

So great job :1st_place_medal: