[3.1] Fix ActiveRecord's completely broken where merge semantics

[haileys]

This PR fixes an ActiveRecord bug that's currently causing a github/github test to fail.

irb(main):004:0> Repository.public.private.to_sql
=> "SELECT `repositories`.* FROM `repositories`  WHERE `repositories`.`public` = 0"
irb(main):005:0> Repository.private.public.to_sql
=> "SELECT `repositories`.* FROM `repositories`  WHERE `repositories`.`public` = 1"

In my testing, ActiveRecord 2.3, 3.0, and 4.0 correctly merge these two scopes and create an unsatisfiable WHERE clause. 3.1 and 3.2 are both broken and will override earlier conditions with later conditions.

Because 4.0 behaves correctly, I feel comfortable making this change to our fork of Rails as we're essentially backporting a bug fix here.

cc @github/rails @github/rails3

[rick]

/cc @jakedouglas -- was this something like what you were seeing? (or am I
just conflating all weird query behavior?)
On Apr 19, 2014 8:19 PM, "Charlie Somerville" [email protected]
wrote:

This PR fixes an ActiveRecord bug that's currently causing a github/github
test to fail.

irb(main):004:0> Repository.public.private.to_sql=> "SELECT repositories.* FROM repositories WHERE repositories.public = 0"irb(main):005:0> Repository.private.public.to_sql=> "SELECT repositories.* FROM repositories WHERE repositories.public = 1"

In my testing, ActiveRecord 2.3, 3.0, and 4.0 correctly merge these two
scopes and create an unsatisfiable WHERE clause. 3.1 and 3.2 are both
broken and will override earlier conditions with later conditions.

Because 4.0 behaves correctly, I feel comfortable making this change to
our fork of Rails as we're essentially backporting a bug fix here.

cc @github/rails @github/rails3

You can merge this Pull Request by running

git pull https://github.com/github/rails 3-1-github+fix-broken-where-behaviour

Or view, comment on, or merge it at:

#60
Commit Summary

• fix the tests so they're not asserting wrong behaviour
• fix activerecord's completely broken where clause merge semantics

File Changes

• M activerecord/lib/active_record/relation/spawn_methods.rbhttps://github.com/[3.1] Fix ActiveRecord's completely broken where merge semantics #60/files#diff-0(17)
• M activerecord/test/cases/named_scope_test.rbhttps://github.com/[3.1] Fix ActiveRecord's completely broken where merge semantics #60/files#diff-1(6)

Patch Links:

• https://github.com/github/rails/pull/60.patch
• https://github.com/github/rails/pull/60.diff

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/60
.

[haileys]

Tests are failing here because we don't have the 3-1-github branch set up in CI. I've opened another PR (#61) based on the 3-0-github+ar-3-1 branch which does have CI. Probably should've opened this PR against that branch to begin with.