This project is archived and is in readonly mode.
Arel update method silently drops OFFSET clause
Reported by GSnyder | February 21st, 2011 @ 07:10 PM
Arel::Crud::update silently fails to propagate OFFSET clauses to the update manager, probably because common SQL dialects do not support OFFSET in UPDATEs.
For example, the following code
relation = Thing.order("freshness ASC").offset(20).limit(10)
relation.update_all(["broken = ?", true])
executes as "UPDATE things SET broken = 1 ORDER BY freshness ASC LIMIT 10".
The bug (I would claim) is that the bogus update attempt is not detected and rejected. Instead, erroneous SQL is emitted and allowed to corrupt the database by updating random records.
AR shouldn't try to protect programmers from every possible manifestation of their own folly. However, in this case, Arel is actually partially responsible for creating the problem: it turns an invalid request into a valid-but-inaccurate SQL statement. Better to pass the OFFSET along and let the database choke on the invalid query syntax.
I'm using MySQL, but it looks like Postgres does not even allow LIMIT in updates. I would be curious to know what happens when the example above is run against Postgres.
Comments and changes to this ticket
-
Hugo Peixoto March 6th, 2011 @ 01:54 AM
- Tag set to “arel”
In postgresql, that query gets converted to the something like the following:
UPDATE things SET broken = 1 WHERE id IN (SELECT id FROM things ORDER BY freshness ASC LIMIT 10)The OFFSET clause is ignored independently of the RDBMS. I made a patch that fixes this by converting the query to the following:
UPDATE things SET broken = 1 WHERE id IN (SELECT id FROM things ORDER BY freshness ASC LIMIT 10 OFFSET 20)Unfortunately, this is invalid in MySQL, as it doesn't support LIMIT/OFFSET in subqueries. One possible work-around (http://forums.mysql.com/read.php?86,14788,239000#msg-239000) is to create the following query:
UPDATE things SET broken = 1 WHERE id IN (SELECT id FROM (SELECT id FROM things ORDER BY freshness ASC LIMIT 10 OFFSET 20) alias)The patch I'm uploading does both. The first commit adds the ability to specify an OFFSET clause in an UPDATE statement, while the second commit adds this workaround so that it works in MySQL too.
Comments on this are welcome.
-
Hugo Peixoto March 6th, 2011 @ 11:01 AM
The previous patch had a problem. The WHERE conditions should not be in the parent UPDATE statement, but in the subselect one. This is related to Bug #6058").
The patch I'm attaching now removes the WHERE conditions from the UPDATE statement. As there is a bug regarding the where clauses propagation, the patch in the aforementioned ticket should also be applied.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
GSnyder