Fix pluck and select with custom attributes

Currently custom attributes are always qualified by the table name in
the generated SQL wrongly even if the table doesn't have the named
column, it would cause an invalid SQL error.

Custom attributes should only be qualified if the table has the same
named column.

activerecord/lib/active_record/relation/calculations.rb

@@ -186,11 +186,9 @@ def pluck(*column_names)
         relation = apply_join_dependency
         relation.pluck(*column_names)
       else
-        disallow_raw_sql!(column_names)
+        klass.disallow_raw_sql!(column_names)
         relation = spawn
-        relation.select_values = column_names.map { |cn|
-          @klass.has_attribute?(cn) || @klass.attribute_alias?(cn) ? arel_attribute(cn) : cn
-        }
+        relation.select_values = column_names
         result = skip_query_cache_if_necessary { klass.connection.select_all(relation.arel, nil) }
         result.cast_values(klass.attribute_types)
       end

activerecord/lib/active_record/relation/query_methods.rb

@@ -1052,18 +1052,30 @@ def build_select(arel)
 
       def arel_columns(columns)
         columns.flat_map do |field|
-          if (Symbol === field || String === field) && (klass.has_attribute?(field) || klass.attribute_alias?(field)) && !from_clause.value
-            arel_attribute(field)
-          elsif Symbol === field
-            connection.quote_table_name(field.to_s)
-          elsif Proc === field
+          case field
+          when Symbol
+            field = field.to_s
+            arel_column(field) { connection.quote_table_name(field) }
+          when String
+            arel_column(field) { field }
+          when Proc
             field.call
           else
             field
           end
         end
       end
 
+      def arel_column(field)
+        field = klass.attribute_alias(field) if klass.attribute_alias?(field)
+
+        if klass.columns_hash.key?(field) && !from_clause.value
+          arel_attribute(field)
+        else
+          yield
+        end
+      end
+
       def reverse_sql_order(order_query)
         if order_query.empty?
           return [arel_attribute(primary_key).desc] if primary_key

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -448,8 +448,13 @@ def test_with_polymorphic_and_condition
   end
 
   def test_with_select
-    assert_equal 1, Company.find(2).firm_with_select.attributes.size
-    assert_equal 1, Company.all.merge!(includes: :firm_with_select).find(2).firm_with_select.attributes.size
+    assert_equal 1, Post.find(2).author_with_select.attributes.size
+    assert_equal 1, Post.includes(:author_with_select).find(2).author_with_select.attributes.size
+  end
+
+  def test_custom_attribute_with_select
+    assert_equal 2, Company.find(2).firm_with_select.attributes.size
+    assert_equal 2, Company.includes(:firm_with_select).find(2).firm_with_select.attributes.size
   end
 
   def test_belongs_to_without_counter_cache_option

activerecord/test/cases/base_test.rb

@@ -1226,14 +1226,15 @@ def test_marshalling_new_record_round_trip_with_associations
   end
 
   def test_attribute_names
-    assert_equal ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description"],
-                 Company.attribute_names
+    expected = ["id", "type", "firm_id", "firm_name", "name", "client_of", "rating", "account_id", "description", "metadata"]
+    assert_equal expected, Company.attribute_names
   end
 
   def test_has_attribute
     assert Company.has_attribute?("id")
     assert Company.has_attribute?("type")
     assert Company.has_attribute?("name")
+    assert Company.has_attribute?("metadata")
     assert_not Company.has_attribute?("lastname")
     assert_not Company.has_attribute?("age")
   end

activerecord/test/cases/calculations_test.rb

@@ -690,8 +690,9 @@ def test_pluck_if_table_included
   end
 
   def test_pluck_not_auto_table_name_prefix_if_column_joined
-    Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
-    assert_equal [7], Company.joins(:contracts).pluck(:developer_id)
+    company = Company.create!(name: "test", contracts: [Contract.new(developer_id: 7)])
+    metadata = company.contracts.first.metadata
+    assert_equal [metadata], Company.joins(:contracts).pluck(:metadata)
   end
 
   def test_pluck_with_selection_clause

activerecord/test/cases/relations_test.rb

@@ -14,6 +14,7 @@
 require "models/computer"
 require "models/reply"
 require "models/company"
+require "models/contract"
 require "models/bird"
 require "models/car"
 require "models/engine"
@@ -1815,6 +1816,12 @@ def test_presence
     assert_equal [1, 1, 1], posts.map(&:author_address_id)
   end
 
+  test "joins with select custom attribute" do
+    contract = Company.create!(name: "test").contracts.create!
+    company = Company.joins(:contracts).select(:id, :metadata).find(contract.company_id)
+    assert_equal contract.metadata, company.metadata
+  end
+
   test "delegations do not leak to other classes" do
     Topic.all.by_lifo
     assert Topic.all.class.method_defined?(:by_lifo)

activerecord/test/models/company.rb

@@ -13,6 +13,8 @@ class Company < AbstractCompany
   has_many :contracts
   has_many :developers, through: :contracts
 
+  attribute :metadata, :json
+
   scope :of_first_firm, lambda {
     joins(account: :firm).
     where("firms.id" => 1)

activerecord/test/models/contract.rb

@@ -5,7 +5,9 @@ class Contract < ActiveRecord::Base
   belongs_to :developer, primary_key: :id
   belongs_to :firm, foreign_key: "company_id"
 
-  before_save :hi
+  attribute :metadata, :json
+
+  before_save :hi, :update_metadata
   after_save :bye
 
   attr_accessor :hi_count, :bye_count
@@ -19,6 +21,10 @@ def bye
     @bye_count ||= 0
     @bye_count += 1
   end
+
+  def update_metadata
+    self.metadata = { company_id: company_id, developer_id: developer_id }
+  end
 end
 
 class NewContract < Contract

activerecord/test/models/post.rb

@@ -31,6 +31,7 @@ def greeting
 
   belongs_to :author_with_posts, -> { includes(:posts) }, class_name: "Author", foreign_key: :author_id
   belongs_to :author_with_address, -> { includes(:author_address) }, class_name: "Author", foreign_key: :author_id
+  belongs_to :author_with_select, -> { select(:id) }, class_name: "Author", foreign_key: :author_id
 
   def first_comment
     super.body

activerecord/test/schema/schema.rb

@@ -247,6 +247,7 @@
   create_table :contracts, force: true do |t|
     t.references :developer, index: false
     t.references :company, index: false
+    t.string :metadata
   end
 
   create_table :customers, force: true do |t|