From 947a5de75d65dda1ab9f5ec4b3ef53007d34e90f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 4 Dec 2020 14:35:56 +0900 Subject: [PATCH] Don't use explicit references for join aliases As the API doc shows, `references` should be given table names. https://api.rubyonrails.org/classes/ActiveRecord/QueryMethods.html#method-i-references However, people (and test cases in the our codebase) sometimes give association names for `references`. Since the API example and all test cases in the codebase use symbols as arguments, so I regarded symbols as user given values in #40106. But as #40743 indicates, people also use string association names, it would have unexpected side-effect especially if singular association names given. As much as possible to avoid to use user given values, mark internal references for `where` and use only them for join aliases. Fixes #40743. --- .../lib/active_record/associations/join_dependency.rb | 2 +- activerecord/lib/active_record/relation/predicate_builder.rb | 4 ++-- activerecord/test/cases/associations/eager_test.rb | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 4ee566586e..193b9e708a 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -84,7 +84,7 @@ module ActiveRecord @references = {} references.each do |table_name| - @references[table_name.to_sym] = table_name if table_name.is_a?(String) + @references[table_name.to_sym] = table_name if table_name.is_a?(Arel::Nodes::SqlLiteral) end unless references.empty? joins = make_join_constraints(join_root, join_type) diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 2039418fb1..9a14c2a627 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -32,9 +32,9 @@ module ActiveRecord def self.references(attributes) attributes.each_with_object([]) do |(key, value), result| if value.is_a?(Hash) - result << key + result << Arel.sql(key) elsif key.include?(".") - result << key.split(".").first + result << Arel.sql(key.split(".").first) end end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 4265fb3089..b3f9860a79 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -663,10 +663,10 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_has_many_and_limit_and_conditions_array_on_the_eagers - posts = Post.includes(:author, :comments).limit(2).references(:author).where("authors.name = ?", "David") + posts = Post.includes(:author, :comments).limit(2).references("author").where("authors.name = ?", "David") assert_equal 2, posts.size - count = Post.includes(:author, :comments).limit(2).references(:author).where("authors.name = ?", "David").count + count = Post.includes(:author, :comments).limit(2).references("author").where("authors.name = ?", "David").count assert_equal posts.size, count end