This project is archived and is in readonly mode.
default_scope treats hashes and relations inconsistently when overwriting
Reported by David Chelimsky | May 14th, 2010 @ 10:41 PM | in 3.1
In working on #4583, Brian and I discovered some inconsistent behavior. Here are a couple of tests that should both behave the same way, but one passes and one fails:
http://github.com/dchelimsky/rails/commit/6180043572ff30a2e01690b86...
require "cases/helper"
require 'models/developer'
class ArelBugTest < ActiveRecord::TestCase
fixtures :developers
# We would expect both to pass or both to fail, but one passes and one fails.
# fails
def test_default_scope_called_twice_with_relations
klass = Class.new(Developer)
klass.class_eval do
default_scope where(:name => 'David')
default_scope where(:name => 'Jamis')
end
assert_equal ["Jamis"], klass.all.map(&:name).uniq.sort
end
# passes
def test_default_scope_called_twice_with_hashes
klass = Class.new(Developer)
klass.class_eval do
default_scope :conditions => { :name => 'David' }
default_scope :conditions => { :name => 'Jamis' }
end
assert_equal ["Jamis"], klass.all.map(&:name).uniq.sort
end
end
Comments and changes to this ticket
-
Neeraj Singh May 15th, 2010 @ 04:41 AM
In the first test case where relation is being passed the sql query is being built something like this.
> us = User.unscoped; > us=us.merge(User.where(:name => 'David')); > us=us.merge(User.where(:name => 'Jamis')); > us.to_sql => "SELECT \"users\".* FROM \"users\" WHERE (\"users\".\"name\" = 'Jamis')"
In the second case where hash is being passed the sql query is being built something like this.
> us = User.unscoped; > us = us.where(:name => 'David'); > us = us.where(:name => 'Jamis'); > us.to_sql => "SELECT \"users\".* FROM \"users\" WHERE (\"users\".\"name\" = 'David') AND (\"users\".\"name\" = 'Jamis')"
Before I suggest a fix, I would like to know what direction to proceed.
Based on ticket #4583 I should assume that the desired behavior should be the second sql so that one can pass more than one default_scope and all the conditions passed should be ANDed to build the final sql.
-
David Chelimsky May 15th, 2010 @ 04:56 AM
I would expect the same column to get ORed (an IN clause) and different columns to get ANDed:
user = User.unscoped user = user.where(:name => 'David') user = user.where(:name => 'Jamis') user = user.where(:role => 'admin') user.to_sql => SELECT \"users\".* FROM \"users\" WHERE (\"users\".\"name\" in ('David','Jamis')) AND (\"users\".\"role\" = 'admin')
Agree?
-
Santiago Pastorino May 15th, 2010 @ 08:49 PM
- Milestone cleared.
- Tag changed from activerecord, bug to activerecord, bug, bugmash
- State changed from new to open
- Assigned user changed from José Valim to Santiago Pastorino
-
Santiago Pastorino May 15th, 2010 @ 09:45 PM
Test added here, i don't know if we should test this kind of things or fix in Arel and rely on his behavior.
Anyways i add the test case only for now.
I'm going to patch it. -
Neeraj Singh May 16th, 2010 @ 01:17 AM
- Tag changed from activerecord, bug, bugmash to activerecord, bug, bugmash, patch
Attached is a code patch.
@Santiago thanks for the test.
-
Rohit Arondekar August 4th, 2010 @ 11:47 AM
- Importance changed from to Low
The test still fails in 3-0-stable.
1) Failure: test_find_all_using_where_twice_should_or_the_relation(RelationTest) [test/cases/relations_test.rb:655]: <[#<Author id: 1, name: "David", author_address_id: 1, author_address_extra_id: 2>]> expected but was <[]>.
However the patch provided by Neeraj doesn't apply any more so I couldn't check if it solves the problem.
-
Rohit Arondekar August 4th, 2010 @ 01:55 PM
I applied Neeraj's patch manually and it made the test pass. Here is a combined patch for 3-0-stable with both the test and fix. Tests are passing.
-
Santiago Pastorino November 7th, 2010 @ 11:38 AM
- Milestone changed from 3.0.2 to 3.1
- Assigned user changed from Santiago Pastorino to Aaron Patterson
-
Repository November 18th, 2010 @ 01:14 AM
- State changed from open to resolved
(from [fdc591351e5a231c4da47a4b363e961ae89cc864]) collapsing same table / column WHERE clauses to be OR [#4598 state:resolved] https://github.com/rails/rails/commit/fdc591351e5a231c4da47a4b363e9...
-
Repository November 18th, 2010 @ 01:14 AM
(from [00693209ecc222842949d7cab076f89890cbd507]) collapsing same table / column WHERE clauses to be OR [#4598 state:resolved] https://github.com/rails/rails/commit/00693209ecc222842949d7cab076f...
-
Jeremy Kemper February 16th, 2011 @ 05:27 PM
- State changed from resolved to open
- Milestone changed from 3.1 to 3.0.5
http://groups.google.com/group/rubyonrails-core/browse_thread/threa...
This inconsistency seems ok in context: second hash condition overwrites the first, whereas the second .where relation ANDs on to it.
-
David Chelimsky February 17th, 2011 @ 07:56 AM
FWIW, I think that having to know that the .where and conditions work differently is confusing and puts an unnecessary burden on users, even if everything is visible in the same context. In our case, we broke the behavior of a gem by adding conditions to the default scope in a model, so we didn't have all the context in one place. Having conditions and .where work differently made it all the more confusing, which is why I submitted this ticket in the first place.
-
Aaron Patterson February 24th, 2011 @ 02:03 AM
- Milestone changed from 3.0.5 to 3.1
I think what we need to do is change this to do an AND in both cases for Rails 3.1. I don't like that we change it to an OR when combining only certain columns. It's like we're tying to read the user's mind, and I am terrible at mind reading.
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>
People watching this ticket
Attachments
Tags
Referenced by
- 4598 default_scope treats hashes and relations inconsistently when overwriting (from [fdc591351e5a231c4da47a4b363e961ae89cc864]) collaps...
- 4598 default_scope treats hashes and relations inconsistently when overwriting (from [00693209ecc222842949d7cab076f89890cbd507]) collaps...
- 6433 Chained where conditions referencing same attribute joined by OR This is expected behavior that fixes #4598. The OR will o...