`delete_all` triggers error "relation "my_cte" does not exist"

[nbrustein]

trafficstars

Using ActiveRecord 5.2.3, the following code works (with the generated sql included below)

Widget.with(my_cte: "select id from widgets limit 1").joins('join my_cte using (id)').delete_all

DELETE FROM "widgets"
WHERE "widgets"."id" IN ( 
    WITH "my_cte" AS (
            SELECT id
            FROM widgets
            LIMIT 1
    )
    SELECT "widgets"."id"
    FROM "widgets"
    JOIN my_cte USING (id)
)

If I do the same thing without the join, however, it generates the error "relation "my_cte" does not exist" (with the generated sql included below)

Widget.with(my_cte: "select id from widgets limit 1").where("id in (select id from my_cte)").delete_all

DELETE FROM 
"widgets" WHERE 
(id in (select id from my_cte))

The issue seems to have something to do with the following lines in activerecord-5.2.3/lib/active_record/relation.rb#delete_all. It behaves differently if there are joins present or not:

stmt = Arel::DeleteManager.new
stmt.from(table)
if has_join_values? || has_limit_or_offset?
  @klass.connection.join_to_delete(stmt, arel, arel_attribute(primary_key))
else
  stmt.wheres = arel.constraints
end
affected = @klass.connection.delete(stmt, "#{@klass} Destroy")

Using ActiveRecord 6.0.0, the delete_all method has been completely refactored, and neither one of the approaches described above work. Both generate the error "relation "my_cte" does not exist".

Here is the relevant section of activerecord-6.0.0/lib/active_record/relation.rb#delete_all

stmt = Arel::DeleteManager.new
stmt.from(arel.join_sources.empty? ? table : arel.source)
stmt.key = arel_attribute(primary_key)
stmt.take(arel.limit)
stmt.offset(arel.offset)
stmt.order(*arel.orders)
stmt.wheres = arel.constraints

affected = @klass.connection.delete(stmt, "#{@klass} Destroy")

I'm not sure if it makes sense to think of this as a bug in this gem, or a bug in activerecord. I don't totally understand the issue, but it seems like this gem is using features that are supported in arel, but activerecord is doing things in such a way as to break those arel features. Is that right?

[kmurph73]

Hmm, yeah that's a strange edge case. Thanks for the detailed diagnosis. I'm not really sure what the proper solution is. Will have to muck around with the gem a bit. Am certainly open to ideas 🤷‍♂️

[booleanbetrayal]

@kmurph73 - is it worth trying to organize support around CTEs in the flagship ActiveRecord PostgreSQL adapter? Seems like this project would be a good reference point. I am no way familiar with the AR internals, but wouldn't mind helping to rally support / sponsor bounties / etc.

[kmurph73]

@booleanbetrayal I definitely agree that CTEs should be in AR standard since they're so powerful. I'm not sure of the best way to campaign for that... do we need to bug somebody like Aaron Patterson? I worry that throwing another GH issue onto the pile would go nowhere fast.

[simi]

Hello!

:information_source: I'm not Aaron Patterson (even I dream about that sometimes :smile:), but I tried to port with to active record (for now without recursive support). I hope it will get merged. :pray: Feel free to take a look and suggest any changes.

https://github.com/rails/rails/pull/37973

[simi]

I have found out the problem with delete_all/update_all and fixed it in my pull request. It needs to patch some Arel classes. I'm not sure if it is worth it to backport here.