Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Pass a base relation to build_default_scope when joining #14154

Merged
merged 1 commit into from

4 participants

@al2o3cr

This PR fixes the problem noted in #12770 where the clause added by the default_scope did not correctly pick up the aliased table name (causing the condition to be on "categorizations"."special" instead of "special_categorizations_categories"."special"). This is apparently an error on some versions of Postgres, but produces incorrect results even when it can be successfully executed.

Before this patch, the SQL generated in the added test case is:

SELECT "categories"."id" AS t0_r0, "categories"."name" AS t0_r1, "categories"."type" AS t0_r2, "categories"."categorizations_count" AS t0_r3, "categorizations"."id" AS t1_r0, "categorizations"."category_id" AS t1_r1, "categorizations"."named_category_name" AS t1_r2, "categorizations"."post_id" AS t1_r3, "categorizations"."author_id" AS t1_r4, "categorizations"."special" AS t1_r5 FROM "categories"
LEFT OUTER JOIN "categorizations" "special_categorizations_categories"
    ON "special_categorizations_categories"."category_id" = "categories"."id"
    AND "categorizations"."special" = 't' 
INNER JOIN "categorizations" ON "categories"."id" = "categorizations"."category_id"
WHERE "categorizations"."author_id" = ?

After the patch, the SQL is (mostly) correct:

SELECT "categories"."id" AS t0_r0, "categories"."name" AS t0_r1, "categories"."type" AS t0_r2, "categories"."categorizations_count" AS t0_r3, "categorizations"."id" AS t1_r0, "categorizations"."category_id" AS t1_r1, "categorizations"."named_category_name" AS t1_r2, "categorizations"."post_id" AS t1_r3, "categorizations"."author_id" AS t1_r4, "categorizations"."special" AS t1_r5 FROM "categories"
LEFT OUTER JOIN "categorizations" "special_categorizations_categories" 
    ON "special_categorizations_categories"."category_id" = "categories"."id"
    AND "special_categorizations_categories"."special" = 't'
INNER JOIN "categorizations" ON "categories"."id" = "categorizations"."category_id" WHERE "categorizations"."author_id" = ?

The SQL is only "mostly correct" because the SELECT isn't picking up the table alias and results in incorrect data loaded into special_characterizations (both before and after the patch). I'll open a separate issue for that (#14155).

/cc @tenderlove

@al2o3cr al2o3cr Pass a base relation to build_default_scope when joining
This allows the default scope to be built using the current table alias.
Resolves #12770
70a5e56
@nixme

friendly ping to @tenderlove. Will these fixes make it for 4.1.0?

@tenderlove tenderlove merged commit c81e4e6 into rails:master

1 check passed

Details default The Travis CI build passed
@al2o3cr al2o3cr referenced this pull request from a commit in al2o3cr/rails
@al2o3cr al2o3cr Correctly handle joining scoped associations with table aliases.
Backport of #14154 to 4-0-stable.
90b9234
@al2o3cr al2o3cr referenced this pull request from a commit in al2o3cr/rails
@al2o3cr al2o3cr Correctly handle joining scoped associations with table aliases.
Backport of #14154 to 4-0-stable.
9a6dcf0
@chancancode chancancode referenced this pull request from a commit
@al2o3cr al2o3cr Correctly handle joining scoped associations with table aliases.
Backport of #14154 to 4-0-stable.
4262797
@stgeneral stgeneral referenced this pull request in radar/paranoia
Merged

Remove table_name from where methods #212

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 21, 2014
  1. @al2o3cr

    Pass a base relation to build_default_scope when joining

    al2o3cr authored
    This allows the default scope to be built using the current table alias.
    Resolves #12770
This page is out of date. Refresh to see the latest.
View
2  activerecord/lib/active_record/associations/join_dependency/join_association.rb
@@ -54,7 +54,7 @@ def join_constraints(foreign_table, foreign_klass, node, join_type, tables, scop
end
scope_chain_index += 1
- scope_chain_items.concat [klass.send(:build_default_scope)].compact
+ scope_chain_items.concat [klass.send(:build_default_scope, ActiveRecord::Relation.create(klass, table))].compact
rel = scope_chain_items.inject(scope_chain_items.shift) do |left, right|
left.merge right
View
6 activerecord/lib/active_record/scoping/default.rb
@@ -93,14 +93,14 @@ def default_scope(scope = nil)
self.default_scopes += [scope]
end
- def build_default_scope # :nodoc:
+ def build_default_scope(base_rel = relation) # :nodoc:
if !Base.is_a?(method(:default_scope).owner)
# The user has defined their own default scope method, so call that
evaluate_default_scope { default_scope }
elsif default_scopes.any?
evaluate_default_scope do
- default_scopes.inject(relation) do |default_scope, scope|
- default_scope.merge(unscoped { scope.call })
+ default_scopes.inject(base_rel) do |default_scope, scope|
+ default_scope.merge(base_rel.scoping { scope.call })
end
end
end
View
9 activerecord/test/cases/associations/inner_join_association_test.rb
@@ -117,4 +117,13 @@ def test_find_with_conditions_on_through_reflection
assert_equal [author], Author.where(id: author).joins(:special_categorizations)
end
+
+ test "the default scope of the target is correctly aliased when joining associations" do
+ author = Author.create! name: "Jon"
+ author.categories.create! name: 'Not Special'
+ author.special_categories.create! name: 'Special'
+
+ categories = author.categories.includes(:special_categorizations).references(:special_categorizations).to_a
+ assert_equal 2, categories.size
+ end
end
View
1  activerecord/test/models/category.rb
@@ -22,6 +22,7 @@ def self.what_are_you
end
has_many :categorizations
+ has_many :special_categorizations
has_many :post_comments, :through => :posts, :source => :comments
has_many :authors, :through => :categorizations
Something went wrong with that request. Please try again.