Skip to content

Commit

Permalink
Cached columns_hash fields should be excluded from ResultSet#column_t…
Browse files Browse the repository at this point in the history
…ypes
  • Loading branch information
DmitryTsepelev committed Nov 27, 2018
1 parent b9c7305 commit d34c1fc
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 1 deletion.
29 changes: 29 additions & 0 deletions 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
Expand Down
3 changes: 2 additions & 1 deletion activerecord/lib/active_record/querying.rb
Expand Up @@ -40,7 +40,8 @@ module Querying
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 = {
Expand Down
8 changes: 8 additions & 0 deletions activerecord/test/cases/base_test.rb
Expand Up @@ -1447,6 +1447,14 @@ def test_default_values_are_deeply_dupped
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

Expand Down
4 changes: 4 additions & 0 deletions activerecord/test/cases/instrumentation_test.rb
Expand Up @@ -5,6 +5,10 @@

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|
Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/models/developer.rb
Expand Up @@ -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

0 comments on commit d34c1fc

Please sign in to comment.