Merge pull request #30980 from sobrinho/sobrinho/arel-star-ignored-columns

Do not use `Arel.star` when `ignored_columns`

activerecord/lib/active_record/relation/query_methods.rb

@@ -1038,6 +1038,8 @@ module ActiveRecord
       def build_select(arel)
         if select_values.any?
           arel.project(*arel_columns(select_values.uniq))
+        elsif @klass.ignored_columns.any?
+          arel.project(*arel_columns(@klass.column_names))
         else
           arel.project(table[Arel.star])
         end

activerecord/test/cases/adapters/mysql2/explain_test.rb

@@ -1,23 +1,23 @@
 # frozen_string_literal: true
 
 require "cases/helper"
-require "models/developer"
-require "models/computer"
+require "models/author"
+require "models/post"
 
 class Mysql2ExplainTest < ActiveRecord::Mysql2TestCase
-  fixtures :developers
+  fixtures :authors
 
   def test_explain_for_one_query
-    explain = Developer.where(id: 1).explain
-    assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain
-    assert_match %r(developers |.* const), explain
+    explain = Author.where(id: 1).explain
+    assert_match %(EXPLAIN for: SELECT `authors`.* FROM `authors` WHERE `authors`.`id` = 1), explain
+    assert_match %r(authors |.* const), explain
   end
 
   def test_explain_with_eager_loading
-    explain = Developer.where(id: 1).includes(:audit_logs).explain
-    assert_match %(EXPLAIN for: SELECT `developers`.* FROM `developers` WHERE `developers`.`id` = 1), explain
-    assert_match %r(developers |.* const), explain
-    assert_match %(EXPLAIN for: SELECT `audit_logs`.* FROM `audit_logs` WHERE `audit_logs`.`developer_id` = 1), explain
-    assert_match %r(audit_logs |.* ALL), explain
+    explain = Author.where(id: 1).includes(:posts).explain
+    assert_match %(EXPLAIN for: SELECT `authors`.* FROM `authors` WHERE `authors`.`id` = 1), explain
+    assert_match %r(authors |.* const), explain
+    assert_match %(EXPLAIN for: SELECT `posts`.* FROM `posts` WHERE `posts`.`author_id` = 1), explain
+    assert_match %r(posts |.* ALL), explain
   end
 end

activerecord/test/cases/adapters/postgresql/explain_test.rb

@@ -1,22 +1,22 @@
 # frozen_string_literal: true
 
 require "cases/helper"
-require "models/developer"
-require "models/computer"
+require "models/author"
+require "models/post"
 
 class PostgreSQLExplainTest < ActiveRecord::PostgreSQLTestCase
-  fixtures :developers
+  fixtures :authors
 
   def test_explain_for_one_query
-    explain = Developer.where(id: 1).explain
-    assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain
+    explain = Author.where(id: 1).explain
+    assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain
     assert_match %(QUERY PLAN), explain
   end
 
   def test_explain_with_eager_loading
-    explain = Developer.where(id: 1).includes(:audit_logs).explain
+    explain = Author.where(id: 1).includes(:posts).explain
     assert_match %(QUERY PLAN), explain
-    assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain
-    assert_match %r(EXPLAIN for: SELECT "audit_logs"\.\* FROM "audit_logs" WHERE "audit_logs"\."developer_id" = (?:\$1 \[\["developer_id", 1\]\]|1)), explain
+    assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\$1 \[\["id", 1\]\]|1)), explain
+    assert_match %r(EXPLAIN for: SELECT "posts"\.\* FROM "posts" WHERE "posts"\."author_id" = (?:\$1 \[\["author_id", 1\]\]|1)), explain
   end
 end

activerecord/test/cases/adapters/sqlite3/explain_test.rb

@@ -1,23 +1,23 @@
 # frozen_string_literal: true
 
 require "cases/helper"
-require "models/developer"
-require "models/computer"
+require "models/author"
+require "models/post"
 
 class SQLite3ExplainTest < ActiveRecord::SQLite3TestCase
