This project is archived and is in readonly mode.

Support for :joins in ActiveRecord::Base#update_all

#2558

I needed to update records based on data in another table so I added support for :joins in ActiveRecord::Base#update_all.

Example usage: (from rdoc)


Book.update_all "rating = 'high'", "title LIKE '%Rails%' and authors.name = 'David'", :joins => :authors

It also works on named_scopes:


Book.by_author('David').update_all("rating = 'high'")

where


class Book < ActiveRecord::Base
  belongs_to :author
  named_scope :by_author lambda{|name|
    {:conditions => {:author => {:name => name}}, :joins => :author}
  }

There is also a fork on github at http://github.com/kjeldahl/rails...

Reported by Jacob Kjeldahl · April 24th, 2009 @ 02:14 PM

Activity

CancelProfileIsBroken
- Tag changed from 2.3.2, :joins, active_record, patch, update_all to 2.3.2, :joins, active_record, bugmash, patch, update_all
August 6th, 2009 @ 02:12 PM
Jacob Kjeldahl
Here is the patch file
- joins_in_update_all.diff 3.29 KB
August 7th, 2009 @ 09:50 AM
Josh Sharpe
The idea makes sense, but the patch doesn't apply:

Patch does not have a valid e-mail address.

August 8th, 2009 @ 03:33 AM
John Trupiano
+1 on the idea. i have revised the patch so that it will apply cleanly (unfortunately it has my commit info and not Jacob's).

The patch, however, causes 3 sqlite test failures. It seems to me that sqlite does not support the "UPDATE tbl INNER JOIN tbl2 ON tbl.id = tbl2.tbl_id" syntax. How do we go about solving this? Do we add a method to each of the adapters (:supports_updates_with_joins?) to check before applying any :joins options? If so, do we fail or simply ignore when :joins is passed in when being used with sqlite (or any other db that doesn't support the syntax)?
- 2558-joins-in-update-all.diff 3.9 KB
August 9th, 2009 @ 06:09 PM
Rizwan Reza
verified

+1 The patch works only on 2-3-stable. All tests pass.

August 9th, 2009 @ 06:12 PM
Matt Jones
The patch looks interesting, but I don't think it's a good idea: it seems likely that update_all is limited for a good reason. Encouraging people to, essentially, bypass ActiveRecord goes against the intention of the framework.

August 9th, 2009 @ 06:22 PM
Elad Meidar
+1 Verified, -1 Patch i agree with Matt, seems like it's a better approach to leave #update_all limited and not encourage AR workarounds that will probably lead to unexpected issues.

August 9th, 2009 @ 06:51 PM
Derander
-1 verified the patch

Agreed w/ Elad, if you're going to do something like this, might as well just write the plain sql.

August 9th, 2009 @ 09:03 PM
Jeremy Kemper
- State changed from new to wontfix
- Tag changed from 2.3.2, :joins, active_record, bugmash, patch, update_all to 2.3.2, :joins, active_record, patch, update_all
Agree that dropping to SQL is clearer.

August 10th, 2009 @ 06:50 AM
Jens
- Importance changed from to
This is not true and prevents one from clean separation between objects:
```
class Parent < AR::Base
  has_many :children
  has_many :grandchildren, :through => :children, :dependent => :nullify

  def before_update
    self.grandchildren.delete_all  # this and ".clear" does NOT WORK, thus:
    self.grandchildren.disconnect_disliked_ones # see below
  end
end

class Child < AR::Base
  has_many :grandchildren
end

class Grandchild < AR::Base
  def self.disconnect_disliked_ones
    self.update_all("child_id = NULL", "brave IS FALSE")
  end
end
```
The idea is that I put the intricacies of the "disconnect" operation where the implementation is, i.e. in the grandchild. Also, the type of connection between grandchild and parent is not restricted per se.

However, if called from within a Parent instance with ID=42, this will result in an invalid SQL query like
```
UPDATE "grandchildren" 
  SET grandchildren.child_id = NULL
  WHERE (grandchildren.brave IS FALSE) 
    AND (children.parent_id = 42)
```
which will result in an error, because I cannot specify a JOIN clause.

Of course, I can use Parent.find_by_sql and specify the whole UPDATE operation verbatim, however, then the separation between Parent and Grandchild internal structure is violated.

Is it not possible to put the JOIN patch into the source and leave it to the user whether to use it or not?

Thanks,

Jens
July 19th, 2010 @ 05:10 PM
Claudio Poli
I agree on having this functionality in core.

October 1st, 2010 @ 03:30 PM
eydaimon
+1 on supporting this.

I'd want to be able to do:
```
User.find(30).tasks.update_all completed: true
```
November 12th, 2010 @ 06:43 PM