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.
