From d34c1fc3d64e51f9aeecfbffefd843e744dfbeac Mon Sep 17 00:00:00 2001
From: DmitryTsepelev <dmitry.a.tsepelev@gmail.com>
Date: Mon, 26 Nov 2018 11:12:42 +0300
Subject: [PATCH] Cached columns_hash fields should be excluded from
 ResultSet#column_types

---
 activerecord/CHANGELOG.md                     | 29 +++++++++++++++++++
 activerecord/lib/active_record/querying.rb    |  3 +-
 activerecord/test/cases/base_test.rb          |  8 +++++
 .../test/cases/instrumentation_test.rb        |  4 +++
 activerecord/test/models/developer.rb         | 14 +++++++++
 5 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 13cc486e7f..9d7bfbcef1 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,32 @@
+*   Cached columns_hash fields should be excluded from ResultSet#column_types
+
+    PR #34528 addresses the inconsistent behaviour when attribute is defined for an ignored column. The following test
+    was passing for SQLite and MySQL, but failed for PostgreSQL:
+
+    ```ruby
+    class DeveloperName < ActiveRecord::Type::String
+      def deserialize(value)
+        "Developer: #{value}"
+      end
+    end
+
+    class AttributedDeveloper < ActiveRecord::Base
+      self.table_name = "developers"
+
+      attribute :name, DeveloperName.new
+
+      self.ignored_columns += ["name"]
+    end
+
+    developer = AttributedDeveloper.create
+    developer.update_column :name, "name"
+
+    loaded_developer = AttributedDeveloper.where(id: developer.id).select("*").first
+    puts loaded_developer.name # should be "Developer: name" but it's just "name"
+    ```
+
+    *Dmitry Tsepelev*
+
 *   Make the implicit order column configurable.
 
     When calling ordered finder methods such as +first+ or +last+ without an
diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb
index c84f3d0fbb..0c55284f4c 100644
--- a/activerecord/lib/active_record/querying.rb
+++ b/activerecord/lib/active_record/querying.rb
@@ -40,7 +40,8 @@ module ActiveRecord
     def find_by_sql(sql, binds = [], preparable: nil, &block)
       result_set = connection.select_all(sanitize_sql(sql), "#{name} Load", binds, preparable: preparable)
       column_types = result_set.column_types.dup
-      columns_hash.each_key { |k| column_types.delete k }
+      cached_columns_hash = connection.schema_cache.columns_hash(table_name)
+      cached_columns_hash.each_key { |k| column_types.delete k }
       message_bus = ActiveSupport::Notifications.instrumenter
 
       payload = {
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 09e5517449..63a73101c4 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -1447,6 +1447,14 @@ class BasicsTest < ActiveRecord::TestCase
     assert_not_respond_to developer, :first_name=
   end
 
+  test "when ignored attribute is loaded, cast type should be preferred over DB type" do
+    developer = AttributedDeveloper.create
+    developer.update_column :name, "name"
+
+    loaded_developer = AttributedDeveloper.where(id: developer.id).select("*").first
+    assert_equal "Developer: name", loaded_developer.name
+  end
+
   test "ignored columns not included in SELECT" do
     query = Developer.all.to_sql.downcase
 
diff --git a/activerecord/test/cases/instrumentation_test.rb b/activerecord/test/cases/instrumentation_test.rb
index e6e8468757..c09ea32991 100644
--- a/activerecord/test/cases/instrumentation_test.rb
+++ b/activerecord/test/cases/instrumentation_test.rb
@@ -5,6 +5,10 @@ require "models/book"
 
 module ActiveRecord
   class InstrumentationTest < ActiveRecord::TestCase
+    def setup
+      ActiveRecord::Base.connection.schema_cache.add(Book.table_name)
+    end
+
     def test_payload_name_on_load
       Book.create(name: "test book")
       subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args|
diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb
index 8881c69368..ec48094207 100644
--- a/activerecord/test/models/developer.rb
+++ b/activerecord/test/models/developer.rb
@@ -279,3 +279,17 @@ class DeveloperWithIncorrectlyOrderedHasManyThrough < ActiveRecord::Base
   has_many :companies, through: :contracts
   has_many :contracts, foreign_key: :developer_id
 end
+
+class DeveloperName < ActiveRecord::Type::String
+  def deserialize(value)
+    "Developer: #{value}"
+  end
+end
+
+class AttributedDeveloper < ActiveRecord::Base
+  self.table_name = "developers"
+
+  attribute :name, DeveloperName.new
+
+  self.ignored_columns += ["name"]
+end