-  fixtures :developers
+  fixtures :authors
 
   def test_explain_for_one_query
-    explain = Developer.where(id: 1).explain
-    assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\? \[\["id", 1\]\]|1)), explain
-    assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain)
+    explain = Author.where(id: 1).explain
+    assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\? \[\["id", 1\]\]|1)), explain
+    assert_match(/(SEARCH )?TABLE authors USING (INTEGER )?PRIMARY KEY/, explain)
   end
 
   def test_explain_with_eager_loading
-    explain = Developer.where(id: 1).includes(:audit_logs).explain
-    assert_match %r(EXPLAIN for: SELECT "developers"\.\* FROM "developers" WHERE "developers"\."id" = (?:\? \[\["id", 1\]\]|1)), explain
-    assert_match(/(SEARCH )?TABLE developers USING (INTEGER )?PRIMARY KEY/, explain)
-    assert_match %r(EXPLAIN for: SELECT "audit_logs"\.\* FROM "audit_logs" WHERE "audit_logs"\."developer_id" = (?:\? \[\["developer_id", 1\]\]|1)), explain
-    assert_match(/(SCAN )?TABLE audit_logs/, explain)
+    explain = Author.where(id: 1).includes(:posts).explain
+    assert_match %r(EXPLAIN for: SELECT "authors"\.\* FROM "authors" WHERE "authors"\."id" = (?:\? \[\["id", 1\]\]|1)), explain
+    assert_match(/(SEARCH )?TABLE authors USING (INTEGER )?PRIMARY KEY/, explain)
+    assert_match %r(EXPLAIN for: SELECT "posts"\.\* FROM "posts" WHERE "posts"\."author_id" = (?:\? \[\["author_id", 1\]\]|1)), explain
+    assert_match(/(SCAN )?TABLE posts/, explain)
   end
 end

activerecord/test/cases/base_test.rb

@@ -1476,4 +1476,25 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal(%w(first_name last_name), Developer.ignored_columns)
     assert_equal(%w(first_name last_name), SymbolIgnoredDeveloper.ignored_columns)
   end
+
+  test "when #reload called, ignored columns' attribute methods are not defined" do
+    developer = Developer.create!(name: "Developer")
+    refute developer.respond_to?(:first_name)
+    refute developer.respond_to?(:first_name=)
+
+    developer.reload
+
+    refute developer.respond_to?(:first_name)
+    refute developer.respond_to?(:first_name=)
+  end
+
+  test "ignored columns not included in SELECT" do
+    query = Developer.all.to_sql
+
+    # ignored column
+    refute query.include?("first_name")
+
+    # regular column
+    assert query.include?("name")
+  end
 end

activerecord/test/cases/scoping/default_scoping_test.rb

@@ -120,49 +120,49 @@ class DefaultScopingTest < ActiveRecord::TestCase
   def test_unscope_with_where_attributes
     expected = Developer.order("salary DESC").collect(&:name)
     received = DeveloperOrderedBySalary.where(name: "David").unscope(where: :name).collect(&:name)
-    assert_equal expected, received
+    assert_equal expected.sort, received.sort
 
     expected_2 = Developer.order("salary DESC").collect(&:name)
     received_2 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope({ where: :name }, :select).collect(&:name)
-    assert_equal expected_2, received_2
+    assert_equal expected_2.sort, received_2.sort
 
     expected_3 = Developer.order("salary DESC").collect(&:name)
     received_3 = DeveloperOrderedBySalary.select("id").where("name" => "Jamis").unscope(:select, :where).collect(&:name)
-    assert_equal expected_3, received_3
+    assert_equal expected_3.sort, received_3.sort
 
     expected_4 = Developer.order("salary DESC").collect(&:name)
     received_4 = DeveloperOrderedBySalary.where.not("name" => "Jamis").unscope(where: :name).collect(&:name)
