ARel silently ignoring "where" and "offset" clauses on updates

Hey,

I've submitted some patches to ARel that (supposedly) fix the following bugs:

https://rails.lighthouseapp.com/projects/8994/tickets/6058-update_all-ignores-conditions-when-orders-and-limit-options-are-supplied#ticket-6058-7
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/6459-arel-update-method-silently-drops-offset-clause#ticket-6459-2

Before applying these patches, ARel has the following behaviour:

ruby > News.where("highlighted = ?",
true).order("highlighted_at").limit(1).offset(1).update_all(:highlighted
=> false)
UPDATE "news" SET "highlighted" = 'f' WHERE "news"."id" IN (SELECT
"news"."id" FROM "news" ORDER BY highlighted_at LIMIT 1)

Note how both the where("highlighted = ?", true) and offset(1) clauses
are ignored. After the patches, it yields the following:
UPDATE "news" SET "highlighted" = 'f' WHERE (highlighted = 't') AND
"news"."id" IN (SELECT "news"."id" FROM "news" ORDER BY highlighted_at
LIMIT 1 OFFSET 1)

Feedback on these would be appreciated.

Hugo Peixoto

I think the "WHERE (highlighted = 't')" part should be put in the inner query for this to work correctly.

The query you've shown us selects id of the first highlighted_at object. It might or might not pass the update condition and so the complete query will update one or no rows, even though further in the defined order there might be other rows that would pass the update condition.

Adam

I believe you are right. I'm replacing the patches.

Hugo Peixoto