Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Arel update method silently drops OFFSET clause

#6459

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.

Reported by GSnyder · February 21st, 2011 @ 07:10 PM

State: new
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Hugo Peixoto
    Hugo Peixoto
    • 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.

    March 6th, 2011 @ 01:54 AM

  2. Hugo Peixoto
    Hugo Peixoto

    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.

    March 6th, 2011 @ 11:01 AM