> 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. :)
Understood. :-) > 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 agree with this reasoning, too. Under the 3.0.4 logic, we won't be able to chain independent scopes together on the fly with any confidence. E.g., a loop that iterates over various input conditions and adds scopes one by one based on the input. +1 in favor of reverting to the original behavior: chaining 'where' clauses should consistently use 'AND'. Brian On Feb 16, 4:24 am, Ernie Miller <[email protected]> wrote: > On Feb 16, 2011, at 6:38 AM, Will Bryant <[email protected]> wrote: > > > > > On Thu, Feb 17, 2011 at 12:21 AM, Ernie Miller <[email protected]> wrote: > >> 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. > > > 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. > > 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. -- 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 [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
