From 05dd3df35db8b2d39ed177f4ccfe112bdfe7726d Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 23 May 2014 10:59:30 -0700 Subject: Remove `Column#primary` It appears to have been used at some point in the past, but is no longer used in any meaningful way. Whether a column is considered primary is a property of the model, not the schema/column. This also removes the need for yet another layer of caching of the model's schema, and we can leave that to the schema cache. --- .../lib/active_record/attribute_methods.rb | 2 +- .../active_record/connection_adapters/column.rb | 3 +- activerecord/lib/active_record/model_schema.rb | 12 ++------ .../test/cases/adapters/sqlite3/copy_table_test.rb | 1 - activerecord/test/cases/base_test.rb | 4 +-- activerecord/test/cases/bind_parameter_test.rb | 8 ++---- activerecord/test/cases/primary_keys_test.rb | 32 ---------------------- 7 files changed, 10 insertions(+), 52 deletions(-) diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 8bd51dc71f..d44aebec39 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -455,7 +455,7 @@ module ActiveRecord end def pk_attribute?(name) - column_for_attribute(name).primary + name == self.class.primary_key end def typecasted_attribute_value(name) diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 704868c058..42aabd6d7f 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -14,7 +14,7 @@ module ActiveRecord end attr_reader :name, :default, :cast_type, :null, :sql_type, :default_function - attr_accessor :primary, :coder + attr_accessor :coder alias :encoded? :coder @@ -36,7 +36,6 @@ module ActiveRecord @null = null @default = extract_default(default) @default_function = nil - @primary = nil @coder = nil end diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index aa1166750f..8449fb1266 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -219,16 +219,12 @@ module ActiveRecord # Returns an array of column objects for the table associated with this class. def columns - @columns ||= connection.schema_cache.columns(table_name).map do |col| - col = col.dup - col.primary = (col.name == primary_key) - col - end + connection.schema_cache.columns(table_name) end # Returns a hash of column objects for the table associated with this class. def columns_hash - @columns_hash ||= Hash[columns.map { |c| [c.name, c] }] + connection.schema_cache.columns_hash(table_name) end def column_types # :nodoc: @@ -271,7 +267,7 @@ module ActiveRecord # Returns an array of column objects where the primary id, all columns ending in "_id" or "_count", # and columns used for single table inheritance have been removed. def content_columns - @content_columns ||= columns.reject { |c| c.primary || c.name =~ /(_id|_count)$/ || c.name == inheritance_column } + @content_columns ||= columns.reject { |c| c.name == primary_key || c.name =~ /(_id|_count)$/ || c.name == inheritance_column } end # Resets all the cached information about columns, which will cause them @@ -308,8 +304,6 @@ module ActiveRecord @arel_engine = nil @column_defaults = nil @column_names = nil - @columns = nil - @columns_hash = nil @column_types = nil @content_columns = nil @dynamic_methods_hash = nil diff --git a/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb b/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb index b478db749d..13b754d226 100644 --- a/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb @@ -60,7 +60,6 @@ class CopyTableTest < ActiveRecord::TestCase assert_equal original_id.type, copied_id.type assert_equal original_id.sql_type, copied_id.sql_type assert_equal original_id.limit, copied_id.limit - assert_equal original_id.primary, copied_id.primary end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 7c7c1fbfbd..c565daba65 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -102,8 +102,8 @@ class BasicsTest < ActiveRecord::TestCase end def test_columns_should_obey_set_primary_key - pk = Subscriber.columns.find { |x| x.name == 'nick' } - assert pk.primary, 'nick should be primary key' + pk = Subscriber.columns_hash[Subscriber.primary_key] + assert_equal 'nick', pk.name, 'nick should be primary key' end def test_primary_key_with_no_id diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 40f73cd68c..0bc7ee6d64 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -21,7 +21,7 @@ module ActiveRecord super @connection = ActiveRecord::Base.connection @subscriber = LogListener.new - @pk = Topic.columns.find { |c| c.primary } + @pk = Topic.columns_hash[Topic.primary_key] @subscription = ActiveSupport::Notifications.subscribe('sql.active_record', @subscriber) end @@ -60,12 +60,10 @@ module ActiveRecord end def test_logs_bind_vars - pk = Topic.columns.find { |x| x.primary } - payload = { :name => 'SQL', :sql => 'select * from topics where id = ?', - :binds => [[pk, 10]] + :binds => [[@pk, 10]] } event = ActiveSupport::Notifications::Event.new( 'foo', @@ -87,7 +85,7 @@ module ActiveRecord }.new logger.sql event - assert_match([[pk.name, 10]].inspect, logger.debugs.first) + assert_match([[@pk.name, 10]].inspect, logger.debugs.first) end end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 56d0dd6a77..c719918fd7 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -149,38 +149,6 @@ class PrimaryKeysTest < ActiveRecord::TestCase assert_equal k.connection.quote_column_name("foo"), k.quoted_primary_key end - def test_two_models_with_same_table_but_different_primary_key - k1 = Class.new(ActiveRecord::Base) - k1.table_name = 'posts' - k1.primary_key = 'id' - - k2 = Class.new(ActiveRecord::Base) - k2.table_name = 'posts' - k2.primary_key = 'title' - - assert k1.columns.find { |c| c.name == 'id' }.primary - assert !k1.columns.find { |c| c.name == 'title' }.primary - assert k1.columns_hash['id'].primary - assert !k1.columns_hash['title'].primary - - assert !k2.columns.find { |c| c.name == 'id' }.primary - assert k2.columns.find { |c| c.name == 'title' }.primary - assert !k2.columns_hash['id'].primary - assert k2.columns_hash['title'].primary - end - - def test_models_with_same_table_have_different_columns - k1 = Class.new(ActiveRecord::Base) - k1.table_name = 'posts' - - k2 = Class.new(ActiveRecord::Base) - k2.table_name = 'posts' - - k1.columns.zip(k2.columns).each do |col1, col2| - assert !col1.equal?(col2) - end - end - def test_auto_detect_primary_key_from_schema MixedCaseMonkey.reset_primary_key assert_equal "monkeyID", MixedCaseMonkey.primary_key -- cgit v1.2.3