-    assert_equal expected_4, received_4
+    assert_equal expected_4.sort, received_4.sort
 
     expected_5 = Developer.order("salary DESC").collect(&:name)
     received_5 = DeveloperOrderedBySalary.where.not("name" => ["Jamis", "David"]).unscope(where: :name).collect(&:name)
-    assert_equal expected_5, received_5
+    assert_equal expected_5.sort, received_5.sort
 
     expected_6 = Developer.order("salary DESC").collect(&:name)
     received_6 = DeveloperOrderedBySalary.where(Developer.arel_table["name"].eq("David")).unscope(where: :name).collect(&:name)
-    assert_equal expected_6, received_6
+    assert_equal expected_6.sort, received_6.sort
 
     expected_7 = Developer.order("salary DESC").collect(&:name)
     received_7 = DeveloperOrderedBySalary.where(Developer.arel_table[:name].eq("David")).unscope(where: :name).collect(&:name)
-    assert_equal expected_7, received_7
+    assert_equal expected_7.sort, received_7.sort
   end
 
   def test_unscope_comparison_where_clauses
     # unscoped for WHERE (`developers`.`id` <= 2)
     expected = Developer.order("salary DESC").collect(&:name)
     received = DeveloperOrderedBySalary.where(id: -Float::INFINITY..2).unscope(where: :id).collect { |dev| dev.name }
-    assert_equal expected, received
+    assert_equal expected.sort, received.sort
 
     # unscoped for WHERE (`developers`.`id` < 2)
     expected = Developer.order("salary DESC").collect(&:name)
     received = DeveloperOrderedBySalary.where(id: -Float::INFINITY...2).unscope(where: :id).collect { |dev| dev.name }
-    assert_equal expected, received
+    assert_equal expected.sort, received.sort
   end
 
   def test_unscope_multiple_where_clauses
     expected = Developer.order("salary DESC").collect(&:name)
     received = DeveloperOrderedBySalary.where(name: "Jamis").where(id: 1).unscope(where: [:name, :id]).collect(&:name)
-    assert_equal expected, received
+    assert_equal expected.sort, received.sort
   end
 
   def test_unscope_string_where_clauses_involved
@@ -172,23 +172,23 @@ class DefaultScopingTest < ActiveRecord::TestCase
     dev_ordered_relation = DeveloperOrderedBySalary.where(name: "Jamis").where("created_at > ?", 1.year.ago)
     received = dev_ordered_relation.unscope(where: [:name]).collect(&:name)
 
-    assert_equal expected, received
+    assert_equal expected.sort, received.sort
   end
 
   def test_unscope_with_grouping_attributes
     expected = Developer.order("salary DESC").collect(&:name)
     received = DeveloperOrderedBySalary.group(:name).unscope(:group).collect(&:name)
-    assert_equal expected, received
+    assert_equal expected.sort, received.sort
 
     expected_2 = Developer.order("salary DESC").collect(&:name)
     received_2 = DeveloperOrderedBySalary.group("name").unscope(:group).collect(&:name)
-    assert_equal expected_2, received_2
+    assert_equal expected_2.sort, received_2.sort
   end
 
   def test_unscope_with_limit_in_query
     expected = Developer.order("salary DESC").collect(&:name)
     received = DeveloperOrderedBySalary.limit(1).unscope(:limit).collect(&:name)
-    assert_equal expected, received
+    assert_equal expected.sort, received.sort
   end
 
   def test_order_to_unscope_reordering
@@ -472,7 +472,7 @@ class DefaultScopingTest < ActiveRecord::TestCase
   test "a scope can remove the condition from the default scope" do
     scope = DeveloperCalledJamis.david2
     assert_equal 1, scope.where_clause.ast.children.length
-    assert_equal Developer.where(name: "David"), scope
+    assert_equal Developer.where(name: "David").map(&:id), scope.map(&:id)
   end
 
   def test_with_abstract_class_where_clause_should_not_be_duplicated