kotovalexarian-likes-github/rails--rails
1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00
Code Releases Activity

Preserve user supplied joins order as much as possible

Browse source 
Currently, string joins are always applied as last joins part, and Arel
join nodes are always applied as leading joins part (since #36304), it
makes people struggled to preserve user supplied joins order.

To mitigate this problem, preserve the order of string joins and Arel
join nodes either before or after of association joins.

Fixes #36761.
Fixes #34328.
Fixes #24281.
Fixes #12953.
This commit is contained in:
Ryuta Kamizono 2019-07-30 02:37:26 +09:00
parent 8d2e75e84c
commit b4478ae8bc
3 changed files with 65 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
activerecord/CHANGELOG.md
View file

@ -1,3 +1,9 @@
* Preserve user supplied joins order as much as possible.
Fixes #36761, #34328, #24281, #12953.
*Ryuta Kamizono*
* Allow `matches_regex` and `does_not_match_regexp` on the MySQL Arel visitor.
*James Pearson*

46
activerecord/lib/active_record/relation/query_methods.rb
View file

@ -1095,26 +1095,44 @@ module ActiveRecord
end
def build_left_outer_joins(manager, outer_joins, aliases)
buckets = { association_join: valid_association_list(outer_joins) }
buckets = Hash.new { |h, k| h[k] = [] }
buckets[:association_join] = valid_association_list(outer_joins)
build_join_query(manager, buckets, Arel::Nodes::OuterJoin, aliases)
end
def build_joins(manager, joins, aliases)
buckets = Hash.new { |h, k| h[k] = [] }
unless left_outer_joins_values.empty?
left_joins = valid_association_list(left_outer_joins_values.flatten)
joins.unshift construct_join_dependency(left_joins, Arel::Nodes::OuterJoin)
buckets[:stashed_join] << construct_join_dependency(left_joins, Arel::Nodes::OuterJoin)
end
buckets = joins.group_by do |join|
joins.map! do |join|
if join.is_a?(String)
table.create_string_join(Arel.sql(join.strip)) unless join.blank?
else
join
end
end.compact_blank!
while joins.first.is_a?(Arel::Nodes::Join)
join_node = joins.shift
if join_node.is_a?(Arel::Nodes::StringJoin) && !buckets[:stashed_join].empty?
buckets[:join_node] << join_node
else
buckets[:leading_join] << join_node
end
end
joins.each do |join|
case join
when String
:string_join
when Hash, Symbol, Array
:association_join
buckets[:association_join] << join
when ActiveRecord::Associations::JoinDependency
:stashed_join
buckets[:stashed_join] << join
when Arel::Nodes::Join
:join_node
buckets[:join_node] << join
else
raise "unknown class: %s" % join.class.name
end
@ -1124,25 +1142,21 @@ module ActiveRecord
end
def build_join_query(manager, buckets, join_type, aliases)
buckets.default = []
association_joins = buckets[:association_join]
stashed_joins = buckets[:stashed_join]
leading_joins = buckets[:leading_join].tap(&:uniq!)
join_nodes = buckets[:join_node].tap(&:uniq!)
string_joins = buckets[:string_join].compact_blank!.map!(&:strip).tap(&:uniq!)
string_joins.map! { |join| table.create_string_join(Arel.sql(join)) }
join_sources = manager.join_sources
join_sources.concat(join_nodes) unless join_nodes.empty?
join_sources.concat(leading_joins) unless leading_joins.empty?
unless association_joins.empty? && stashed_joins.empty?
alias_tracker = alias_tracker(join_nodes + string_joins, aliases)
alias_tracker = alias_tracker(leading_joins + join_nodes, aliases)
join_dependency = construct_join_dependency(association_joins, join_type)
join_sources.concat(join_dependency.join_constraints(stashed_joins, alias_tracker))
end
join_sources.concat(string_joins) unless string_joins.empty?
join_sources.concat(join_nodes) unless join_nodes.empty?
end
def build_select(arel)

37
activerecord/test/cases/associations/inner_join_association_test.rb
View file

@ -13,7 +13,7 @@ require "models/tag"
class InnerJoinAssociationTest < ActiveRecord::TestCase
fixtures :authors, :author_addresses, :essays, :posts, :comments, :categories, :categories_posts, :categorizations,
:taggings, :tags
:taggings, :tags, :people
def test_construct_finder_sql_applies_aliases_tables_on_association_conditions
result = Author.joins(:thinking_posts, :welcome_posts).to_a
@ -36,16 +36,37 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase
end
def test_construct_finder_sql_does_not_table_name_collide_with_string_joins
sql = Person.joins(:agents).joins("JOIN people agents_people ON agents_people.primary_contact_id = people.id").to_sql
assert_match(/agents_people_2/i, sql)
string_join = <<~SQL
JOIN people agents_people ON agents_people.primary_contact_id = agents_people_2.id AND agents_people.id > agents_people_2.id
SQL
expected = people(:susan)
assert_sql(/agents_people_2/i) do
assert_equal [expected], Person.joins(:agents).joins(string_join)
end
end
def test_construct_finder_sql_does_not_table_name_collide_with_aliased_joins
people = Person.arel_table
agents = people.alias("agents_people")
constraint = agents[:primary_contact_id].eq(people[:id])
sql = Person.joins(:agents).joins(agents.create_join(agents, agents.create_on(constraint))).to_sql
assert_match(/agents_people_2/i, sql)
agents = Person.arel_table.alias("agents_people")
agents_2 = Person.arel_table.alias("agents_people_2")
constraint = agents[:primary_contact_id].eq(agents_2[:id]).and(agents[:id].gt(agents_2[:id]))
expected = people(:susan)
assert_sql(/agents_people_2/i) do
assert_equal [expected], Person.joins(:agents).joins(agents.create_join(agents, agents.create_on(constraint)))
end
end
def test_user_supplied_joins_order_should_be_preserved
string_join = <<~SQL
JOIN people agents_people_2 ON agents_people_2.primary_contact_id = people.id
SQL
agents = Person.arel_table.alias("agents_people")
agents_2 = Person.arel_table.alias("agents_people_2")
constraint = agents[:primary_contact_id].eq(agents_2[:id]).and(agents[:id].gt(agents_2[:id]))
expected = people(:susan)
assert_equal [expected], Person.joins(string_join).joins(agents.create_join(agents, agents.create_on(constraint)))
end
def test_construct_finder_sql_ignores_empty_joins_hash