I notice on Rails 3.0.4 that the following two statements generate
different results.
State.where(:abbreviation => 'TX').where(:abbreviation => 'NE')
SQL: SELECT `states`.* FROM `states` WHERE (`states`.`abbreviation` =
'TX') OR (`states`.`abbreviation` = 'NE')
State.where(:abbreviation => 'TX').where("abbreviation = 'NE'")
SQL: SELECT `states`.* FROM `states` WHERE (`states`.`abbreviation` =
'TX') AND (`states`.`abbreviation` = 'NE')
Using Rails 3.0.3 they both generated AND clauses which seems to me to
be correct.
Is this a bug or by design please?
Colin
The OR functionality is new, but intentional. See
Relation::QueryMethods#collapse_wheres. Since the idea of having a
column being equal to two different values doesn't make sense,
equalities with the same left-hand side get combined with OR, which is
more likely the desired result if two scopes are merged and both
include equality conditions on the same column.
Since your example above used a string in the second where call, it
didn't result in an Arel::Nodes::Equality, so it wasn't ORed.
I doubt that. There are a lot of cases (especially when using
search/filter functionality of application) where you want it to
generate an empty set because there are no matching records. Everybody
everywhere in current code assumes that the condition will be added
with 'AND' operator and now we are breaking this assumption ?
If I want to have 'OR' then I would build the query this way:
State.where(:abbreviation => ['TX', 'NE'])
or using longer but perfectly fine and valid arel version...
An example:
def where_something(scope, value)
scope.where(:column => value)
end
This code is ambiguous now. I do not know what sql it is going to
generate. Will it be
"AND column = value"
or
"(... OR column = value)" ?
Please revert it to the original behavior and provide a different way
of achieving this like:
State.possible(:abbreviation => 'TX').possible(:abbreviation => 'NE') =>
sql: "states.abbrevation = 'TX' OR states.abbrevation = 'NE"
or whatever different method name that reveals the intentions clearly
Robert Pankowecki
Even on just that point alone, this is a really bad API idea - it is
completely inconsistent, and this will cause subtle and confusing bugs
when people change between two otherwise-identical forms of where()
argument.
And to do it at all breaks the relational algebra idea badly. If I
pass a scope to something, and it does several different queries on it
using its own where scopes, each of those scopes should further
restrict the original scope, not broaden it.
With this commit, now neither callers nor callees have any idea what
where(:hash => arguments) will result in - whether the returned
results will match the conditions hash - unless they have constructed
the entire scope from scratch.
I also struggle to understand how it was decided that a minor point
patch release - for a security issue no less - should include an API
behavior change that will cause preexisting code to start getting
completely different query results.
OK, thanks for that. If that is the way it is supposed to be then
that is the way it is supposed to be. It seems odd to me that
chaining a second where *adds* to the dataset of the first where. It
would seem particularly unexpected in the case
items = Model.where( :x => y)
...
...
myitems = items.where(:x => z)
resulting in myitems having more records than items, and in particular
records that do not satisfy :x => z. Note that at this point the code
would not necessarily 'know' the details of the first scope.
Colin
Sound arguments. I didn't implement collapse_wheres, just in case a
witch hunt starts. I was just the first person awake this morning to
pose the answer. :)
The change probably shouldn't have made it into a point release, but I
do find a certain convenience and logic to it, and I long for a day
when we aren't hacking about trying to make a String and a Hash act
like they're the same thing. Hashes aren't strings, and (IMHO, and I
know there are people on the core team who disagree) it would be
better if we gradually eliminated hand coded SQL string conditions and
relied more heavily on hashes and other data structures, converting
them to SQL when needed via ARel. They're already a last resort in my
own code and I expect them to be relatively brittle when I use them.
The intentional change fixes this bug:
If there is a different way to fix that ticket, I'm happy to apply
patches.
--
Aaron Patterson
http://tenderlovemaking.com/
The different way is to make both cases return an empty array. David
(who created the ticket) expects something different but I am not sure
that this is what most people expect.
Could we provide a different method for such behavior as expected by David ?
Robert Pankowecki
Perhaps:
#where_in == OR
#where == AND
???
--
*** E-Mail is NOT a SECURE channel ***
James B. Byrne mailto:Byr...@Harte-Lyne.ca
Harte & Lyne Limited http://www.harte-lyne.ca
9 Brockley Drive vox: +1 905 561 1241
Hamilton, Ontario fax: +1 905 561 0757
Canada L8E 3C3
User.where(:foo => 1).and(:foo => 2)
User.where(:foo => 1).or(:foo => 2)
--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To post to this group, send email to rubyonra...@googlegroups.com.
To unsubscribe from this group, send email to rubyonrails-co...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
It seems odd to me that chaining a second where *adds* to the dataset of the first where.
default_scope where( :foo => 1 )
default_scope where( :foo => 2 ) # effectively negates previous default scopedefault_scope and( :foo => 2 ) # same as abovedefault_scope or( :foo => 2 ) # merges with original
User.or( :foo => 2 )
User.where( :foo => 2 )
Just want to get this onto the table, "what most people expect" is not the only measure of a good API choice.
To specifically answer Aaron's question, for #4598, I think the fix for that would be for subsequent `default_scope`s to just overwrite previous ones entirely. Not to merge with them.
> Just want to get this onto the table, "what most people expect" is not the only measure of a good API choice.
>
> To specifically answer Aaron's question, for #4598, I think the fix for that would be for subsequent `default_scope`s to just overwrite previous ones entirely. Not to merge with them.
>
> For the broader question which seems to be what this thread is about, ie. what to do with: User.where(:foo => 1).where(:foo => 2) how about adding alias `and` as an alias for `where`, and adding `or` as an alias for "where that does ORs". So the syntax could be:
>
> User.where(:foo => 1).and(:foo => 2)
>
> And:
>
> User.where(:foo => 1).or(:foo => 2)
>
> I'm sure I don't need to state what SQL each of those turn into (the real test for a good API :)
I suspect the 'and' behavior is already pretty well understood as the result of chaining 'where's. What about an explicit one for the 'or' case:
User.where(:foo => 1).or_where(:foo => 2)
The only issue I could see with this syntax is that it's somewhat unclear what should happen here:
User.where(:foo => 1).where(:bar => 2).or_where(:bar => 3)
as it could be interpreted as either
Version A: (foo = 1 AND bar = 2) OR (bar = 2)
or
Version B: (foo = 1) AND (bar = 1 OR bar = 2)
The issue is especially tricky once you're constructing relations in more than one place. For instance, this should do something reasonable:
r = User.where(:foo => 1)
def foo(some_relation)
some_relation.where(:bar => 1).or_where(:bar => 2) # probably expects version B SQL
end
foo(r) # probably expects version B SQL
but so should this:
r2 = User.where(:foo => 1).where(:bar => 1)
def other_foo(some_relation)
some_relation.or_where(:bar => 2)
end
other_foo(r2) # probably expects version A SQL
the difficulty is that the sequence of method calls on the relation are identical, they're just in different scopes.
On the other hand, isn't this sort of issue exactly why Arel has operators to combine scopes? The chained notation is alright for some things, but it's never going to be sufficient to create arbitrary SQL without a lot of fussing.
--Matt Jones
r = User.where( :foo => 1 )
r = r.and( r.where( :bar => 1 ).or( :bar => 2 ))
r = User.where( :foo => 1 ).where( :bar => 1 )r = r.or( :bar => 2 )
--
Robert Pankowecki
It's fixed in 3.0.5.rc1 and in master. :-)