From cc405496ce85ee0073268baefdb2be5d4b062f91 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Tue, 18 Jun 2013 15:10:48 -0700 Subject: =?UTF-8?q?Fix=20that=20a=20collection=20proxy=20could=20be=20cach?= =?UTF-8?q?ed=20before=20the=20save=20of=20the=20owner,=20resulting=20in?= =?UTF-8?q?=20an=20invalid=20proxy=20lacking=20the=20owner=E2=80=99s=20id.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Absent this fix calls like: owner.association.update_all to behave unexpectedly because they try to act on association objects where owner_id is null. more evidence here: https://gist.github.com/Empact/5865555 ``` Active Record 3.2.13 -- create_table(:firms, {:force=>true}) -> 0.1371s -- create_table(:clients, {:force=>true}) -> 0.0005s 1 clients. 1 expected. 1 clients updated. 1 expected. ``` ``` Active Record 4.0.0 -- create_table(:firms, {:force=>true}) -> 0.1606s -- create_table(:clients, {:force=>true}) -> 0.0004s 1 clients. 1 expected. 0 clients updated. 1 expected. ``` --- .../lib/active_record/associations/collection_association.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 1836ff0910..c5dff7ee1a 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -33,7 +33,13 @@ module ActiveRecord reload end - @proxy ||= CollectionProxy.create(klass, self) + if owner.new_record? + # Cache the proxy separately before the owner has an id + # or else a post-save proxy will still lack the id + @new_record_proxy ||= CollectionProxy.create(klass, self) + else + @proxy ||= CollectionProxy.create(klass, self) + end end # Implements the writer method, e.g. foo.items= for Foo.has_many :items -- cgit v1.2.3 From 0f3cefa445c6b43711b29f38257adcedede74baa Mon Sep 17 00:00:00 2001 From: Byron Bischoff Date: Wed, 22 Oct 2014 11:39:47 -0700 Subject: =?UTF-8?q?copy=20reflection=5Fscopes=E2=80=99s=20unscoped=20value?= =?UTF-8?q?=20when=20building=20scope=20for=20preloading,=20fixes=20#11036?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- activerecord/lib/active_record/associations/preloader/association.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index c0639742be..2300435f91 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -136,6 +136,10 @@ module ActiveRecord preload_values = preload_scope.values preload_binds = preload_scope.bind_values + if values[:unscope] + scope.unscope! values[:unscope] + end + scope.where_values = Array(values[:where]) + Array(preload_values[:where]) scope.references_values = Array(values[:references]) + Array(preload_values[:references]) scope.bind_values = (reflection_binds + preload_binds) -- cgit v1.2.3 From 28eb085cdf989395957cbc50c7e1bb981e70ff9b Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Sat, 1 Nov 2014 16:45:31 +0530 Subject: - Remove unused duplicated method `add_column_position` from AbstractMysqlAdapter --- .../active_record/connection_adapters/abstract_mysql_adapter.rb | 8 -------- 1 file changed, 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 7f04e35042..5f512758e9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -585,14 +585,6 @@ module ActiveRecord end end - def add_column_position!(sql, options) - if options[:first] - sql << " FIRST" - elsif options[:after] - sql << " AFTER #{quote_column_name(options[:after])}" - end - end - # SHOW VARIABLES LIKE 'name' def show_variable(name) variables = select_all("SHOW VARIABLES LIKE '#{name}'", 'SCHEMA') -- cgit v1.2.3 From 9007b789e197ad0445a03b591f982a44b92f715d Mon Sep 17 00:00:00 2001 From: Ted O'Meara Date: Thu, 30 Oct 2014 14:01:38 -0400 Subject: Added SchemaDumper support for tables with jsonb columns. --- activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index e472741660..9941f74766 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -104,6 +104,7 @@ module ActiveRecord macaddr: { name: "macaddr" }, uuid: { name: "uuid" }, json: { name: "json" }, + jsonb: { name: "jsonb" }, ltree: { name: "ltree" }, citext: { name: "citext" }, point: { name: "point" }, -- cgit v1.2.3 From 4bcb0942241fe10d734b9777a61e0e0026cf0d5d Mon Sep 17 00:00:00 2001 From: Paul Annesley Date: Tue, 4 Nov 2014 13:58:09 -0800 Subject: activerecord: rake db:create shows underlying error message. --- activerecord/lib/active_record/tasks/mysql_database_tasks.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index d890196f47..eafbb2c249 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -31,6 +31,7 @@ module ActiveRecord end establish_connection configuration else + $stderr.puts error.inspect $stderr.puts "Couldn't create database for #{configuration.inspect}, #{creation_options.inspect}" $stderr.puts "(If you set the charset manually, make sure you have a matching collation)" if configuration['encoding'] end -- cgit v1.2.3 From 8fee923888192a658d8823b31e77ed0683dfd665 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 5 Nov 2014 10:59:50 -0700 Subject: Improve performance of AR object instantiation We introduced a performance hit by adding an additional iteration through a model's attributes on creation. We don't actually need the values from `Result` to be a hash, we can separate the columns and values and zip them up ourself during the iteration that we have to do. --- .../lib/active_record/attribute_set/builder.rb | 21 +++++++++++++--- activerecord/lib/active_record/inheritance.rb | 12 ++++++---- activerecord/lib/active_record/persistence.rb | 28 +++++++++++++++++++--- activerecord/lib/active_record/querying.rb | 2 +- activerecord/lib/active_record/result.rb | 9 +++++++ 5 files changed, 61 insertions(+), 11 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index d4a787f2fe..92640f1579 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -8,18 +8,33 @@ module ActiveRecord end def build_from_database(values = {}, additional_types = {}) - attributes = build_attributes_from_values(values, additional_types) + build_from_database_pairs(values.keys, values.values, additional_types) + end + + def build_from_database_pairs(columns = [], values = [], additional_types = {}) + attributes = build_attributes_from_values(columns, values, additional_types) add_uninitialized_attributes(attributes) AttributeSet.new(attributes) end private - def build_attributes_from_values(values, additional_types) - values.each_with_object({}) do |(name, value), hash| + def build_attributes_from_values(columns, values, additional_types) + # We are performing manual iteration here as this method is a performance + # hotspot + hash = {} + index = 0 + length = columns.length + + while index < length + name = columns[index] + value = values[index] type = additional_types.fetch(name, types[name]) hash[name] = Attribute.from_database(name, value, type) + index += 1 end + + hash end def add_uninitialized_attributes(attributes) diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index f58145ab05..4aad3217cb 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -165,15 +165,19 @@ module ActiveRecord # record instance. For single-table inheritance, we check the record # for a +type+ column and return the corresponding class. def discriminate_class_for_record(record) - if using_single_table_inheritance?(record) - find_sti_class(record[inheritance_column]) + discriminate_class_for_value(record[inheritance_column]) + end + + def discriminate_class_for_value(value) + if using_single_table_inheritance?(value) + find_sti_class(value) else super end end - def using_single_table_inheritance?(record) - record[inheritance_column].present? && columns_hash.include?(inheritance_column) + def using_single_table_inheritance?(value) + value.present? && columns_hash.include?(inheritance_column) end def find_sti_class(type_name) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 755ff2b2f1..9d2c9d3b9c 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -65,19 +65,41 @@ module ActiveRecord # how this "single-table" inheritance mapping is implemented. def instantiate(attributes, column_types = {}) klass = discriminate_class_for_record(attributes) - attributes = klass.attributes_builder.build_from_database(attributes, column_types) - klass.allocate.init_with('attributes' => attributes, 'new_record' => false) + klass.instantiate_pairs(attributes.keys, attributes.values, column_types) + end + + def instantiate_pairs(columns, values, column_types = {}) # :nodoc: + attributes = attributes_builder.build_from_database_pairs(columns, values, column_types) + allocate.init_with('attributes' => attributes, 'new_record' => false) + end + + def instantiate_result_set(result_set, column_types = {}) # :nodoc: + inheritance_column_index = inheritance_column && result_set.columns.find_index(inheritance_column) + + result_set.each_pair.map do |columns, values| + inheritance_value = inheritance_column_index && values[inheritance_column_index] + klass = discriminate_class_for_value(inheritance_value) + klass.instantiate_pairs(columns, values, column_types) + end end private # Called by +instantiate+ to decide which class to use for a new # record instance. # - # See +ActiveRecord::Inheritance#discriminate_class_for_record+ for + # See +ActiveRecord::Inheritance#discriminate_class_for_value+ for # the single-table inheritance discriminator. + def discriminate_class_for_value(*) + self + end + def discriminate_class_for_record(record) self end + + def inheritance_column + nil + end end # Returns true if this object hasn't been saved yet -- that is, a record diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index e8de4db3a7..9d4df81b07 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -47,7 +47,7 @@ module ActiveRecord } message_bus.instrument('instantiation.active_record', payload) do - result_set.map { |record| instantiate(record, column_types) } + instantiate_result_set(result_set, column_types) end end diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index 3a3e65ef32..c84ad586e2 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -54,6 +54,15 @@ module ActiveRecord end end + def each_pair + return to_enum(__method__) unless block_given? + + columns = @columns.map { |c| c.dup.freeze } + @rows.each do |row| + yield columns, row + end + end + def to_hash hash_rows end -- cgit v1.2.3 From 75099a933d0bf5e037037f78fbb305520955b064 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 5 Nov 2014 13:34:44 -0700 Subject: Remove unneccesary default parameters --- activerecord/lib/active_record/attribute_set/builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 92640f1579..0a62c68bfb 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -11,7 +11,7 @@ module ActiveRecord build_from_database_pairs(values.keys, values.values, additional_types) end - def build_from_database_pairs(columns = [], values = [], additional_types = {}) + def build_from_database_pairs(columns, values, additional_types) attributes = build_attributes_from_values(columns, values, additional_types) add_uninitialized_attributes(attributes) AttributeSet.new(attributes) -- cgit v1.2.3 From d424ded6fd4b923802e9dab62b463a6f3439417f Mon Sep 17 00:00:00 2001 From: Franky W Date: Thu, 6 Nov 2014 09:07:29 -0800 Subject: Print out a meaningful error when ActiveRecord::ReadOnlyRecord is raised Currently, there is no messages which get printed out. Convoluted system may have hooks that create other objects in which case we only fail with no messages. This commit changes this information allowing you to know which object is the one that actually raised the error. --- activerecord/lib/active_record/persistence.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 9d2c9d3b9c..06c8bceb30 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -188,7 +188,7 @@ module ActiveRecord # and destroy returns +false+. See # ActiveRecord::Callbacks for further details. def destroy - raise ReadOnlyRecord if readonly? + raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly? destroy_associations destroy_row if persisted? @destroyed = true @@ -519,7 +519,7 @@ module ActiveRecord end def create_or_update - raise ReadOnlyRecord if readonly? + raise ReadOnlyRecord, "#{self.class} is marked as readonly" if readonly? result = new_record? ? _create_record : _update_record result != false end -- cgit v1.2.3 From 58f3cf9bcb64ad2ee4997e89f4d7646f4c7bdbe7 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Fri, 7 Nov 2014 09:58:15 +0100 Subject: docs, the abstract data type `:timestamp` was removed. See #15184 [ci skip] --- .../active_record/connection_adapters/abstract/schema_definitions.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index f8b6daea5a..dd4c4ae9fc 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -124,8 +124,8 @@ module ActiveRecord # which is one of the following: # :primary_key, :string, :text, # :integer, :float, :decimal, - # :datetime, :timestamp, :time, - # :date, :binary, :boolean. + # :datetime, :time, :date, + # :binary, :boolean. # # You may use a type not in this list as long as it is supported by your # database (for example, "polygon" in MySQL), but this will not be database -- cgit v1.2.3 From c2fc9848b1495768a4c550e4cc532882146902c9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Fri, 7 Nov 2014 16:23:15 -0800 Subject: default scopes should break the cache on has_many. if you specify a default scope on a model, it will break caching. We cannot predict what will happen inside the scope, so play it safe for now. fixes #17495 --- activerecord/lib/active_record/associations/collection_association.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index bdfd569be2..4411e5ae62 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -409,7 +409,8 @@ module ActiveRecord def get_records if reflection.scope_chain.any?(&:any?) || scope.eager_loading? || - klass.current_scope + klass.current_scope || + klass.default_scopes.any? return scope.to_a end -- cgit v1.2.3 From 9bd43868502f5e8a67649ffb5e766853a05714c5 Mon Sep 17 00:00:00 2001 From: alfa-jpn Date: Sat, 8 Nov 2014 17:03:54 +0900 Subject: default scopes should break the cache on singulur_association. fixes #17495 --- activerecord/lib/active_record/associations/singular_association.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index c360ef1b2c..c44242a0f0 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -41,7 +41,8 @@ module ActiveRecord def get_records if reflection.scope_chain.any?(&:any?) || scope.eager_loading? || - klass.current_scope + klass.current_scope || + klass.default_scopes.any? return scope.limit(1).to_a end -- cgit v1.2.3 From f43f56e16e99dae083cf1400a48c236d1af265d4 Mon Sep 17 00:00:00 2001 From: Sammy Larbi Date: Wed, 8 Oct 2014 15:58:11 -0500 Subject: Ensure HABTM relationships produce valid class names (Fixes #17119) --- activerecord/lib/active_record/associations.rb | 2 +- .../lib/active_record/associations/builder/has_and_belongs_to_many.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 8911506694..d2bcdc55bf 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1700,7 +1700,7 @@ module ActiveRecord hm_options[:through] = middle_reflection.name hm_options[:source] = join_model.right_reflection.name - [:before_add, :after_add, :before_remove, :after_remove, :autosave, :validate, :join_table].each do |k| + [:before_add, :after_add, :before_remove, :after_remove, :autosave, :validate, :join_table, :class_name].each do |k| hm_options[k] = options[k] if options.key? k end diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 815e8eb97f..357b28ac94 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -98,7 +98,7 @@ module ActiveRecord::Associations::Builder def middle_options(join_model) middle_options = {} - middle_options[:class] = join_model + middle_options[:class_name] = "#{lhs_model.name}::#{join_model.name}" middle_options[:source] = join_model.left_reflection.name if options.key? :foreign_key middle_options[:foreign_key] = options[:foreign_key] -- cgit v1.2.3 From 52c3a16fa07cde643af3c2200e4b87bcb470eb12 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sun, 9 Nov 2014 20:38:40 -0700 Subject: Revert the behavior of booleans in string columns to that of 4.1 Why are people assigning booleans to string columns? >_> We unintentionally changed the behavior on Sqlite3 and PostgreSQL. Boolean values should cast to the database's representation of true and false. This is 't' and 'f' by default, and "1" and "0" on Mysql. The implementation to make the connection adapter specific behavior is hacky at best, and should be re-visted once we decide how we actually want to separate the concerns related to things that should change based on the database adapter. That said, this isn't something I'd expect to change based on my database adapter. We're storing a string, so the way the database represents a boolean should be irrelevant. It also seems strange for us to give booleans special behavior at all in string columns. Why is `to_s` not sufficient? It's inconsistent and confusing. Perhaps we should consider deprecating in the future. Fixes #17571 --- .../connection_adapters/abstract_mysql_adapter.rb | 24 +++++++++++++++++++++- activerecord/lib/active_record/type/string.rb | 8 ++++---- 2 files changed, 27 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 5f512758e9..26015528e5 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -648,6 +648,8 @@ module ActiveRecord def initialize_type_map(m) # :nodoc: super + register_class_with_limit m, %r(char)i, MysqlString + m.register_type %r(tinytext)i, Type::Text.new(limit: 2**8 - 1) m.register_type %r(tinyblob)i, Type::Binary.new(limit: 2**8 - 1) m.register_type %r(text)i, Type::Text.new(limit: 2**16 - 1) @@ -672,7 +674,7 @@ module ActiveRecord m.register_type(%r(enum)i) do |sql_type| limit = sql_type[/^enum\((.+)\)/i, 1] .split(',').map{|enum| enum.strip.length - 2}.max - Type::String.new(limit: limit) + MysqlString.new(limit: limit) end end @@ -847,6 +849,26 @@ module ActiveRecord end end end + + class MysqlString < Type::String # :nodoc: + def type_cast_for_database(value) + case value + when true then "1" + when false then "0" + else super + end + end + + private + + def cast_value(value) + case value + when true then "1" + when false then "0" + else super + end + end + end end end end diff --git a/activerecord/lib/active_record/type/string.rb b/activerecord/lib/active_record/type/string.rb index 150defb106..fbc0af2c5a 100644 --- a/activerecord/lib/active_record/type/string.rb +++ b/activerecord/lib/active_record/type/string.rb @@ -15,8 +15,8 @@ module ActiveRecord case value when ::Numeric, ActiveSupport::Duration then value.to_s when ::String then ::String.new(value) - when true then "1" - when false then "0" + when true then "t" + when false then "f" else super end end @@ -25,8 +25,8 @@ module ActiveRecord def cast_value(value) case value - when true then "1" - when false then "0" + when true then "t" + when false then "f" # String.new is slightly faster than dup else ::String.new(value.to_s) end -- cgit v1.2.3 From 5e4a99821dd9a710fe3ab434881cfdae15bd0838 Mon Sep 17 00:00:00 2001 From: Andrey Deryabin Date: Mon, 10 Nov 2014 22:55:40 +0300 Subject: tiny code improvement in sqlite3 adapter: - remove unused method `supports_add_column?` - change additional restriction method to `valid_alter_table_type?` - fix code style --- .../lib/active_record/connection_adapters/sqlite3_adapter.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 1b5e3bdbac..b18cb353f1 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -179,10 +179,6 @@ module ActiveRecord true end - def supports_add_column? - true - end - def supports_views? true end @@ -451,12 +447,12 @@ module ActiveRecord # See: http://www.sqlite.org/lang_altertable.html # SQLite has an additional restriction on the ALTER TABLE statement - def valid_alter_table_options( type, options) + def valid_alter_table_type?(type) type.to_sym != :primary_key end def add_column(table_name, column_name, type, options = {}) #:nodoc: - if supports_add_column? && valid_alter_table_options( type, options ) + if valid_alter_table_type?(type) super(table_name, column_name, type, options) else alter_table(table_name) do |definition| -- cgit v1.2.3 From 76e7305ea16b41a3674f4213ab7faa8c65196d75 Mon Sep 17 00:00:00 2001 From: Cody Cutrer Date: Mon, 10 Nov 2014 12:06:58 -0700 Subject: add a Table#name accessor like TableDefinition#name --- .../abstract/schema_definitions.rb | 36 ++++++++++++---------- 1 file changed, 19 insertions(+), 17 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index dd4c4ae9fc..5c4c214385 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -414,8 +414,10 @@ module ActiveRecord class Table include TimestampDefaultDeprecation + attr_reader :name + def initialize(table_name, base) - @table_name = table_name + @name = table_name @base = base end @@ -425,12 +427,12 @@ module ActiveRecord # ====== Creating a simple column # t.column(:name, :string) def column(column_name, type, options = {}) - @base.add_column(@table_name, column_name, type, options) + @base.add_column(name, column_name, type, options) end # Checks to see if a column exists. See SchemaStatements#column_exists? def column_exists?(column_name, type = nil, options = {}) - @base.column_exists?(@table_name, column_name, type, options) + @base.column_exists?(name, column_name, type, options) end # Adds a new index to the table. +column_name+ can be a single Symbol, or @@ -443,19 +445,19 @@ module ActiveRecord # ====== Creating a named index # t.index([:branch_id, :party_id], unique: true, name: 'by_branch_party') def index(column_name, options = {}) - @base.add_index(@table_name, column_name, options) + @base.add_index(name, column_name, options) end # Checks to see if an index exists. See SchemaStatements#index_exists? def index_exists?(column_name, options = {}) - @base.index_exists?(@table_name, column_name, options) + @base.index_exists?(name, column_name, options) end # Renames the given index on the table. # # t.rename_index(:user_id, :account_id) def rename_index(index_name, new_index_name) - @base.rename_index(@table_name, index_name, new_index_name) + @base.rename_index(name, index_name, new_index_name) end # Adds timestamps (+created_at+ and +updated_at+) columns to the table. See SchemaStatements#add_timestamps @@ -463,7 +465,7 @@ module ActiveRecord # t.timestamps def timestamps(options = {}) emit_warning_if_null_unspecified(options) - @base.add_timestamps(@table_name, options) + @base.add_timestamps(name, options) end # Changes the column's definition according to the new options. @@ -472,7 +474,7 @@ module ActiveRecord # t.change(:name, :string, limit: 80) # t.change(:description, :text) def change(column_name, type, options = {}) - @base.change_column(@table_name, column_name, type, options) + @base.change_column(name, column_name, type, options) end # Sets a new default value for a column. See SchemaStatements#change_column_default @@ -480,7 +482,7 @@ module ActiveRecord # t.change_default(:qualification, 'new') # t.change_default(:authorized, 1) def change_default(column_name, default) - @base.change_column_default(@table_name, column_name, default) + @base.change_column_default(name, column_name, default) end # Removes the column(s) from the table definition. @@ -488,7 +490,7 @@ module ActiveRecord # t.remove(:qualification) # t.remove(:qualification, :experience) def remove(*column_names) - @base.remove_columns(@table_name, *column_names) + @base.remove_columns(name, *column_names) end # Removes the given index from the table. @@ -502,21 +504,21 @@ module ActiveRecord # ====== Remove the index named by_branch_party in the table_name table # t.remove_index name: :by_branch_party def remove_index(options = {}) - @base.remove_index(@table_name, options) + @base.remove_index(name, options) end # Removes the timestamp columns (+created_at+ and +updated_at+) from the table. # # t.remove_timestamps def remove_timestamps - @base.remove_timestamps(@table_name) + @base.remove_timestamps(name) end # Renames a column. # # t.rename(:description, :name) def rename(column_name, new_column_name) - @base.rename_column(@table_name, column_name, new_column_name) + @base.rename_column(name, column_name, new_column_name) end # Adds a reference. Optionally adds a +type+ column, if :polymorphic option is provided. @@ -531,7 +533,7 @@ module ActiveRecord def references(*args) options = args.extract_options! args.each do |ref_name| - @base.add_reference(@table_name, ref_name, options) + @base.add_reference(name, ref_name, options) end end alias :belongs_to :references @@ -546,7 +548,7 @@ module ActiveRecord def remove_references(*args) options = args.extract_options! args.each do |ref_name| - @base.remove_reference(@table_name, ref_name, options) + @base.remove_reference(name, ref_name, options) end end alias :remove_belongs_to :remove_references @@ -558,8 +560,8 @@ module ActiveRecord [:string, :text, :integer, :float, :decimal, :datetime, :timestamp, :time, :date, :binary, :boolean].each do |column_type| define_method column_type do |*args| options = args.extract_options! - args.each do |name| - @base.add_column(@table_name, name, column_type, options) + args.each do |column_name| + @base.add_column(name, column_name, column_type, options) end end end -- cgit v1.2.3 From fd1364e021155d04318571ec08d50e2c740199db Mon Sep 17 00:00:00 2001 From: Andrey Deryabin Date: Tue, 11 Nov 2014 01:33:29 +0300 Subject: remove never called method `limited_update_conditions` --- .../active_record/connection_adapters/abstract/database_statements.rb | 4 ---- .../lib/active_record/connection_adapters/abstract_mysql_adapter.rb | 4 ---- 2 files changed, 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 1a3ed28d66..12b16b2473 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -287,10 +287,6 @@ module ActiveRecord "DEFAULT VALUES" end - def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) - "WHERE #{quoted_primary_key} IN (SELECT #{quoted_primary_key} FROM #{quoted_table_name} #{where_sql})" - end - # Sanitizes the given LIMIT parameter in order to prevent SQL injection. # # The +limit+ may be anything that can evaluate to a string via #to_s. It diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 26015528e5..167453657d 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -631,10 +631,6 @@ module ActiveRecord end end - def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key) - where_sql - end - def strict_mode? self.class.type_cast_config_to_boolean(@config.fetch(:strict, true)) end -- cgit v1.2.3 From b1879124a82b34168412ac699cf6f654e005c4d6 Mon Sep 17 00:00:00 2001 From: Andrey Deryabin Date: Wed, 12 Nov 2014 15:11:36 +0300 Subject: Follow the coding conventions --- activerecord/lib/active_record/scoping/named.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/scoping/named.rb b/activerecord/lib/active_record/scoping/named.rb index ec1edf0e01..35420e6551 100644 --- a/activerecord/lib/active_record/scoping/named.rb +++ b/activerecord/lib/active_record/scoping/named.rb @@ -139,7 +139,7 @@ module ActiveRecord # Article.published.featured.latest_article # Article.featured.titles def scope(name, body, &block) - unless body.respond_to?:call + unless body.respond_to?(:call) raise ArgumentError, 'The scope body needs to be callable.' end -- cgit v1.2.3 From 07e0d8cc0c108b2814d5c8d29f5f3f5b1db4f696 Mon Sep 17 00:00:00 2001 From: shiksha Date: Thu, 13 Nov 2014 11:13:44 +0530 Subject: Allow habtm class_name option to be consistent with other associations --- .../lib/active_record/associations/builder/has_and_belongs_to_many.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 357b28ac94..092b4ebd2f 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -110,7 +110,7 @@ module ActiveRecord::Associations::Builder rhs_options = {} if options.key? :class_name - rhs_options[:foreign_key] = options[:class_name].foreign_key + rhs_options[:foreign_key] = options[:class_name].to_s.foreign_key rhs_options[:class_name] = options[:class_name] end -- cgit v1.2.3 From aeb917d475dfccb8b5f3b5a57f62df5358ad3ef0 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 13 Nov 2014 15:31:54 -0800 Subject: exec_prepared is GVL friendly, so lets use it. also increase the version of pg required so that people will get the GVL friendly version --- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 9941f74766..6310d70192 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -13,7 +13,7 @@ require 'active_record/connection_adapters/postgresql/database_statements' require 'arel/visitors/bind_visitor' # Make sure we're using pg high enough for PGResult#values -gem 'pg', '~> 0.11' +gem 'pg', '~> 0.15' require 'pg' require 'ipaddr' @@ -596,9 +596,7 @@ module ActiveRecord } log(sql, name, type_casted_binds, stmt_key) do - @connection.send_query_prepared(stmt_key, type_casted_binds.map { |_, val| val }) - @connection.block - @connection.get_last_result + @connection.exec_prepared(stmt_key, type_casted_binds.map { |_, val| val }) end rescue ActiveRecord::StatementInvalid => e pgerror = e.original_exception -- cgit v1.2.3 From 85b40edd949678abe2a50a4291029be123cb03c5 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 14 Nov 2014 07:39:32 -0700 Subject: Use `DelegateClass` instead of `SimpleDelegator` for type decorators There is a significant performance difference between the two. Closes --- .../lib/active_record/attribute_methods/time_zone_conversion.rb | 2 +- activerecord/lib/active_record/type/serialized.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index b7fe079ef5..33d9d2002c 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -1,7 +1,7 @@ module ActiveRecord module AttributeMethods module TimeZoneConversion - class TimeZoneConverter < SimpleDelegator # :nodoc: + class TimeZoneConverter < DelegateClass(Type::Value) # :nodoc: include Type::Decorator def type_cast_from_database(value) diff --git a/activerecord/lib/active_record/type/serialized.rb b/activerecord/lib/active_record/type/serialized.rb index 17004b3593..3191a868ef 100644 --- a/activerecord/lib/active_record/type/serialized.rb +++ b/activerecord/lib/active_record/type/serialized.rb @@ -1,6 +1,6 @@ module ActiveRecord module Type - class Serialized < SimpleDelegator # :nodoc: + class Serialized < DelegateClass(Type::Value) # :nodoc: include Mutable include Decorator -- cgit v1.2.3 From 70d1b5a7f8e25b077168deaf592e0e58c3f2bdd1 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 14 Nov 2014 10:45:53 -0700 Subject: Revert "Improve performance of AR object instantiation" This reverts commit 8fee923888192a658d8823b31e77ed0683dfd665. Conflicts: activerecord/lib/active_record/attribute_set/builder.rb This solution sucks, and is hard to actually apply across the board. Going to try other solutions --- .../lib/active_record/attribute_set/builder.rb | 21 +++------------- activerecord/lib/active_record/inheritance.rb | 12 ++++------ activerecord/lib/active_record/persistence.rb | 28 +++------------------- activerecord/lib/active_record/querying.rb | 2 +- activerecord/lib/active_record/result.rb | 9 ------- 5 files changed, 11 insertions(+), 61 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 0a62c68bfb..d4a787f2fe 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -8,33 +8,18 @@ module ActiveRecord end def build_from_database(values = {}, additional_types = {}) - build_from_database_pairs(values.keys, values.values, additional_types) - end - - def build_from_database_pairs(columns, values, additional_types) - attributes = build_attributes_from_values(columns, values, additional_types) + attributes = build_attributes_from_values(values, additional_types) add_uninitialized_attributes(attributes) AttributeSet.new(attributes) end private - def build_attributes_from_values(columns, values, additional_types) - # We are performing manual iteration here as this method is a performance - # hotspot - hash = {} - index = 0 - length = columns.length - - while index < length - name = columns[index] - value = values[index] + def build_attributes_from_values(values, additional_types) + values.each_with_object({}) do |(name, value), hash| type = additional_types.fetch(name, types[name]) hash[name] = Attribute.from_database(name, value, type) - index += 1 end - - hash end def add_uninitialized_attributes(attributes) diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index 4aad3217cb..f58145ab05 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -165,19 +165,15 @@ module ActiveRecord # record instance. For single-table inheritance, we check the record # for a +type+ column and return the corresponding class. def discriminate_class_for_record(record) - discriminate_class_for_value(record[inheritance_column]) - end - - def discriminate_class_for_value(value) - if using_single_table_inheritance?(value) - find_sti_class(value) + if using_single_table_inheritance?(record) + find_sti_class(record[inheritance_column]) else super end end - def using_single_table_inheritance?(value) - value.present? && columns_hash.include?(inheritance_column) + def using_single_table_inheritance?(record) + record[inheritance_column].present? && columns_hash.include?(inheritance_column) end def find_sti_class(type_name) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 06c8bceb30..507faf2879 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -65,41 +65,19 @@ module ActiveRecord # how this "single-table" inheritance mapping is implemented. def instantiate(attributes, column_types = {}) klass = discriminate_class_for_record(attributes) - klass.instantiate_pairs(attributes.keys, attributes.values, column_types) - end - - def instantiate_pairs(columns, values, column_types = {}) # :nodoc: - attributes = attributes_builder.build_from_database_pairs(columns, values, column_types) - allocate.init_with('attributes' => attributes, 'new_record' => false) - end - - def instantiate_result_set(result_set, column_types = {}) # :nodoc: - inheritance_column_index = inheritance_column && result_set.columns.find_index(inheritance_column) - - result_set.each_pair.map do |columns, values| - inheritance_value = inheritance_column_index && values[inheritance_column_index] - klass = discriminate_class_for_value(inheritance_value) - klass.instantiate_pairs(columns, values, column_types) - end + attributes = klass.attributes_builder.build_from_database(attributes, column_types) + klass.allocate.init_with('attributes' => attributes, 'new_record' => false) end private # Called by +instantiate+ to decide which class to use for a new # record instance. # - # See +ActiveRecord::Inheritance#discriminate_class_for_value+ for + # See +ActiveRecord::Inheritance#discriminate_class_for_record+ for # the single-table inheritance discriminator. - def discriminate_class_for_value(*) - self - end - def discriminate_class_for_record(record) self end - - def inheritance_column - nil - end end # Returns true if this object hasn't been saved yet -- that is, a record diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 9d4df81b07..e8de4db3a7 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -47,7 +47,7 @@ module ActiveRecord } message_bus.instrument('instantiation.active_record', payload) do - instantiate_result_set(result_set, column_types) + result_set.map { |record| instantiate(record, column_types) } end end diff --git a/activerecord/lib/active_record/result.rb b/activerecord/lib/active_record/result.rb index c84ad586e2..3a3e65ef32 100644 --- a/activerecord/lib/active_record/result.rb +++ b/activerecord/lib/active_record/result.rb @@ -54,15 +54,6 @@ module ActiveRecord end end - def each_pair - return to_enum(__method__) unless block_given? - - columns = @columns.map { |c| c.dup.freeze } - @rows.each do |row| - yield columns, row - end - end - def to_hash hash_rows end -- cgit v1.2.3 From 0f29c216074c5da6644feddb5184c4881c078b0d Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 14 Nov 2014 11:20:28 -0700 Subject: Reduce the amount of work performed when instantiating AR models We don't know which attributes will or won't be used, and we don't want to create massive bottlenecks at instantiation. Rather than doing *any* iteration over types and values, we can lazily instantiate the object. The lazy attribute hash should not fully implement hash, or subclass hash at any point in the future. It is not meant to be a replacement, but instead implement its own interface which happens to overlap. --- .../active_record/attribute_methods/primary_key.rb | 1 + activerecord/lib/active_record/attribute_set.rb | 14 ++-- .../lib/active_record/attribute_set/builder.rb | 78 +++++++++++++++++----- activerecord/lib/active_record/core.rb | 2 - activerecord/lib/active_record/model_schema.rb | 2 +- 5 files changed, 70 insertions(+), 27 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 9bd333bbac..104d84a1f8 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -120,6 +120,7 @@ module ActiveRecord def primary_key=(value) @primary_key = value && value.to_s @quoted_primary_key = nil + @attributes_builder = nil end end end diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index 98ac63c7e1..21c58cbf1d 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -2,8 +2,6 @@ require 'active_record/attribute_set/builder' module ActiveRecord class AttributeSet # :nodoc: - delegate :keys, to: :initialized_attributes - def initialize(attributes) @attributes = attributes end @@ -25,6 +23,10 @@ module ActiveRecord attributes.key?(name) && self[name].initialized? end + def keys + attributes.initialized_keys + end + def fetch_value(name, &block) self[name].value(&block) end @@ -43,7 +45,7 @@ module ActiveRecord end def initialize_dup(_) - @attributes = attributes.transform_values(&:dup) + @attributes = attributes.dup super end @@ -58,12 +60,6 @@ module ActiveRecord end end - def ensure_initialized(key) - unless self[key].initialized? - write_from_database(key, nil) - end - end - protected attr_reader :attributes diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index d4a787f2fe..3946e02d10 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -1,35 +1,83 @@ module ActiveRecord class AttributeSet # :nodoc: class Builder # :nodoc: - attr_reader :types + attr_reader :types, :always_initialized - def initialize(types) + def initialize(types, always_initialized = nil) @types = types + @always_initialized = always_initialized end def build_from_database(values = {}, additional_types = {}) - attributes = build_attributes_from_values(values, additional_types) - add_uninitialized_attributes(attributes) + if always_initialized && !values.key?(always_initialized) + values[always_initialized] = nil + end + + attributes = LazyAttributeHash.new(types, values, additional_types) AttributeSet.new(attributes) end private + end + end + + class LazyAttributeHash + delegate :select, :transform_values, to: :materialize + delegate :[], :[]=, :freeze, to: :delegate_hash + + def initialize(types, values, additional_types) + @types = types + @values = values + @additional_types = additional_types + @materialized = false + @delegate_hash = {} + assign_default_proc + end + + def key?(key) + delegate_hash.key?(key) || values.key?(key) || types.key?(key) + end + + def initialized_keys + delegate_hash.keys | values.keys + end - def build_attributes_from_values(values, additional_types) - values.each_with_object({}) do |(name, value), hash| - type = additional_types.fetch(name, types[name]) - hash[name] = Attribute.from_database(name, value, type) + def initialize_dup(_) + @delegate_hash = delegate_hash.transform_values(&:dup) + assign_default_proc + super + end + + def initialize_clone(_) + @delegate_hash = delegate_hash.clone + super + end + + protected + + attr_reader :types, :values, :additional_types, :delegate_hash + + private + + def assign_default_proc + delegate_hash.default_proc = proc do |hash, name| + type = additional_types.fetch(name, types[name]) + + if values.key?(name) + hash[name] = Attribute.from_database(name, values[name], type) + elsif type + hash[name] = Attribute.uninitialized(name, type) end end + end - def add_uninitialized_attributes(attributes) - types.each_key do |name| - next if attributes.key? name - type = types[name] - attributes[name] = - Attribute.uninitialized(name, type) - end + def materialize + unless @materialized + values.each_key { |key| delegate_hash[key] } + types.each_key { |key| delegate_hash[key] } + @materialized = true end + delegate_hash end end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 952aeaa703..89d8932e9e 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -536,8 +536,6 @@ module ActiveRecord end def init_internals - @attributes.ensure_initialized(self.class.primary_key) - @aggregation_cache = {} @association_cache = {} @readonly = false diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index a444aac23c..adad7774b9 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -231,7 +231,7 @@ module ActiveRecord end def attributes_builder # :nodoc: - @attributes_builder ||= AttributeSet::Builder.new(column_types) + @attributes_builder ||= AttributeSet::Builder.new(column_types, primary_key) end def column_types # :nodoc: -- cgit v1.2.3 From 3f63ac4e4dd22a452a37ad7a654cf5e4f4e66349 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 14 Nov 2014 11:58:35 -0700 Subject: Correctly determine if an attribute is uninitialized In real usage, we give the builder a types hash with a default value of `Type::Value.new`. This means we need to explicitly check for the key, rather than the truthiness of the type to determine if it's a known but uninitialized attribute --- activerecord/lib/active_record/attribute_set/builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 3946e02d10..54e9cbb779 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -65,7 +65,7 @@ module ActiveRecord if values.key?(name) hash[name] = Attribute.from_database(name, values[name], type) - elsif type + elsif types.key?(name) hash[name] = Attribute.uninitialized(name, type) end end -- cgit v1.2.3 From 895a53e7161e9bdc6285fe03f16b49ec74972416 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 14 Nov 2014 12:02:05 -0700 Subject: Allow `LazyAttributeHash` to be marshalled `default_proc` makes a hash unmarshallable, and adds unneccessary overhead. Since we control all access to the hash, let's just handle it in that method. This has the side effect of improving performance on initialization (but not neccessarily on access). We'll need to profile further once the tests are passing. --- .../lib/active_record/attribute_set/builder.rb | 30 ++++++++++++---------- 1 file changed, 17 insertions(+), 13 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 54e9cbb779..4134032659 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -23,7 +23,7 @@ module ActiveRecord class LazyAttributeHash delegate :select, :transform_values, to: :materialize - delegate :[], :[]=, :freeze, to: :delegate_hash + delegate :[]=, :freeze, to: :delegate_hash def initialize(types, values, additional_types) @types = types @@ -31,20 +31,26 @@ module ActiveRecord @additional_types = additional_types @materialized = false @delegate_hash = {} - assign_default_proc end def key?(key) delegate_hash.key?(key) || values.key?(key) || types.key?(key) end + def [](key) + if delegate_hash.key?(key) + delegate_hash[key] + else + assign_default_value(key) + end + end + def initialized_keys delegate_hash.keys | values.keys end def initialize_dup(_) @delegate_hash = delegate_hash.transform_values(&:dup) - assign_default_proc super end @@ -59,22 +65,20 @@ module ActiveRecord private - def assign_default_proc - delegate_hash.default_proc = proc do |hash, name| - type = additional_types.fetch(name, types[name]) + def assign_default_value(name) + type = additional_types.fetch(name, types[name]) - if values.key?(name) - hash[name] = Attribute.from_database(name, values[name], type) - elsif types.key?(name) - hash[name] = Attribute.uninitialized(name, type) - end + if values.key?(name) + delegate_hash[name] = Attribute.from_database(name, values[name], type) + elsif types.key?(name) + delegate_hash[name] = Attribute.uninitialized(name, type) end end def materialize unless @materialized - values.each_key { |key| delegate_hash[key] } - types.each_key { |key| delegate_hash[key] } + values.each_key { |key| self[key] } + types.each_key { |key| self[key] } @materialized = true end delegate_hash -- cgit v1.2.3 From 48f2620df98e6926d38b8951087ab1bbc512580c Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 14 Nov 2014 12:37:25 -0700 Subject: Don't freeze the same hash we use for memoization --- activerecord/lib/active_record/attribute_set/builder.rb | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 4134032659..73b77d2eda 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -23,7 +23,6 @@ module ActiveRecord class LazyAttributeHash delegate :select, :transform_values, to: :materialize - delegate :[]=, :freeze, to: :delegate_hash def initialize(types, values, additional_types) @types = types @@ -45,6 +44,13 @@ module ActiveRecord end end + def []=(key, value) + if frozen? + raise RuntimeError, "Can't modify frozen hash" + end + delegate_hash[key] = value + end + def initialized_keys delegate_hash.keys | values.keys end @@ -54,11 +60,6 @@ module ActiveRecord super end - def initialize_clone(_) - @delegate_hash = delegate_hash.clone - super - end - protected attr_reader :types, :values, :additional_types, :delegate_hash -- cgit v1.2.3 From da99a2a2982d35f670ad9647463e09bfe9032b70 Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 17 Nov 2014 14:55:17 +1100 Subject: PERF: optimise type lookup to avoid invoking procs --- .../connection_adapters/postgresql_adapter.rb | 20 +++++++++++++------- .../lib/active_record/type/hash_lookup_type_map.rb | 20 +++++++++++++++++++- 2 files changed, 32 insertions(+), 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 6310d70192..d3e5b0a4ad 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -431,16 +431,22 @@ module ActiveRecord private def get_oid_type(oid, fmod, column_name, sql_type = '') # :nodoc: - if !type_map.key?(oid) + + result = type_map.fetch(oid, fmod, sql_type) { + nil + } + + unless result load_additional_types(type_map, [oid]) + result = type_map.fetch(oid, fmod, sql_type) { + warn "unknown OID #{oid}: failed to recognize type of '#{column_name}'. It will be treated as String." + Type::Value.new.tap do |cast_type| + type_map.register_type(oid, cast_type) + end + } end - type_map.fetch(oid, fmod, sql_type) { - warn "unknown OID #{oid}: failed to recognize type of '#{column_name}'. It will be treated as String." - Type::Value.new.tap do |cast_type| - type_map.register_type(oid, cast_type) - end - } + result end def initialize_type_map(m) # :nodoc: diff --git a/activerecord/lib/active_record/type/hash_lookup_type_map.rb b/activerecord/lib/active_record/type/hash_lookup_type_map.rb index bf92680268..2a146d38a9 100644 --- a/activerecord/lib/active_record/type/hash_lookup_type_map.rb +++ b/activerecord/lib/active_record/type/hash_lookup_type_map.rb @@ -1,6 +1,12 @@ module ActiveRecord module Type class HashLookupTypeMap < TypeMap # :nodoc: + + def initialize + @cache = {} + super + end + delegate :key?, to: :@mapping def lookup(type, *args) @@ -8,12 +14,24 @@ module ActiveRecord end def fetch(type, *args, &block) - @mapping.fetch(type, block).call(type, *args) + cache = (@cache[type] ||= {}) + resolved = cache[args] + + unless resolved + resolved = cache[args] = @mapping.fetch(type, block).call(type, *args) + end + + resolved end def alias_type(type, alias_type) register_type(type) { |_, *args| lookup(alias_type, *args) } end + + def register_type(key, value=nil, &block) + @cache = {} + super(key, value, &block) + end end end end -- cgit v1.2.3 From 409e7e4fc5332bf6199ac927d816c51f1d6a3307 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 17 Nov 2014 05:59:06 -0800 Subject: Remove pointless `private` This class no longer has any private methods --- activerecord/lib/active_record/attribute_set/builder.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 73b77d2eda..60154a0647 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -16,8 +16,6 @@ module ActiveRecord attributes = LazyAttributeHash.new(types, values, additional_types) AttributeSet.new(attributes) end - - private end end -- cgit v1.2.3 From 29b3e548f32c42cb99d9dd2d03c2cd34fa53d867 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 17 Nov 2014 07:11:13 -0800 Subject: Improve the performance of quoting table names on PG This caused a pretty major performance regression for 4.2, as this is a hotspot for query construction. We're still slightly slower than 4.1, but it's much less significant. --- .../connection_adapters/postgresql/utils.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb b/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb index 0290bcb48c..9a0b80d7d3 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/utils.rb @@ -18,7 +18,11 @@ module ActiveRecord end def quoted - parts.map { |p| PGconn.quote_ident(p) }.join SEPARATOR + if schema + PGconn.quote_ident(schema) << SEPARATOR << PGconn.quote_ident(identifier) + else + PGconn.quote_ident(identifier) + end end def ==(o) @@ -32,8 +36,11 @@ module ActiveRecord protected def unquote(part) - return unless part - part.gsub(/(^"|"$)/,'') + if part && part.start_with?('"') + part[1..-2] + else + part + end end def parts @@ -57,7 +64,11 @@ module ActiveRecord # * "schema_name".table_name # * "schema.name"."table name" def extract_schema_qualified_name(string) - table, schema = string.scan(/[^".\s]+|"[^"]*"/)[0..1].reverse + schema, table = string.scan(/[^".\s]+|"[^"]*"/) + if table.nil? + table = schema + schema = nil + end PostgreSQL::Name.new(schema, table) end end -- cgit v1.2.3 From c01b20b658c9fe4b7d54f4a227a09cb090b5763d Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 17 Nov 2014 14:56:06 -0800 Subject: rm `reorder_bind_params` Arel handles this for us automatically. Updated tests, as BindParam is no longer a subclass of SqlLiteral. We should remove the second argument to substitute_at entirely, as it's no longer used --- .../lib/active_record/connection_adapters/abstract_adapter.rb | 2 +- .../connection_adapters/postgresql/database_statements.rb | 4 ---- activerecord/lib/active_record/relation.rb | 1 - activerecord/lib/active_record/relation/query_methods.rb | 10 ---------- 4 files changed, 1 insertion(+), 16 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 582dd360f0..a2df76259b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -265,7 +265,7 @@ module ActiveRecord # Returns a bind substitution value given a bind +index+ and +column+ # NOTE: The column param is currently being used by the sqlserver-adapter def substitute_at(column, index = 0) - Arel::Nodes::BindParam.new '?' + Arel::Nodes::BindParam.new end # REFERENTIAL INTEGRITY ==================================== diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index cf379ab210..d09468329a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -156,10 +156,6 @@ module ActiveRecord end end - def substitute_at(column, index = 0) - Arel::Nodes::BindParam.new "$#{index + 1}" - end - def exec_query(sql, name = 'SQL', binds = []) execute_and_clear(sql, name, binds) do |result| types = {} diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 03bce4f5b7..3aaf037d31 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -84,7 +84,6 @@ module ActiveRecord um = relation .arel .compile_update(substitutes, @klass.primary_key) - reorder_bind_params(um.ast, bvs) @klass.connection.update( um, diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index a686e3263b..cb4e33f1b1 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -881,19 +881,9 @@ module ActiveRecord arel.from(build_from) if from_value arel.lock(lock_value) if lock_value - # Reorder bind indexes if joins produced bind values - bvs = arel.bind_values + bind_values - reorder_bind_params(arel.ast, bvs) arel end - def reorder_bind_params(ast, bvs) - ast.grep(Arel::Nodes::BindParam).each_with_index do |bp, i| - column = bvs[i].first - bp.replace connection.substitute_at(column, i) - end - end - def symbol_unscoping(scope) if !VALID_UNSCOPING_VALUES.include?(scope) raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}." -- cgit v1.2.3 From eb26f24bde62cbbcd8ef0e7ee9c64060b098baff Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 17 Nov 2014 15:04:40 -0800 Subject: Remove the unused second argument to `substitute_at` Oh hey, we got to remove some code because of that! --- .../lib/active_record/associations/association_scope.rb | 3 +-- .../associations/join_dependency/join_association.rb | 2 +- .../active_record/connection_adapters/abstract_adapter.rb | 4 ++-- activerecord/lib/active_record/locking/optimistic.rb | 2 +- activerecord/lib/active_record/persistence.rb | 2 +- activerecord/lib/active_record/relation.rb | 2 +- activerecord/lib/active_record/relation/merger.rb | 12 ------------ 7 files changed, 7 insertions(+), 20 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb index b965230e60..dcbd57e61d 100644 --- a/activerecord/lib/active_record/associations/association_scope.rb +++ b/activerecord/lib/active_record/associations/association_scope.rb @@ -11,8 +11,7 @@ module ActiveRecord end def bind_value(scope, column, value, alias_tracker) - substitute = alias_tracker.connection.substitute_at( - column, scope.bind_values.length) + substitute = alias_tracker.connection.substitute_at(column) scope.bind_values += [[column, @block.call(value)]] substitute end diff --git a/activerecord/lib/active_record/associations/join_dependency/join_association.rb b/activerecord/lib/active_record/associations/join_dependency/join_association.rb index e7d3c9ba40..5dede5527d 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -67,7 +67,7 @@ module ActiveRecord value = foreign_klass.base_class.name column = klass.columns_hash[reflection.type.to_s] - substitute = klass.connection.substitute_at(column, bind_values.length) + substitute = klass.connection.substitute_at(column) bind_values.push [column, value] constraint = constraint.and table[reflection.type].eq substitute end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index a2df76259b..57aa2f9c58 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -262,9 +262,9 @@ module ActiveRecord # QUOTING ================================================== - # Returns a bind substitution value given a bind +index+ and +column+ + # Returns a bind substitution value given a bind +column+ # NOTE: The column param is currently being used by the sqlserver-adapter - def substitute_at(column, index = 0) + def substitute_at(column) Arel::Nodes::BindParam.new end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 52eeb8ae1f..ced694ba9a 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -120,7 +120,7 @@ module ActiveRecord if locking_enabled? column_name = self.class.locking_column column = self.class.columns_hash[column_name] - substitute = self.class.connection.substitute_at(column, relation.bind_values.length) + substitute = self.class.connection.substitute_at(column) relation = relation.where(self.class.arel_table[column_name].eq(substitute)) relation.bind_values << [column, self[column_name].to_i] diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 507faf2879..ee3b7b6163 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -487,7 +487,7 @@ module ActiveRecord def relation_for_destroy pk = self.class.primary_key column = self.class.columns_hash[pk] - substitute = self.class.connection.substitute_at(column, 0) + substitute = self.class.connection.substitute_at(column) relation = self.class.unscoped.where( self.class.arel_table[pk].eq(substitute)) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 3aaf037d31..8575e5a7be 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -98,7 +98,7 @@ module ActiveRecord end substitutes = values.each_with_index.map do |(arel_attr, _), i| - [arel_attr, @klass.connection.substitute_at(binds[i][0], i)] + [arel_attr, @klass.connection.substitute_at(binds[i][0])] end [substitutes, binds] diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index a8febf6a18..a27f990f74 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -118,18 +118,6 @@ module ActiveRecord where_values = kept + rhs_wheres bind_values = filter_binds(lhs_binds, removed) + rhs_binds - conn = relation.klass.connection - bv_index = 0 - where_values.map! do |node| - if Arel::Nodes::Equality === node && Arel::Nodes::BindParam === node.right - substitute = conn.substitute_at(bind_values[bv_index].first, bv_index) - bv_index += 1 - Arel::Nodes::Equality.new(node.left, substitute) - else - node - end - end - relation.where_values = where_values relation.bind_values = bind_values -- cgit v1.2.3 From 6f7910aed5ceeb8833de760f396858bee917c7bc Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Mon, 17 Nov 2014 17:58:49 -0800 Subject: [PERF] Speed up integer type casting from DB MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't have the check the range when the value is coming from the DB, so override type_cast_from_database to short-circuit the extra work. type_cast_from_database (small) 3437507.5 (±29.2%) i/s - 14223135 in 4.996973s type_cast_from_database (large) 3158588.7 (±28.3%) i/s - 13265628 in 4.992121s type_cast (small) 481984.8 (±14.2%) i/s - 2352012 in 5.005694s type_cast (large) 477331.8 (±14.2%) i/s - 2332824 in 5.012365s Comparison: type_cast_from_database (small): 3437507.5 i/s type_cast_from_database (large): 3158588.7 i/s - 1.09x slower type_cast (small): 481984.8 i/s - 7.13x slower type_cast (large): 477331.8 i/s - 7.20x slower The difference is huge but the absolute gain is quite small. That being said this is a hotspot and it showed up on the radar when benchmarking discourse. --- activerecord/lib/active_record/type/integer.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb index d69e5b3f28..fb6d9df58f 100644 --- a/activerecord/lib/active_record/type/integer.rb +++ b/activerecord/lib/active_record/type/integer.rb @@ -12,7 +12,9 @@ module ActiveRecord :integer end - alias type_cast_for_database type_cast + def type_cast_from_database(value) + value.to_i unless value.nil? + end protected -- cgit v1.2.3 From 52c70d496f34ed4d6c9c8cc1719bc7f86011634b Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Mon, 17 Nov 2014 18:09:40 -0800 Subject: :nail_care: Put escape clause first, keeps @sgrif happy :grin: See comment on 6f7910a --- activerecord/lib/active_record/type/integer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb index fb6d9df58f..0149d60b6e 100644 --- a/activerecord/lib/active_record/type/integer.rb +++ b/activerecord/lib/active_record/type/integer.rb @@ -13,7 +13,8 @@ module ActiveRecord end def type_cast_from_database(value) - value.to_i unless value.nil? + return if value.nil? + value.to_i end protected -- cgit v1.2.3 From 3098579f276d5cecbca0b57e35526b99fadfbe5f Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Mon, 17 Nov 2014 18:33:07 -0800 Subject: Revert "[PERF] Speed up integer type casting from DB" This reverts commit 6f7910a and 52c70d4. Query params are type cased through the same method, so this approach doesn't work. --- activerecord/lib/active_record/type/integer.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb index 0149d60b6e..d69e5b3f28 100644 --- a/activerecord/lib/active_record/type/integer.rb +++ b/activerecord/lib/active_record/type/integer.rb @@ -12,10 +12,7 @@ module ActiveRecord :integer end - def type_cast_from_database(value) - return if value.nil? - value.to_i - end + alias type_cast_for_database type_cast protected -- cgit v1.2.3 From 9aa2b2d7bf52c0f3974090cd033341be2edeae6d Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 18 Nov 2014 21:38:27 +1100 Subject: PERF: stop allocating the string "id" over and over --- activerecord/lib/active_record/attribute_methods/read.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index bf2a084a00..55beb309ca 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -76,12 +76,14 @@ module ActiveRecord end end + ID = 'id'.freeze + # Returns the value of the attribute identified by attr_name after # it has been typecast (for example, "2004-12-12" in a date column is cast # to a date object, like Date.new(2004, 12, 12)). def read_attribute(attr_name, &block) name = attr_name.to_s - name = self.class.primary_key if name == 'id' + name = self.class.primary_key if name == ID @attributes.fetch_value(name, &block) end -- cgit v1.2.3 From 8b2c98f6c7c0b7371265905a43fc7d3ad966643a Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 18 Nov 2014 11:26:16 -0800 Subject: Remove call to `key?` in `LazyAttributeHash#[]` Performance improvement, as well as improved code clarity --- activerecord/lib/active_record/attribute_set/builder.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index 60154a0647..b433205af3 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -35,11 +35,7 @@ module ActiveRecord end def [](key) - if delegate_hash.key?(key) - delegate_hash[key] - else - assign_default_value(key) - end + delegate_hash[key] || assign_default_value(key) end def []=(key, value) -- cgit v1.2.3 From 3ecc5d30eb0901f4e3c4d3bca1957f633a6a695b Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 18 Nov 2014 11:46:17 -0800 Subject: LazyAttributeHash is private --- activerecord/lib/active_record/attribute_set/builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index b433205af3..fd134a304b 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -19,7 +19,7 @@ module ActiveRecord end end - class LazyAttributeHash + class LazyAttributeHash # :nodoc: delegate :select, :transform_values, to: :materialize def initialize(types, values, additional_types) -- cgit v1.2.3 From edc39ff756d09b746bdc1acb8d814c3148289846 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 18 Nov 2014 12:31:23 -0800 Subject: Remove needless call to `key?` when building attributes This appears to be a performance hotspot, see #17655. --- activerecord/lib/active_record/attribute_set/builder.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_set/builder.rb b/activerecord/lib/active_record/attribute_set/builder.rb index fd134a304b..05138ae36d 100644 --- a/activerecord/lib/active_record/attribute_set/builder.rb +++ b/activerecord/lib/active_record/attribute_set/builder.rb @@ -62,9 +62,11 @@ module ActiveRecord def assign_default_value(name) type = additional_types.fetch(name, types[name]) + value_present = true + value = values.fetch(name) { value_present = false } - if values.key?(name) - delegate_hash[name] = Attribute.from_database(name, values[name], type) + if value_present + delegate_hash[name] = Attribute.from_database(name, value, type) elsif types.key?(name) delegate_hash[name] = Attribute.uninitialized(name, type) end -- cgit v1.2.3 From 52fddcc653458456f98b3683dffd781cf00b35fe Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 18 Nov 2014 13:40:03 -0800 Subject: Speed up integer casting from DB We don't have the check the range when the value is coming from the DB, so override type_cast_from_database to short-circuit the extra work. The difference is huge but the absolute gain is quite small. That being said this is a hotspot and it showed up on the radar when benchmarking discourse. --- activerecord/lib/active_record/type/integer.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb index d69e5b3f28..36bbd9cd5e 100644 --- a/activerecord/lib/active_record/type/integer.rb +++ b/activerecord/lib/active_record/type/integer.rb @@ -14,6 +14,11 @@ module ActiveRecord alias type_cast_for_database type_cast + def type_cast_from_database(value) + return if value.nil? + value.to_i + end + protected attr_reader :range -- cgit v1.2.3 From 78e7a0d3b7eaafe1ad0a45e3e355e1123cae3f8b Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 18 Nov 2014 14:22:33 -0800 Subject: pull the preloader allocation in to a factory method --- activerecord/lib/active_record/relation.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 8575e5a7be..460daf99bc 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -639,7 +639,7 @@ module ActiveRecord preload = preload_values preload += includes_values unless eager_loading? - preloader = ActiveRecord::Associations::Preloader.new + preloader = build_preloader preload.each do |associations| preloader.preload @records, associations end @@ -650,6 +650,10 @@ module ActiveRecord @records end + def build_preloader + ActiveRecord::Associations::Preloader.new + end + def references_eager_loaded_tables? joined_tables = arel.join_sources.map do |join| if join.is_a?(Arel::Nodes::StringJoin) -- cgit v1.2.3 From 08576b94ad4f19dfc368619d7751e211d23dcad8 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 18 Nov 2014 15:19:15 -0800 Subject: Improve the performance of reading attributes We added a comparison to "id", and call to `self.class.primary_key` a *lot*. We also have performance hits from `&block` all over the place. We skip the check in a new method, in order to avoid breaking the behavior of `read_attribute` --- activerecord/lib/active_record/aggregations.rb | 4 ++-- .../lib/active_record/associations/has_many_association.rb | 2 +- .../associations/has_many_through_association.rb | 2 +- activerecord/lib/active_record/attribute_methods.rb | 4 ++-- activerecord/lib/active_record/attribute_methods/dirty.rb | 2 +- .../lib/active_record/attribute_methods/primary_key.rb | 2 +- activerecord/lib/active_record/attribute_methods/read.rb | 14 ++++++++++---- activerecord/lib/active_record/attribute_set.rb | 4 ++-- activerecord/lib/active_record/enum.rb | 2 +- activerecord/lib/active_record/reflection.rb | 2 +- activerecord/lib/active_record/validations/uniqueness.rb | 2 +- 11 files changed, 23 insertions(+), 17 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/aggregations.rb b/activerecord/lib/active_record/aggregations.rb index e576ec4d40..1040e6e3bb 100644 --- a/activerecord/lib/active_record/aggregations.rb +++ b/activerecord/lib/active_record/aggregations.rb @@ -230,8 +230,8 @@ module ActiveRecord private def reader_method(name, class_name, mapping, allow_nil, constructor) define_method(name) do - if @aggregation_cache[name].nil? && (!allow_nil || mapping.any? {|key, _| !read_attribute(key).nil? }) - attrs = mapping.collect {|key, _| read_attribute(key)} + if @aggregation_cache[name].nil? && (!allow_nil || mapping.any? {|key, _| !_read_attribute(key).nil? }) + attrs = mapping.collect {|key, _| _read_attribute(key)} object = constructor.respond_to?(:call) ? constructor.call(*attrs) : class_name.constantize.send(constructor, *attrs) diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 1413efaf7f..93084e0dcf 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -66,7 +66,7 @@ module ActiveRecord # the loaded flag is set to true as well. def count_records count = if has_cached_counter? - owner.read_attribute cached_counter_attribute_name + owner._read_attribute cached_counter_attribute_name else scope.count end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index bde23fc116..6329fdfe95 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -20,7 +20,7 @@ module ActiveRecord # SELECT query will be generated by using #length instead. def size if has_cached_counter? - owner.read_attribute cached_counter_attribute_name(reflection) + owner._read_attribute cached_counter_attribute_name(reflection) elsif loaded? target.size else diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 34ec397aee..d766996d37 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -332,7 +332,7 @@ module ActiveRecord # task.attribute_present?(:title) # => true # task.attribute_present?(:is_done) # => true def attribute_present?(attribute) - value = read_attribute(attribute) + value = _read_attribute(attribute) !value.nil? && !(value.respond_to?(:empty?) && value.empty?) end @@ -433,7 +433,7 @@ module ActiveRecord end def typecasted_attribute_value(name) - read_attribute(name) + _read_attribute(name) end end end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 2f02738f6d..9ba46ec4c7 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -110,7 +110,7 @@ module ActiveRecord if attribute_changed?(attr) changed_attributes[attr] else - clone_attribute_value(:read_attribute, attr) + clone_attribute_value(:_read_attribute, attr) end end diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index 104d84a1f8..c28374e4ab 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -17,7 +17,7 @@ module ActiveRecord def id if pk = self.class.primary_key sync_with_transaction_state - read_attribute(pk) + _read_attribute(pk) end end diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 55beb309ca..20f0936e52 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -27,7 +27,7 @@ module ActiveRecord <<-EOMETHOD def #{method_name} name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{const_name} - read_attribute(name) { |n| missing_attribute(n, caller) } + _read_attribute(name) { |n| missing_attribute(n, caller) } end EOMETHOD end @@ -64,7 +64,7 @@ module ActiveRecord generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 def #{temp_method} name = ::ActiveRecord::AttributeMethods::AttrNames::ATTR_#{safe_name} - read_attribute(name) { |n| missing_attribute(n, caller) } + _read_attribute(name) { |n| missing_attribute(n, caller) } end STR @@ -84,13 +84,19 @@ module ActiveRecord def read_attribute(attr_name, &block) name = attr_name.to_s name = self.class.primary_key if name == ID - @attributes.fetch_value(name, &block) + _read_attribute(name, &block) + end + + # This method exists to avoid the expensive primary_key check internally, without + # breaking compatibility with the read_attribute API + def _read_attribute(attr_name) # :nodoc: + @attributes.fetch_value(attr_name.to_s) { |n| yield n if block_given? } end private def attribute(attribute_name) - read_attribute(attribute_name) + _read_attribute(attribute_name) end end end diff --git a/activerecord/lib/active_record/attribute_set.rb b/activerecord/lib/active_record/attribute_set.rb index 21c58cbf1d..6b1d7ea79e 100644 --- a/activerecord/lib/active_record/attribute_set.rb +++ b/activerecord/lib/active_record/attribute_set.rb @@ -27,8 +27,8 @@ module ActiveRecord attributes.initialized_keys end - def fetch_value(name, &block) - self[name].value(&block) + def fetch_value(name) + self[name].value { |n| yield n if block_given? } end def write_from_database(name, value) diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 5958373e88..f053372cfb 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -142,7 +142,7 @@ module ActiveRecord private def save_changed_attribute(attr_name, old) if (mapping = self.class.defined_enums[attr_name.to_s]) - value = read_attribute(attr_name) + value = _read_attribute(attr_name) if attribute_changed?(attr_name) if mapping[old] == value clear_attribute_changes([attr_name]) diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 4b58f2deb7..f4a351b092 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -287,7 +287,7 @@ module ActiveRecord def association_scope_cache(conn, owner) key = conn.prepared_statements if polymorphic? - key = [key, owner.read_attribute(@foreign_type)] + key = [key, owner._read_attribute(@foreign_type)] end @association_scope_cache[key] ||= @scope_lock.synchronize { @association_scope_cache[key] ||= yield diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 2dba4c7b94..3e8afe37a8 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -79,7 +79,7 @@ module ActiveRecord scope_value = record.send(reflection.foreign_key) scope_item = reflection.foreign_key else - scope_value = record.read_attribute(scope_item) + scope_value = record._read_attribute(scope_item) end relation = relation.and(table[scope_item].eq(scope_value)) end -- cgit v1.2.3 From 995c697511a4e77fbe1e00793486ae07b18c5017 Mon Sep 17 00:00:00 2001 From: dtaniwaki Date: Wed, 19 Nov 2014 10:17:44 +0900 Subject: Support symbol foreign key to delete --- .../lib/active_record/connection_adapters/abstract/schema_statements.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index cbf87df356..cc86c3776e 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -740,7 +740,7 @@ module ActiveRecord end fk_name_to_delete = options.fetch(:name) do - fk_to_delete = foreign_keys(from_table).detect {|fk| fk.column == options[:column] } + fk_to_delete = foreign_keys(from_table).detect {|fk| fk.column == options[:column].to_s } if fk_to_delete fk_to_delete.name -- cgit v1.2.3 From b03f9ef86a44b190948be0737bf9ab68e30fc112 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 19 Nov 2014 11:24:49 -0800 Subject: Revert "PERF: optimise type lookup to avoid invoking procs" This reverts commit da99a2a2982d35f670ad9647463e09bfe9032b70. --- .../connection_adapters/postgresql_adapter.rb | 20 +++++++------------- .../lib/active_record/type/hash_lookup_type_map.rb | 20 +------------------- 2 files changed, 8 insertions(+), 32 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index d3e5b0a4ad..6310d70192 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -431,22 +431,16 @@ module ActiveRecord private def get_oid_type(oid, fmod, column_name, sql_type = '') # :nodoc: - - result = type_map.fetch(oid, fmod, sql_type) { - nil - } - - unless result + if !type_map.key?(oid) load_additional_types(type_map, [oid]) - result = type_map.fetch(oid, fmod, sql_type) { - warn "unknown OID #{oid}: failed to recognize type of '#{column_name}'. It will be treated as String." - Type::Value.new.tap do |cast_type| - type_map.register_type(oid, cast_type) - end - } end - result + type_map.fetch(oid, fmod, sql_type) { + warn "unknown OID #{oid}: failed to recognize type of '#{column_name}'. It will be treated as String." + Type::Value.new.tap do |cast_type| + type_map.register_type(oid, cast_type) + end + } end def initialize_type_map(m) # :nodoc: diff --git a/activerecord/lib/active_record/type/hash_lookup_type_map.rb b/activerecord/lib/active_record/type/hash_lookup_type_map.rb index 2a146d38a9..bf92680268 100644 --- a/activerecord/lib/active_record/type/hash_lookup_type_map.rb +++ b/activerecord/lib/active_record/type/hash_lookup_type_map.rb @@ -1,12 +1,6 @@ module ActiveRecord module Type class HashLookupTypeMap < TypeMap # :nodoc: - - def initialize - @cache = {} - super - end - delegate :key?, to: :@mapping def lookup(type, *args) @@ -14,24 +8,12 @@ module ActiveRecord end def fetch(type, *args, &block) - cache = (@cache[type] ||= {}) - resolved = cache[args] - - unless resolved - resolved = cache[args] = @mapping.fetch(type, block).call(type, *args) - end - - resolved + @mapping.fetch(type, block).call(type, *args) end def alias_type(type, alias_type) register_type(type) { |_, *args| lookup(alias_type, *args) } end - - def register_type(key, value=nil, &block) - @cache = {} - super(key, value, &block) - end end end end -- cgit v1.2.3 From cafed35b610d830049b7dffd924cbcdeedab72bb Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 19 Nov 2014 11:54:19 -0800 Subject: Add tests for `TypeMap#fetch` and push up to `TypeMap` It doesn't make sense for the subclass to implement this method, and not have it on the parent. We can also DRY up the implementation of `#lookup` to be defined in terms of fetch, which will give us a single point of entry --- activerecord/lib/active_record/type/hash_lookup_type_map.rb | 4 ---- activerecord/lib/active_record/type/type_map.rb | 6 +++++- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/hash_lookup_type_map.rb b/activerecord/lib/active_record/type/hash_lookup_type_map.rb index bf92680268..90a8c674cb 100644 --- a/activerecord/lib/active_record/type/hash_lookup_type_map.rb +++ b/activerecord/lib/active_record/type/hash_lookup_type_map.rb @@ -3,10 +3,6 @@ module ActiveRecord class HashLookupTypeMap < TypeMap # :nodoc: delegate :key?, to: :@mapping - def lookup(type, *args) - @mapping.fetch(type, proc { default_value }).call(type, *args) - end - def fetch(type, *args, &block) @mapping.fetch(type, block).call(type, *args) end diff --git a/activerecord/lib/active_record/type/type_map.rb b/activerecord/lib/active_record/type/type_map.rb index 88c5f9c497..13d4943861 100644 --- a/activerecord/lib/active_record/type/type_map.rb +++ b/activerecord/lib/active_record/type/type_map.rb @@ -6,6 +6,10 @@ module ActiveRecord end def lookup(lookup_key, *args) + fetch(lookup_key, *args) { default_value } + end + + def fetch(lookup_key, *args) matching_pair = @mapping.reverse_each.detect do |key, _| key === lookup_key end @@ -13,7 +17,7 @@ module ActiveRecord if matching_pair matching_pair.last.call(lookup_key, *args) else - default_value + yield lookup_key, *args end end -- cgit v1.2.3 From f767ac22fa213df754e160c59189d28ed4f95568 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 19 Nov 2014 12:04:51 -0800 Subject: Reintroduce cache with tests --- .../lib/active_record/type/hash_lookup_type_map.rb | 10 ++++---- activerecord/lib/active_record/type/type_map.rb | 28 ++++++++++++++-------- 2 files changed, 24 insertions(+), 14 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/hash_lookup_type_map.rb b/activerecord/lib/active_record/type/hash_lookup_type_map.rb index 90a8c674cb..82d9327fc0 100644 --- a/activerecord/lib/active_record/type/hash_lookup_type_map.rb +++ b/activerecord/lib/active_record/type/hash_lookup_type_map.rb @@ -3,13 +3,15 @@ module ActiveRecord class HashLookupTypeMap < TypeMap # :nodoc: delegate :key?, to: :@mapping - def fetch(type, *args, &block) - @mapping.fetch(type, block).call(type, *args) - end - def alias_type(type, alias_type) register_type(type) { |_, *args| lookup(alias_type, *args) } end + + private + + def perform_fetch(type, *args, &block) + @mapping.fetch(type, block).call(type, *args) + end end end end diff --git a/activerecord/lib/active_record/type/type_map.rb b/activerecord/lib/active_record/type/type_map.rb index 13d4943861..7c194c0cdf 100644 --- a/activerecord/lib/active_record/type/type_map.rb +++ b/activerecord/lib/active_record/type/type_map.rb @@ -3,26 +3,22 @@ module ActiveRecord class TypeMap # :nodoc: def initialize @mapping = {} + @cache = Hash.new do |h, key| + h[key] = {} + end end def lookup(lookup_key, *args) fetch(lookup_key, *args) { default_value } end - def fetch(lookup_key, *args) - matching_pair = @mapping.reverse_each.detect do |key, _| - key === lookup_key - end - - if matching_pair - matching_pair.last.call(lookup_key, *args) - else - yield lookup_key, *args - end + def fetch(lookup_key, *args, &block) + @cache[lookup_key][args] ||= perform_fetch(lookup_key, *args, &block) end def register_type(key, value = nil, &block) raise ::ArgumentError unless value || block + @cache.clear if block @mapping[key] = block @@ -44,6 +40,18 @@ module ActiveRecord private + def perform_fetch(lookup_key, *args) + matching_pair = @mapping.reverse_each.detect do |key, _| + key === lookup_key + end + + if matching_pair + matching_pair.last.call(lookup_key, *args) + else + yield lookup_key, *args + end + end + def default_value @default_value ||= Value.new end -- cgit v1.2.3 From a668b09ee387bf241a3481a764a18a5c8a4df8cd Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 19 Nov 2014 08:44:45 +1100 Subject: PERF: avoid string allocations --- activerecord/lib/active_record/relation/query_methods.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index cb4e33f1b1..eb69943551 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1059,8 +1059,13 @@ module ActiveRecord def build_select(arel, selects) if !selects.empty? expanded_select = selects.map do |field| - columns_hash.key?(field.to_s) ? arel_table[field] : field + if (Symbol === field || String === field) && columns_hash.key?(field.to_s) + arel_table[field] + else + field + end end + arel.project(*expanded_select) else arel.project(@klass.arel_table[Arel.star]) -- cgit v1.2.3 From 9ce105734d0b28de324c79a9e89906219f1555b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20den=20Berg?= Date: Thu, 20 Nov 2014 09:58:45 +0100 Subject: Explain that default_scope also influences update_all This was not explicitly stated before and I needed to try it out to be certain. A little explicit statement in the API docs might help here. --- activerecord/lib/active_record/relation.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 460daf99bc..fc3306ee81 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -304,11 +304,12 @@ module ActiveRecord klass.current_scope = previous end - # Updates all records with details given if they match a set of conditions supplied, limits and order can - # also be supplied. This method constructs a single SQL UPDATE statement and sends it straight to the - # database. It does not instantiate the involved models and it does not trigger Active Record callbacks - # or validations. Values passed to `update_all` will not go through ActiveRecord's type-casting behavior. - # It should receive only values that can be passed as-is to the SQL database. + # Updates all records in the current scope (respecting the default_scope, where, + # limit and order specified) with details given. This method constructs a single SQL update_all + # statement and sends it straight to the database. It does not instantiate the involved models and it does not + # trigger Active Record callbacks or validations. Values passed to `update_all` will not go through + # ActiveRecord's type-casting behavior. It should receive only values that can be passed as-is to the SQL + # database. # # ==== Parameters # @@ -319,6 +320,12 @@ module ActiveRecord # # Update all customers with the given attributes # Customer.update_all wants_email: true # + # # Update all active accounts with the given attributes + # class Account < ActiveRecord::Base + # default_scope -> { where active: true } + # end + # Account.update_all(failed_logins: 0) + # # # Update all books with 'Rails' in their title # Book.where('title LIKE ?', '%Rails%').update_all(author: 'David') # -- cgit v1.2.3 From d56be864f61cc00eeff27ed42e7cec4194c5347f Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Thu, 20 Nov 2014 15:30:46 +0100 Subject: synchronize code and docs for `timestamps` and `add_timestamps`. This makes the following changes: * warn if `:null` is not passed to `add_timestamps` * `timestamps` method docs link to `add_timestamps` docs * explain where additional options go * adjust examples to include `null: false` (to prevent deprecation warnings) --- .../connection_adapters/abstract/schema_definitions.rb | 11 +++++------ .../connection_adapters/abstract/schema_statements.rb | 7 +++++-- .../lib/active_record/connection_adapters/abstract_adapter.rb | 1 + 3 files changed, 11 insertions(+), 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 5c4c214385..d2236d046b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -226,7 +226,7 @@ module ActiveRecord # t.integer :shop_id, :creator_id # t.string :item_number, index: true # t.string :name, :value, default: "Untitled" - # t.timestamps + # t.timestamps null: false # end # # There's a short-hand method for each of the type values declared at the top. And then there's @@ -287,7 +287,9 @@ module ActiveRecord end # Appends :datetime columns :created_at and - # :updated_at to the table. + # :updated_at to the table. See SchemaStatements#add_timestamps + # + # t.timestamps null: false def timestamps(*args) options = args.extract_options! emit_warning_if_null_unspecified(options) @@ -412,8 +414,6 @@ module ActiveRecord # end # class Table - include TimestampDefaultDeprecation - attr_reader :name def initialize(table_name, base) @@ -462,9 +462,8 @@ module ActiveRecord # Adds timestamps (+created_at+ and +updated_at+) columns to the table. See SchemaStatements#add_timestamps # - # t.timestamps + # t.timestamps null: false def timestamps(options = {}) - emit_warning_if_null_unspecified(options) @base.add_timestamps(name, options) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index cc86c3776e..7cc5e666af 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -835,11 +835,14 @@ module ActiveRecord columns end - # Adds timestamps (+created_at+ and +updated_at+) columns to the named table. + include TimestampDefaultDeprecation + # Adds timestamps (+created_at+ and +updated_at+) columns to +table_name+. + # Additional options (like null: false) are forwarded to #add_column. # - # add_timestamps(:suppliers) + # add_timestamps(:suppliers, null: false) # def add_timestamps(table_name, options = {}) + emit_warning_if_null_unspecified(options) add_column table_name, :created_at, :datetime, options add_column table_name, :updated_at, :datetime, options end diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 57aa2f9c58..fa3483a258 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -24,6 +24,7 @@ module ActiveRecord autoload :TableDefinition autoload :Table autoload :AlterTable + autoload :TimestampDefaultDeprecation end autoload_at 'active_record/connection_adapters/abstract/connection_pool' do -- cgit v1.2.3 From 5aedabe82d3a9d7df19007aa0fd7719d4a55ef7e Mon Sep 17 00:00:00 2001 From: claudiob Date: Thu, 20 Nov 2014 14:54:22 -0800 Subject: Wrap code snippets in +, not backticks, in sdoc I grepped the source code for code snippets wrapped in backticks in the comments and replaced the backticks with plus signs so they are correctly displayed in the Rails documentation. [ci skip] --- activerecord/lib/active_record/associations/collection_association.rb | 4 ++-- .../lib/active_record/connection_adapters/connection_specification.rb | 2 +- activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 2 +- .../lib/active_record/connection_adapters/postgresql_adapter.rb | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 93f611dd8d..d59cebb08b 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -175,8 +175,8 @@ module ActiveRecord end # Removes all records from the association without calling callbacks - # on the associated records. It honors the `:dependent` option. However - # if the `:dependent` value is `:destroy` then in that case the `:delete_all` + # on the associated records. It honors the +:dependent+ option. However + # if the +:dependent+ value is +:destroy+ then in that case the +:delete_all+ # deletion strategy for the association is applied. # # You can force a particular deletion strategy by passing a parameter. diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 609ec7dabd..e54e3199ff 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -234,7 +234,7 @@ module ActiveRecord end end - # Takes the environment such as `:production` or `:development`. + # Takes the environment such as +:production+ or +:development+. # This requires that the @configurations was initialized with a key that # matches. # diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 3357ed52e5..8b7459ef27 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -58,7 +58,7 @@ module ActiveRecord # * :encoding - (Optional) Sets the client encoding by executing "SET NAMES " after connection. # * :reconnect - Defaults to false (See MySQL documentation: http://dev.mysql.com/doc/refman/5.0/en/auto-reconnect.html). # * :strict - Defaults to true. Enable STRICT_ALL_TABLES. (See MySQL documentation: http://dev.mysql.com/doc/refman/5.0/en/server-sql-mode.html) - # * :variables - (Optional) A hash session variables to send as `SET @@SESSION.key = value` on each database connection. Use the value `:default` to set a variable to its DEFAULT value. (See MySQL documentation: http://dev.mysql.com/doc/refman/5.0/en/set-statement.html). + # * :variables - (Optional) A hash session variables to send as SET @@SESSION.key = value on each database connection. Use the value +:default+ to set a variable to its DEFAULT value. (See MySQL documentation: http://dev.mysql.com/doc/refman/5.0/en/set-statement.html). # * :sslca - Necessary to use MySQL with an SSL connection. # * :sslkey - Necessary to use MySQL with an SSL connection. # * :sslcert - Necessary to use MySQL with an SSL connection. diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 6310d70192..3698957d8d 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -125,7 +125,7 @@ module ActiveRecord PostgreSQL::SchemaCreation.new self end - # Adds `:array` option to the default set provided by the + # Adds +:array+ option to the default set provided by the # AbstractAdapter def prepare_column_options(column, types) # :nodoc: spec = super @@ -134,7 +134,7 @@ module ActiveRecord spec end - # Adds `:array` as a valid migration key + # Adds +:array+ as a valid migration key def migration_keys super + [:array] end -- cgit v1.2.3 From 3e92806357543cdbabad813884a591fca69193c2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 20 Nov 2014 17:37:54 -0800 Subject: raise a better exception for renaming long indexes --- .../active_record/connection_adapters/abstract/schema_statements.rb | 3 +++ .../active_record/connection_adapters/postgresql/schema_statements.rb | 3 +++ 2 files changed, 6 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 7cc5e666af..99fa9b7d29 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -571,6 +571,9 @@ module ActiveRecord # rename_index :people, 'index_people_on_last_name', 'index_users_on_last_name' # def rename_index(table_name, old_name, new_name) + if new_name.length > allowed_index_name_length + raise ArgumentError, "Index name '#{new_name}' on table '#{table_name}' is too long; the limit is #{allowed_index_name_length} characters" + end # this is a naive implementation; some DBs may support this more efficiently (Postgres, for instance) old_index_def = indexes(table_name).detect { |i| i.name == old_name } return unless old_index_def diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 51853c6812..b4b066c06e 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -478,6 +478,9 @@ module ActiveRecord end def rename_index(table_name, old_name, new_name) + if new_name.length > allowed_index_name_length + raise ArgumentError, "Index name '#{new_name}' on table '#{table_name}' is too long; the limit is #{allowed_index_name_length} characters" + end execute "ALTER INDEX #{quote_column_name(old_name)} RENAME TO #{quote_table_name(new_name)}" end -- cgit v1.2.3 From 17efb3b919e8eff1ca5da49b12e3c1f607eb93e3 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 20 Nov 2014 19:17:10 -0700 Subject: Remove is_a? check when ignoring tables Technically changes the API, as it will allow any object which responds to `===`. Personally, I think this is more flexible. --- activerecord/lib/active_record/schema_dumper.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 261fb9d992..1dd2cadc01 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -244,12 +244,7 @@ HEADER def ignored?(table_name) ['schema_migrations', ignore_tables].flatten.any? do |ignored| - case ignored - when String; remove_prefix_and_suffix(table_name) == ignored - when Regexp; remove_prefix_and_suffix(table_name) =~ ignored - else - raise StandardError, 'ActiveRecord::SchemaDumper.ignore_tables accepts an array of String and / or Regexp values.' - end + ignored === remove_prefix_and_suffix(table_name) end end end -- cgit v1.2.3 From f766abd4cf3eb75469d3646cfb6d85e668c619f3 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Fri, 21 Nov 2014 14:57:25 +0100 Subject: make it possible to access fixtures excluded by a `default_scope`. Prior to this patch you'd end up with an error like: ``` ActiveRecord::RecordNotFound: Couldn't find with 'id'= [WHERE ()] ``` --- activerecord/lib/active_record/fixtures.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index db6421dacb..660ba18a98 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -806,7 +806,7 @@ module ActiveRecord def find if model_class - model_class.find(fixture[model_class.primary_key]) + model_class.unscoped.find(fixture[model_class.primary_key]) else raise FixtureClassNotFound, "No class attached to find." end -- cgit v1.2.3 From 91e3dab804fffe0b1daebb438091418faa1fa256 Mon Sep 17 00:00:00 2001 From: "siddharth@vinsol.com" Date: Thu, 20 Nov 2014 01:35:04 +0530 Subject: Fix includes on association with a scope containing joins along with conditions on the joined assoiciation --- .../lib/active_record/associations/preloader/association.rb | 10 ++-------- activerecord/lib/active_record/relation/query_methods.rb | 6 ++---- 2 files changed, 4 insertions(+), 12 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb index 496c426986..7d6523dbc4 100644 --- a/activerecord/lib/active_record/associations/preloader/association.rb +++ b/activerecord/lib/active_record/associations/preloader/association.rb @@ -142,14 +142,8 @@ module ActiveRecord scope._select! preload_values[:select] || values[:select] || table[Arel.star] scope.includes! preload_values[:includes] || values[:includes] - - if preload_values.key? :order - scope.order! preload_values[:order] - else - if values.key? :order - scope.order! values[:order] - end - end + scope.joins! preload_values[:joins] || values[:joins] + scope.order! preload_values[:order] || values[:order] if preload_values[:readonly] || values[:readonly] scope.readonly! diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index eb69943551..6e384facce 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -427,14 +427,12 @@ module ActiveRecord # => SELECT "users".* FROM "users" LEFT JOIN bookmarks ON bookmarks.bookmarkable_type = 'Post' AND bookmarks.user_id = users.id def joins(*args) check_if_method_has_arguments!(:joins, args) - - args.compact! - args.flatten! - spawn.joins!(*args) end def joins!(*args) # :nodoc: + args.compact! + args.flatten! self.joins_values += args self end -- cgit v1.2.3 From 71bb8c34f539f8ffbb7e7968f50f797e08812133 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Fri, 21 Nov 2014 15:33:46 -0700 Subject: ConnectionAdapter#substitute_at is technically public API... We can't change the signature without a deprecation cycle. --- activerecord/lib/active_record/connection_adapters/abstract_adapter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index fa3483a258..e0a4af5359 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -265,7 +265,7 @@ module ActiveRecord # Returns a bind substitution value given a bind +column+ # NOTE: The column param is currently being used by the sqlserver-adapter - def substitute_at(column) + def substitute_at(column, _unused = 0) Arel::Nodes::BindParam.new end -- cgit v1.2.3 From dcc143cd702bfbfef9cb92e1fde75171d088a5ac Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sat, 22 Nov 2014 13:16:14 -0700 Subject: Rename the primary key index when renaming a table in pg Also checked to make sure this does not affect foreign key constraints. (It doesn't). Fixes #12856 Closes #14088 --- .../active_record/connection_adapters/postgresql/schema_statements.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index b4b066c06e..515ff2dd44 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -411,7 +411,10 @@ module ActiveRecord pk, seq = pk_and_sequence_for(new_name) if seq && seq.identifier == "#{table_name}_#{pk}_seq" new_seq = "#{new_name}_#{pk}_seq" + idx = "#{table_name}_pkey" + new_idx = "#{new_name}_pkey" execute "ALTER TABLE #{quote_table_name(seq)} RENAME TO #{quote_table_name(new_seq)}" + execute "ALTER INDEX #{quote_table_name(idx)} RENAME TO #{quote_table_name(new_idx)}" end rename_table_indexes(table_name, new_name) -- cgit v1.2.3 From 6cd9e2bd29b26e1ea781fafce18614217b7adf7b Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sat, 22 Nov 2014 15:36:50 -0700 Subject: Fix build failures For some reason changing `.find` to `.unscoped.find` in https://github.com/rails/rails/commit/f766abd4cf3eb75469d3646cfb6d85e668c619f3 caused `scoping` to leak in some tests when run in isolation (looks like a concurrency issue?). `relation_scoping_test.rb` is a case that failed. From what I can tell it should not be possible, but changing to the block form fixes it. There is a deeper issue that I can't seem to find. /cc @senny --- activerecord/lib/active_record/fixtures.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 660ba18a98..7c53ee0a9a 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -806,7 +806,9 @@ module ActiveRecord def find if model_class - model_class.unscoped.find(fixture[model_class.primary_key]) + model_class.unscoped do + model_class.find(fixture[model_class.primary_key]) + end else raise FixtureClassNotFound, "No class attached to find." end -- cgit v1.2.3 From 31b3069615529eabc42cc5bee1cd76e52bd1560b Mon Sep 17 00:00:00 2001 From: Arthur Neves Date: Sat, 22 Nov 2014 20:33:00 -0500 Subject: Update reflections public API doc --- activerecord/lib/active_record/reflection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index f4a351b092..4219632596 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -63,7 +63,7 @@ module ActiveRecord # Returns a Hash of name of the reflection as the key and a AssociationReflection as the value. # - # Account.reflections # => {balance: AggregateReflection} + # Account.reflections # => {"balance" => AggregateReflection} # # @api public def reflections -- cgit v1.2.3 From 77fbc5358616c131884df4e0b622241ebf2a129b Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 24 Nov 2014 12:14:47 +0100 Subject: cleanup, remove trailing whitespace [ci skip] --- activerecord/lib/active_record/migration.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 2024b225e4..4e00eb3d99 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -184,8 +184,8 @@ module ActiveRecord # you wish to downgrade. Alternatively, you can also use the STEP option if you # wish to rollback last few migrations. rake db:migrate STEP=2 will rollback # the latest two migrations. - # - # If any of the migrations throw an ActiveRecord::IrreversibleMigration exception, + # + # If any of the migrations throw an ActiveRecord::IrreversibleMigration exception, # that step will fail and you'll have some manual work to do. # # == Database support -- cgit v1.2.3 From 91c0c277698ab6ca4132a580f4212aa913492a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20van=20den=20Berg?= Date: Mon, 24 Nov 2014 16:03:07 +0100 Subject: Reword documentation for update_all It now contains a carefully formulated reference to the "current relation" which might help clarify that the receiving will generate its own scope, escaping the need for explicitly referencing `default_scope` which is, after all, just another way of specifying a scope and nothing special. --- activerecord/lib/active_record/relation.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index fc3306ee81..561ed222d1 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -304,8 +304,7 @@ module ActiveRecord klass.current_scope = previous end - # Updates all records in the current scope (respecting the default_scope, where, - # limit and order specified) with details given. This method constructs a single SQL update_all + # Updates all records in the current relation with details given. This method constructs a single SQL UPDATE # statement and sends it straight to the database. It does not instantiate the involved models and it does not # trigger Active Record callbacks or validations. Values passed to `update_all` will not go through # ActiveRecord's type-casting behavior. It should receive only values that can be passed as-is to the SQL @@ -320,12 +319,6 @@ module ActiveRecord # # Update all customers with the given attributes # Customer.update_all wants_email: true # - # # Update all active accounts with the given attributes - # class Account < ActiveRecord::Base - # default_scope -> { where active: true } - # end - # Account.update_all(failed_logins: 0) - # # # Update all books with 'Rails' in their title # Book.where('title LIKE ?', '%Rails%').update_all(author: 'David') # -- cgit v1.2.3 From 72e8442797b90d865811e39418ef9d49238b193a Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 24 Nov 2014 11:24:31 -0700 Subject: Ensure the type map's cache is thread safe Thanks to @thedarkone for pointing out that an instance of this object is used in a shared context. --- activerecord/lib/active_record/type/type_map.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/type_map.rb b/activerecord/lib/active_record/type/type_map.rb index 7c194c0cdf..09f5ba6b74 100644 --- a/activerecord/lib/active_record/type/type_map.rb +++ b/activerecord/lib/active_record/type/type_map.rb @@ -1,10 +1,12 @@ +require 'thread_safe' + module ActiveRecord module Type class TypeMap # :nodoc: def initialize @mapping = {} - @cache = Hash.new do |h, key| - h[key] = {} + @cache = ThreadSafe::Cache.new do |h, key| + h.fetch_or_store(key, ThreadSafe::Cache.new) end end @@ -13,7 +15,9 @@ module ActiveRecord end def fetch(lookup_key, *args, &block) - @cache[lookup_key][args] ||= perform_fetch(lookup_key, *args, &block) + @cache[lookup_key].fetch_or_store(args) do + perform_fetch(lookup_key, *args, &block) + end end def register_type(key, value = nil, &block) -- cgit v1.2.3 From 83821e2c4a32b17b153fcbcb247dec6e32f47284 Mon Sep 17 00:00:00 2001 From: claudiob Date: Mon, 24 Nov 2014 10:27:58 -0800 Subject: Mark comments that should not be in the docs Some comments that are meant to separate blocks of code in a file show up on http://api.rubyonrails.org as though they were part of the documentation. This commit hides those comments from the documentation. Stems from the discussion with @zzak at https://github.com/voloko/sdoc/issues/79#issuecomment-64158738 [ci skip] --- .../lib/active_record/connection_adapters/abstract_mysql_adapter.rb | 2 ++ .../lib/active_record/connection_adapters/mysql2_adapter.rb | 6 ++++++ activerecord/lib/active_record/connection_adapters/mysql_adapter.rb | 4 ++++ .../lib/active_record/connection_adapters/sqlite3_adapter.rb | 2 ++ 4 files changed, 14 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 167453657d..c824a7b11b 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -285,7 +285,9 @@ module ActiveRecord end end + #-- # DATABASE STATEMENTS ====================================== + #++ def clear_cache! super diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 5b83131f0e..d19bd80576 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -66,7 +66,9 @@ module ActiveRecord exception.error_number if exception.respond_to?(:error_number) end + #-- # QUOTING ================================================== + #++ def quote_string(string) @connection.escape(string) @@ -80,7 +82,9 @@ module ActiveRecord end end + #-- # CONNECTION MANAGEMENT ==================================== + #++ def active? return false unless @connection @@ -104,7 +108,9 @@ module ActiveRecord end end + #-- # DATABASE STATEMENTS ====================================== + #++ def explain(arel, binds = []) sql = "EXPLAIN #{to_sql(arel, binds.dup)}" diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 8b7459ef27..53ad71b445 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -137,7 +137,9 @@ module ActiveRecord @connection.quote(string) end + #-- # CONNECTION MANAGEMENT ==================================== + #++ def active? if @connection.respond_to?(:stat) @@ -178,7 +180,9 @@ module ActiveRecord end end + #-- # DATABASE STATEMENTS ====================================== + #++ def select_rows(sql, name = nil, binds = []) @connection.query_with_result = true diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index b18cb353f1..0b8b6a2bf8 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -266,7 +266,9 @@ module ActiveRecord end end + #-- # DATABASE STATEMENTS ====================================== + #++ def explain(arel, binds = []) sql = "EXPLAIN QUERY PLAN #{to_sql(arel, binds)}" -- cgit v1.2.3 From fbef981fdc7ba64678f7ae1fc82b2cd790469280 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 24 Nov 2014 14:15:45 -0800 Subject: allow the "USING" statement to be specified on change column calls --- .../active_record/connection_adapters/postgresql/schema_statements.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 515ff2dd44..208302b365 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -433,7 +433,9 @@ module ActiveRecord quoted_table_name = quote_table_name(table_name) sql_type = type_to_sql(type, options[:limit], options[:precision], options[:scale]) sql_type << "[]" if options[:array] - execute "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{sql_type}" + sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{sql_type}" + sql << " USING #{options[:using]}" if options[:using] + execute sql change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) -- cgit v1.2.3 From cdd90f39d796986dabf1678b3277b230dbe18961 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 24 Nov 2014 14:29:04 -0800 Subject: allow types to be passed in for USING casts This allows us so abstract the migration from the type that is actually used by Rails. For example, ":string" may be a varchar or something, but the framework does that translation, and the app shouldn't need to know. --- .../active_record/connection_adapters/postgresql/schema_statements.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 208302b365..193c950261 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -435,6 +435,9 @@ module ActiveRecord sql_type << "[]" if options[:array] sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quote_column_name(column_name)} TYPE #{sql_type}" sql << " USING #{options[:using]}" if options[:using] + if options[:cast_as] + sql << " USING CAST(#{quote_column_name(column_name)} AS #{type_to_sql(options[:cast_as], options[:limit], options[:precision], options[:scale])})" + end execute sql change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) -- cgit v1.2.3 From 9e7037f19865021030893880b16be7b1ecbd3470 Mon Sep 17 00:00:00 2001 From: eileencodes Date: Mon, 24 Nov 2014 14:56:27 -0500 Subject: Combine aliased_table_for and aliased_name_for This refactoring reduces the number of conditionals needed to build `aliased_table_for` and removes `aliased_name_for` because it's no longer necessary. `aliased_name_for` was also used in `JoinDependency#initialize` so that was replaced with `aliased_table_for` as well. --- .../lib/active_record/associations/alias_tracker.rb | 15 +++------------ .../lib/active_record/associations/join_dependency.rb | 2 +- 2 files changed, 4 insertions(+), 13 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/alias_tracker.rb b/activerecord/lib/active_record/associations/alias_tracker.rb index a6a1947148..0c3234ed24 100644 --- a/activerecord/lib/active_record/associations/alias_tracker.rb +++ b/activerecord/lib/active_record/associations/alias_tracker.rb @@ -57,20 +57,10 @@ module ActiveRecord end def aliased_table_for(table_name, aliased_name) - table_alias = aliased_name_for(table_name, aliased_name) - - if table_alias == table_name - Arel::Table.new(table_name) - else - Arel::Table.new(table_name).alias(table_alias) - end - end - - def aliased_name_for(table_name, aliased_name) if aliases[table_name].zero? # If it's zero, we can have our table_name aliases[table_name] = 1 - table_name + Arel::Table.new(table_name) else # Otherwise, we need to use an alias aliased_name = connection.table_alias_for(aliased_name) @@ -78,11 +68,12 @@ module ActiveRecord # Update the count aliases[aliased_name] += 1 - if aliases[aliased_name] > 1 + table_alias = if aliases[aliased_name] > 1 "#{truncate(aliased_name)}_#{aliases[aliased_name]}" else aliased_name end + Arel::Table.new(table_name).alias(table_alias) end end diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index c5c4edd090..285d0ec9af 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -94,7 +94,7 @@ module ActiveRecord # def initialize(base, associations, joins) @alias_tracker = AliasTracker.create(base.connection, joins) - @alias_tracker.aliased_name_for(base.table_name, base.table_name) # Updates the count for base.table_name to 1 + @alias_tracker.aliased_table_for(base.table_name, base.table_name) # Updates the count for base.table_name to 1 tree = self.class.make_tree associations @join_root = JoinBase.new base, build(tree, base) @join_root.children.each { |child| construct_tables! @join_root, child } -- cgit v1.2.3 From 03c1055b74a8689a7253bbcb1ec87ae0c44a9d92 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 25 Nov 2014 19:48:09 +0900 Subject: Fix out of range error message --- activerecord/lib/active_record/type/integer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/type/integer.rb b/activerecord/lib/active_record/type/integer.rb index 36bbd9cd5e..750f353472 100644 --- a/activerecord/lib/active_record/type/integer.rb +++ b/activerecord/lib/active_record/type/integer.rb @@ -38,7 +38,7 @@ module ActiveRecord def ensure_in_range(value) unless range.cover?(value) - raise RangeError, "#{value} is too large for #{self.class} with limit #{limit || 4}" + raise RangeError, "#{value} is out of range for #{self.class} with limit #{limit || 4}" end end -- cgit v1.2.3 From 9e9793b440c044b765f2d1f702feeb92aef2b139 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Tue, 25 Nov 2014 18:11:38 +0100 Subject: do not trigger AR lazy load hook before initializers ran. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [Rafael Mendonça França & Yves Senn] This require caused the `active_record.set_configs` initializer to run immediately, before `config/initializers`. This means that setting any configuration on `Rails.application.config.active_record` inside of an initializer had no effects when rails was loaded through `rake`. Introduced by #6518 /cc @rafaelfranca --- activerecord/lib/active_record/railtie.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index a4ceacbf44..f1bdbc845c 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -36,8 +36,6 @@ module ActiveRecord config.eager_load_namespaces << ActiveRecord rake_tasks do - require "active_record/base" - namespace :db do task :load_config do ActiveRecord::Tasks::DatabaseTasks.database_configuration = Rails.application.config.database_configuration -- cgit v1.2.3 From 5c4495538bb4e8808ce077f08ea6b809952daa44 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 24 Nov 2014 12:04:01 +0100 Subject: bring back `db:test:prepare`. This reverts deprecations added in #13528. The task is brought back for two reasons: 1. Give plugins a way to hook into the test database initialization process 2. Give the user a way to force a test database synchronization While `test:prepare` is still a dependency of every test task, `db:test:prepare` no longer hooks into it. This means that `test:prepare` runs before the schema is synchronized. Plugins, which insert data can now hook into `db:test:prepare`. The automatic schema maintenance can't detect when a migration is rolled-back, modified and reapplied. In this case the user has to fall back to `db:test:prepare` to force the synchronization to happen. --- activerecord/lib/active_record/migration.rb | 9 ++++++++- activerecord/lib/active_record/railties/databases.rake | 10 +++++----- 2 files changed, 13 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 4e00eb3d99..92f2951f2d 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -395,7 +395,14 @@ module ActiveRecord def load_schema_if_pending! if ActiveRecord::Migrator.needs_migration? || !ActiveRecord::Migrator.any_migrations? - ActiveRecord::Tasks::DatabaseTasks.load_schema_current_if_exists + # Roundrip to Rake to allow plugins to hook into database initialization. + FileUtils.cd Rails.root do + current_config = Base.connection_config + Base.clear_all_connections! + system("bin/rake db:test:prepare") + # Establish a new connection, the old database may be gone (db:test:prepare uses purge) + Base.establish_connection(current_config) + end check_pending! end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 3ec25f9f17..21b1c3f721 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -305,7 +305,7 @@ db_namespace = namespace :db do end # desc "Recreate the test database from the current schema" - task :load => %w(db:test:deprecated db:test:purge) do + task :load => %w(db:test:purge) do case ActiveRecord::Base.schema_format when :ruby db_namespace["test:load_schema"].invoke @@ -315,7 +315,7 @@ db_namespace = namespace :db do end # desc "Recreate the test database from an existent schema.rb file" - task :load_schema => %w(db:test:deprecated db:test:purge) do + task :load_schema => %w(db:test:purge) do begin should_reconnect = ActiveRecord::Base.connection_pool.active_connection? ActiveRecord::Schema.verbose = false @@ -328,7 +328,7 @@ db_namespace = namespace :db do end # desc "Recreate the test database from an existent structure.sql file" - task :load_structure => %w(db:test:deprecated db:test:purge) do + task :load_structure => %w(db:test:purge) do ActiveRecord::Tasks::DatabaseTasks.load_schema_for ActiveRecord::Base.configurations['test'], :sql, ENV['SCHEMA'] end @@ -349,12 +349,12 @@ db_namespace = namespace :db do task :clone_structure => %w(db:test:deprecated db:structure:dump db:test:load_structure) # desc "Empty the test database" - task :purge => %w(db:test:deprecated environment load_config) do + task :purge => %w(environment load_config) do ActiveRecord::Tasks::DatabaseTasks.purge ActiveRecord::Base.configurations['test'] end # desc 'Check for pending migrations and load the test schema' - task :prepare => %w(db:test:deprecated environment load_config) do + task :prepare => %w(environment load_config) do unless ActiveRecord::Base.configurations.blank? db_namespace['test:load'].invoke end -- cgit v1.2.3 From dd986814e7f528916a44f841a02c955a4522307c Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 25 Nov 2014 10:52:33 -0700 Subject: Setting an association replaces records with the same id in memory The records weren't being replaced since equality in Active Record is defined in terms of `id` only. It is reasonable to expect that the references would be replaced in memory, even if no queries are actually executed. This change did not appear to affect any other parts of the code base. I chose not to execute callbacks since we're not actually modifying the association in a way that will be persisted. Fixes #17730 --- .../associations/collection_association.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index d59cebb08b..2b7d39893d 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -358,6 +358,7 @@ module ActiveRecord if owner.new_record? replace_records(other_array, original_target) else + replace_common_records_in_memory(other_array, original_target) if other_array != original_target transaction { replace_records(other_array, original_target) } end @@ -385,11 +386,18 @@ module ActiveRecord target end - def add_to_target(record, skip_callbacks = false) + def add_to_target(record, skip_callbacks = false, &block) + if association_scope.distinct_value + index = @target.index(record) + end + replace_on_target(record, index, skip_callbacks, &block) + end + + def replace_on_target(record, index, skip_callbacks) callback(:before_add, record) unless skip_callbacks yield(record) if block_given? - if association_scope.distinct_value && index = @target.index(record) + if index @target[index] = record else @target << record @@ -534,6 +542,14 @@ module ActiveRecord target end + def replace_common_records_in_memory(new_target, original_target) + common_records = new_target & original_target + common_records.each do |record| + skip_callbacks = true + replace_on_target(record, @target.index(record), skip_callbacks) + end + end + def concat_records(records, should_raise = false) result = true -- cgit v1.2.3 From d74e716b48b19a608a0445996cfcf17d304f289f Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 25 Nov 2014 13:57:06 -0700 Subject: Move PG float quoting to the correct location Not sure how we missed this case when we moved everything else to the `_quote` method. --- .../connection_adapters/postgresql/quoting.rb | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index f95f45c689..991c41327f 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -14,22 +14,6 @@ module ActiveRecord @connection.unescape_bytea(value) if value end - # Quotes PostgreSQL-specific data types for SQL input. - def quote(value, column = nil) #:nodoc: - return super unless column - - case value - when Float - if value.infinite? || value.nan? - "'#{value}'" - else - super - end - else - super - end - end - # Quotes strings for use in SQL input. def quote_string(s) #:nodoc: @connection.escape(s) @@ -94,6 +78,12 @@ module ActiveRecord elsif value.hex? "X'#{value}'" end + when Float + if value.infinite? || value.nan? + "'#{value}'" + else + super + end else super end -- cgit v1.2.3 From 323334a775bb35d901742e4cc39f6fe7b5e53c8f Mon Sep 17 00:00:00 2001 From: Derek Prior Date: Mon, 24 Nov 2014 21:25:15 -0500 Subject: Generators add foreign keys on references If you run a generator such as: ``` rails generate model accounts supplier:references ``` The resulting migration will now add the corresponding foreign key constraint unless the reference was specified to be polymorphic. --- .../active_record/migration/templates/create_table_migration.rb | 3 +++ .../rails/generators/active_record/migration/templates/migration.rb | 6 ++++++ 2 files changed, 9 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb index f7bf6987c4..fb0fbb4759 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/create_table_migration.rb @@ -14,6 +14,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration end <% attributes_with_index.each do |attribute| -%> add_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> +<% end -%> +<% attributes.select(&:reference?).reject(&:polymorphic?).each do |attribute| -%> + add_foreign_key :<%= table_name %>, :<%= attribute.name.pluralize %> <% end -%> end end diff --git a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb index ae9c74fd05..7df9bcad25 100644 --- a/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb +++ b/activerecord/lib/rails/generators/active_record/migration/templates/migration.rb @@ -4,6 +4,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration <% attributes.each do |attribute| -%> <%- if attribute.reference? -%> add_reference :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_options %> + <%- unless attribute.polymorphic? -%> + add_foreign_key :<%= table_name %>, :<%= attribute.name.pluralize %> + <%- end -%> <%- else -%> add_column :<%= table_name %>, :<%= attribute.name %>, :<%= attribute.type %><%= attribute.inject_options %> <%- if attribute.has_index? -%> @@ -26,6 +29,9 @@ class <%= migration_class_name %> < ActiveRecord::Migration <%- if migration_action -%> <%- if attribute.reference? -%> remove_reference :<%= table_name %>, :<%= attribute.name %><%= attribute.inject_options %> + <%- unless attribute.polymorphic? -%> + remove_foreign_key :<%= table_name %>, :<%= attribute.name.pluralize %> + <%- end -%> <%- else -%> <%- if attribute.has_index? -%> remove_index :<%= table_name %>, :<%= attribute.index_name %><%= attribute.inject_index_options %> -- cgit v1.2.3 From 3e30c5d4222f816cdffd5241d56ef9442f6d1903 Mon Sep 17 00:00:00 2001 From: Recursive Madman Date: Wed, 19 Nov 2014 12:34:25 +0100 Subject: Add #record attribute to RecordNotFound and RecordDestroyed exceptions. This allows these exceptions to be handled generically in conjunction with RecordInvalid. --- activerecord/lib/active_record/errors.rb | 19 +++++++++++++++++++ activerecord/lib/active_record/persistence.rb | 4 ++-- 2 files changed, 21 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 5b3fdf16f5..ca4fede7a2 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -52,10 +52,29 @@ module ActiveRecord # Raised by ActiveRecord::Base.save! and ActiveRecord::Base.create! methods when record cannot be # saved because record is invalid. class RecordNotSaved < ActiveRecordError + attr_reader :record + + def initialize(record) + @record = record + super() + end end # Raised by ActiveRecord::Base.destroy! when a call to destroy would return false. + # + # begin + # complex_operation_that_internally_calls_destroy! + # rescue ActiveRecord::RecordNotDestroyed => invalid + # puts invalid.record.errors + # end + # class RecordNotDestroyed < ActiveRecordError + attr_reader :record + + def initialize(record) + @record = record + super() + end end # Superclass for all database execution errors. diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index ee3b7b6163..8d84a3acae 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -139,7 +139,7 @@ module ActiveRecord # Attributes marked as readonly are silently ignored if the record is # being updated. def save!(*) - create_or_update || raise(RecordNotSaved) + create_or_update || raise(RecordNotSaved, self) end # Deletes the record in the database and freezes this instance to @@ -181,7 +181,7 @@ module ActiveRecord # and destroy! raises ActiveRecord::RecordNotDestroyed. See # ActiveRecord::Callbacks for further details. def destroy! - destroy || raise(ActiveRecord::RecordNotDestroyed) + destroy || raise(ActiveRecord::RecordNotDestroyed, self) end # Returns an instance of the specified +klass+ with the attributes of the -- cgit v1.2.3 From 59190c037999d302dd95b1f53f7049c2cec3f7b9 Mon Sep 17 00:00:00 2001 From: Recursive Madman Date: Wed, 26 Nov 2014 15:05:34 +0100 Subject: various error classes: added newlines & removed :nodoc: flag from public attribute. --- activerecord/lib/active_record/errors.rb | 2 ++ activerecord/lib/active_record/validations.rb | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index ca4fede7a2..14819aab54 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -196,6 +196,7 @@ module ActiveRecord # offending attribute. class AttributeAssignmentError < ActiveRecordError attr_reader :exception, :attribute + def initialize(message, exception, attribute) super(message) @exception = exception @@ -208,6 +209,7 @@ module ActiveRecord # objects, each corresponding to the error while assigning to an attribute. class MultiparameterAssignmentErrors < ActiveRecordError attr_reader :errors + def initialize(errors) @errors = errors end diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb index 7f7d49cdb4..a6c8ff7f3a 100644 --- a/activerecord/lib/active_record/validations.rb +++ b/activerecord/lib/active_record/validations.rb @@ -5,13 +5,14 @@ module ActiveRecord # +record+ method to retrieve the record which did not validate. # # begin - # complex_operation_that_calls_save!_internally + # complex_operation_that_internally_calls_save! # rescue ActiveRecord::RecordInvalid => invalid # puts invalid.record.errors # end class RecordInvalid < ActiveRecordError - attr_reader :record # :nodoc: - def initialize(record) # :nodoc: + attr_reader :record + + def initialize(record) @record = record errors = @record.errors.full_messages.join(", ") super(I18n.t(:"#{@record.class.i18n_scope}.errors.messages.record_invalid", :errors => errors, :default => :"errors.messages.record_invalid")) -- cgit v1.2.3 From 9e6733d5f7867717c760c1cf4b70c36318c28d67 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 27 Nov 2014 21:27:34 +0900 Subject: Refactor `SchemaCreation#visit_AddColumn` --- .../active_record/connection_adapters/abstract/schema_creation.rb | 4 +--- .../connection_adapters/postgresql/schema_statements.rb | 6 ------ 2 files changed, 1 insertion(+), 9 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index 6bab260f5a..792d49da6f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -15,9 +15,7 @@ module ActiveRecord end def visit_AddColumn(o) - sql_type = type_to_sql(o.type, o.limit, o.precision, o.scale) - sql = "ADD #{quote_column_name(o.name)} #{sql_type}" - add_column_options!(sql, column_options(o)) + "ADD #{accept(o)}" end private diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 193c950261..dba905ed56 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -4,12 +4,6 @@ module ActiveRecord class SchemaCreation < AbstractAdapter::SchemaCreation private - def visit_AddColumn(o) - sql_type = type_to_sql(o.type, o.limit, o.precision, o.scale) - sql = "ADD COLUMN #{quote_column_name(o.name)} #{sql_type}" - add_column_options!(sql, column_options(o)) - end - def visit_ColumnDefinition(o) sql = super if o.primary_key? && o.type != :primary_key -- cgit v1.2.3 From 07f4bd5b60d062bda58a45ca5f832a0a6bcdc713 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 27 Nov 2014 13:50:10 -0200 Subject: StatementCache is private API It should not be used in applications --- activerecord/lib/active_record/statement_cache.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index aece446384..767c7561cf 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -13,10 +13,10 @@ module ActiveRecord # # The relation returned by the block is cached, and for each +execute+ call the cached relation gets duped. # Database is queried when +to_a+ is called on the relation. - class StatementCache - class Substitute; end + class StatementCache # :nodoc: + class Substitute; end # :nodoc: - class Query + class Query # :nodoc: def initialize(sql) @sql = sql end @@ -26,7 +26,7 @@ module ActiveRecord end end - class PartialQuery < Query + class PartialQuery < Query # :nodoc: def initialize values @values = values @indexes = values.each_with_index.find_all { |thing,i| @@ -51,11 +51,11 @@ module ActiveRecord PartialQuery.new collected end - class Params + class Params # :nodoc: def bind; Substitute.new; end end - class BindMap + class BindMap # :nodoc: def initialize(bind_values) @indexes = [] @bind_values = bind_values -- cgit v1.2.3 From 8fc7eb5f21e42f1bc484c496a6431c284b9ee805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 27 Nov 2014 14:40:19 -0200 Subject: Update the StatementCache documentation --- activerecord/lib/active_record/statement_cache.rb | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index 767c7561cf..c68990bf99 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -1,18 +1,29 @@ module ActiveRecord # Statement cache is used to cache a single statement in order to avoid creating the AST again. - # Initializing the cache is done by passing the statement in the initialization block: + # Initializing the cache is done by passing the statement in the create block: # - # cache = ActiveRecord::StatementCache.new do - # Book.where(name: "my book").limit(100) + # cache = StatementCache.create(Book.connection) do |params| + # Book.where(name: "my book").where("author_id > 3") # end # # The cached statement is executed by using the +execute+ method: # - # cache.execute + # cache.execute([], Book, Book.connection) # # The relation returned by the block is cached, and for each +execute+ call the cached relation gets duped. # Database is queried when +to_a+ is called on the relation. + # + # If you want to cache the statement without the values you can use the +bind+ method of the + # block parameter. + # + # cache = StatementCache.create(Book.connection) do |params| + # Book.where(name: params.bind) + # end + # + # And pass the bind values as the first argument of +execute+ call. + # + # cache.execute(["my book"], Book, Book.connection) class StatementCache # :nodoc: class Substitute; end # :nodoc: -- cgit v1.2.3 From 5142d5411481c893f817c1431b0869be3745060f Mon Sep 17 00:00:00 2001 From: Yuki Nishijima Date: Thu, 27 Nov 2014 17:44:36 -0800 Subject: Fix a bug where AR::RecordNotSaved loses error messages Since 3e30c5d, it started ignoring the given error message. This commit changes the behavior of AR::RecordNotSaved#initialize so that it no longer loses the given error message. --- activerecord/lib/active_record/errors.rb | 4 ++-- activerecord/lib/active_record/persistence.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 14819aab54..fc28ab585f 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -54,9 +54,9 @@ module ActiveRecord class RecordNotSaved < ActiveRecordError attr_reader :record - def initialize(record) + def initialize(message, record = nil) @record = record - super() + super(message) end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 8d84a3acae..99e104598b 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -139,7 +139,7 @@ module ActiveRecord # Attributes marked as readonly are silently ignored if the record is # being updated. def save!(*) - create_or_update || raise(RecordNotSaved, self) + create_or_update || raise(RecordNotSaved.new(nil, self)) end # Deletes the record in the database and freezes this instance to -- cgit v1.2.3 From 91fca372b14b553a8029d81213ff51c8f3cde39b Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Thu, 27 Nov 2014 21:56:44 +0900 Subject: Refactor `add_column_options!`, to move the quoting of default value for :uuid in `quote_value`. --- .../connection_adapters/postgresql/schema_statements.rb | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 193c950261..89b7f0a6b7 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -19,14 +19,22 @@ module ActiveRecord sql end + def column_options(o) + column_options = super + column_options[:array] = o.array + column_options + end + def add_column_options!(sql, options) - if options[:array] || options[:column].try(:array) + if options[:array] sql << '[]' end + super + end - column = options.fetch(:column) { return super } - if column.type == :uuid && options[:default] =~ /\(\)/ - sql << " DEFAULT #{options[:default]}" + def quote_value(value, column) + if column.type == :uuid && value =~ /\(\)/ + value else super end -- cgit v1.2.3 From 66cc7ce67c4f48f62377ad1a972581c35bcc8373 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 28 Nov 2014 17:30:55 +0900 Subject: Rename to `quote_default_expression` from `quote_value` --- .../lib/active_record/connection_adapters/abstract/schema_creation.rb | 4 ++-- .../active_record/connection_adapters/postgresql/schema_statements.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb index 6bab260f5a..6a684a33f8 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb @@ -83,7 +83,7 @@ module ActiveRecord end def add_column_options!(sql, options) - sql << " DEFAULT #{quote_value(options[:default], options[:column])}" if options_include_default?(options) + sql << " DEFAULT #{quote_default_expression(options[:default], options[:column])}" if options_include_default?(options) # must explicitly check for :null to allow change_column to work on migrations if options[:null] == false sql << " NOT NULL" @@ -94,7 +94,7 @@ module ActiveRecord sql end - def quote_value(value, column) + def quote_default_expression(value, column) column.sql_type ||= type_to_sql(column.type, column.limit, column.precision, column.scale) column.cast_type ||= type_for_column(column) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index 89b7f0a6b7..009b7cd682 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -32,7 +32,7 @@ module ActiveRecord super end - def quote_value(value, column) + def quote_default_expression(value, column) if column.type == :uuid && value =~ /\(\)/ value else -- cgit v1.2.3 From 6405a03c1303c4c2da7029cf9199bd17451fec4d Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Fri, 28 Nov 2014 13:34:38 +0100 Subject: Adds preloaded_records method to NullPreloader This fixes a regression where preloading association throws an exception if one of the associations in the preloading hash doesn't exist for one record. Fixes #16070 --- activerecord/lib/active_record/associations/preloader.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 46bccbf15a..4358f3b581 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -178,6 +178,7 @@ module ActiveRecord class NullPreloader def self.new(klass, owners, reflection, preload_scope); self; end def self.run(preloader); end + def self.preloaded_records; []; end end def preloader_for(reflection, owners, rhs_klass) -- cgit v1.2.3 From f25ad07f5ade46eb978fa82658463232d0247c65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Fri, 28 Nov 2014 15:00:06 -0200 Subject: Start Rails 5 development :tada: We will support only Ruby >= 2.1. But right now we don't accept pull requests with syntax changes to drop support to Ruby 1.9. --- activerecord/lib/active_record/gem_version.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/gem_version.rb b/activerecord/lib/active_record/gem_version.rb index e820835626..a388b529c9 100644 --- a/activerecord/lib/active_record/gem_version.rb +++ b/activerecord/lib/active_record/gem_version.rb @@ -5,10 +5,10 @@ module ActiveRecord end module VERSION - MAJOR = 4 - MINOR = 2 + MAJOR = 5 + MINOR = 0 TINY = 0 - PRE = "beta4" + PRE = "alpha" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end -- cgit v1.2.3 From d1374f99bf3090f3e659687c112ed0c5c0865cfb Mon Sep 17 00:00:00 2001 From: Erik Michaels-Ober Date: Mon, 27 Oct 2014 17:28:53 +0100 Subject: Pass symbol as an argument instead of a block --- activerecord/lib/active_record/associations.rb | 2 +- .../lib/active_record/associations/collection_association.rb | 10 +++++----- .../active_record/associations/has_many_through_association.rb | 4 +--- activerecord/lib/active_record/associations/join_dependency.rb | 2 +- .../active_record/associations/preloader/has_many_through.rb | 2 +- activerecord/lib/active_record/attribute_assignment.rb | 2 +- activerecord/lib/active_record/autosave_association.rb | 6 +++--- .../connection_adapters/abstract/connection_pool.rb | 4 +--- .../connection_adapters/abstract/schema_statements.rb | 2 +- .../connection_adapters/abstract_mysql_adapter.rb | 4 ++-- .../lib/active_record/connection_adapters/mysql_adapter.rb | 4 +--- .../lib/active_record/connection_adapters/schema_cache.rb | 4 +--- .../lib/active_record/connection_adapters/sqlite3_adapter.rb | 10 +++++----- activerecord/lib/active_record/fixtures.rb | 6 +++--- activerecord/lib/active_record/inheritance.rb | 2 +- activerecord/lib/active_record/migration.rb | 2 +- activerecord/lib/active_record/model_schema.rb | 2 +- activerecord/lib/active_record/railties/databases.rake | 2 +- activerecord/lib/active_record/readonly_attributes.rb | 2 +- activerecord/lib/active_record/reflection.rb | 2 +- activerecord/lib/active_record/relation.rb | 8 ++++---- activerecord/lib/active_record/relation/finder_methods.rb | 2 +- activerecord/lib/active_record/serialization.rb | 2 +- activerecord/lib/active_record/statement_cache.rb | 2 +- activerecord/lib/active_record/validations/presence.rb | 2 +- 25 files changed, 41 insertions(+), 49 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index d2bcdc55bf..eb21712f96 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -57,7 +57,7 @@ module ActiveRecord through_reflection = reflection.through_reflection source_reflection_names = reflection.source_reflection_names source_associations = reflection.through_reflection.klass._reflections.keys - super("Could not find the source association(s) #{source_reflection_names.collect{ |a| a.inspect }.to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)} in model #{through_reflection.klass}. Try 'has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}, :source => '. Is it one of #{source_associations.to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)}?") + super("Could not find the source association(s) #{source_reflection_names.collect(&:inspect).to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)} in model #{through_reflection.klass}. Try 'has_many #{reflection.name.inspect}, :through => #{through_reflection.name.inspect}, :source => '. Is it one of #{source_associations.to_sentence(:two_words_connector => ' or ', :last_word_connector => ', or ', :locale => :en)}?") end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 2b7d39893d..7b6aefe345 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -62,9 +62,9 @@ module ActiveRecord # Implements the ids writer method, e.g. foo.item_ids= for Foo.has_many :items def ids_writer(ids) pk_type = reflection.primary_key_type - ids = Array(ids).reject { |id| id.blank? } + ids = Array(ids).reject(&:blank?) ids.map! { |i| pk_type.type_cast_from_user(i) } - replace(klass.find(ids).index_by { |r| r.id }.values_at(*ids)) + replace(klass.find(ids).index_by(&:id).values_at(*ids)) end def reset @@ -289,7 +289,7 @@ module ActiveRecord elsif !loaded? && !association_scope.group_values.empty? load_target.size elsif !loaded? && !association_scope.distinct_value && target.is_a?(Array) - unsaved_records = target.select { |r| r.new_record? } + unsaved_records = target.select(&:new_record?) unsaved_records.size + count_records else count_records @@ -506,7 +506,7 @@ module ActiveRecord def delete_or_destroy(records, method) records = records.flatten records.each { |record| raise_on_type_mismatch!(record) } - existing_records = records.reject { |r| r.new_record? } + existing_records = records.reject(&:new_record?) if existing_records.empty? remove_records(existing_records, records, method) @@ -609,7 +609,7 @@ module ActiveRecord # specified, then #find scans the entire collection. def find_by_scan(*args) expects_array = args.first.kind_of?(Array) - ids = args.flatten.compact.map{ |arg| arg.to_s }.uniq + ids = args.flatten.compact.map(&:to_s).uniq if ids.size == 1 id = ids.first diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 6329fdfe95..5cfedcc009 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -161,9 +161,7 @@ module ActiveRecord if scope.klass.primary_key count = scope.destroy_all.length else - scope.each do |record| - record._run_destroy_callbacks - end + scope.each(&:_run_destroy_callbacks) arel = scope.arel diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 285d0ec9af..cf63430a97 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -20,7 +20,7 @@ module ActiveRecord end def columns - @tables.flat_map { |t| t.column_aliases } + @tables.flat_map(&:column_aliases) end # An array of [column_name, alias] pairs for the table diff --git a/activerecord/lib/active_record/associations/preloader/has_many_through.rb b/activerecord/lib/active_record/associations/preloader/has_many_through.rb index 7b37b5942d..2029871f39 100644 --- a/activerecord/lib/active_record/associations/preloader/has_many_through.rb +++ b/activerecord/lib/active_record/associations/preloader/has_many_through.rb @@ -8,7 +8,7 @@ module ActiveRecord records_by_owner = super if reflection_scope.distinct_value - records_by_owner.each_value { |records| records.uniq! } + records_by_owner.each_value(&:uniq!) end records_by_owner diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index 2887db3bf7..bf64830417 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -87,7 +87,7 @@ module ActiveRecord end end unless errors.empty? - error_descriptions = errors.map { |ex| ex.message }.join(",") + error_descriptions = errors.map(&:message).join(",") raise MultiparameterAssignmentErrors.new(errors), "#{errors.size} error(s) on assignment of multiparameter attributes [#{error_descriptions}]" end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index a0d70435fa..c39b045a5e 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -263,9 +263,9 @@ module ActiveRecord if new_record association && association.target elsif autosave - association.target.find_all { |record| record.changed_for_autosave? } + association.target.find_all(&:changed_for_autosave?) else - association.target.find_all { |record| record.new_record? } + association.target.find_all(&:new_record?) end end @@ -275,7 +275,7 @@ module ActiveRecord self.class._reflections.values.any? do |reflection| if reflection.options[:autosave] association = association_instance_get(reflection.name) - association && Array.wrap(association.target).any? { |a| a.changed_for_autosave? } + association && Array.wrap(association.target).any?(&:changed_for_autosave?) end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index 46812b75bb..3a9db4a109 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -320,9 +320,7 @@ module ActiveRecord checkin conn conn.disconnect! if conn.requires_reloading? end - @connections.delete_if do |conn| - conn.requires_reloading? - end + @connections.delete_if(&:requires_reloading?) @available.clear @connections.each do |conn| @available.add conn diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 99fa9b7d29..aa7c406050 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -781,7 +781,7 @@ module ActiveRecord version = version.to_i sm_table = quote_table_name(ActiveRecord::Migrator.schema_migrations_table_name) - migrated = select_values("SELECT version FROM #{sm_table}").map { |v| v.to_i } + migrated = select_values("SELECT version FROM #{sm_table}").map(&:to_i) paths = migrations_paths.map {|p| "#{p}/[0-9]*_*.rb" } versions = Dir[*paths].map do |filename| filename.split('/').last.split('_').first.to_i diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index c824a7b11b..bf16804aa0 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -398,7 +398,7 @@ module ActiveRecord sql << "LIKE #{quote(like)}" if like execute_and_free(sql, 'SCHEMA') do |result| - result.collect { |field| field.first } + result.collect(&:first) end end @@ -786,7 +786,7 @@ module ActiveRecord private def version - @version ||= full_version.scan(/^(\d+)\.(\d+)\.(\d+)/).flatten.map { |v| v.to_i } + @version ||= full_version.scan(/^(\d+)\.(\d+)\.(\d+)/).flatten.map(&:to_i) end def mariadb? diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 53ad71b445..23d8389abb 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -423,9 +423,7 @@ module ActiveRecord cols = nil if metadata = stmt.result_metadata - cols = cache[:cols] ||= metadata.fetch_fields.map { |field| - field.name - } + cols = cache[:cols] ||= metadata.fetch_fields.map(&:name) metadata.free end diff --git a/activerecord/lib/active_record/connection_adapters/schema_cache.rb b/activerecord/lib/active_record/connection_adapters/schema_cache.rb index a10ce330c7..37ff4e4613 100644 --- a/activerecord/lib/active_record/connection_adapters/schema_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/schema_cache.rb @@ -61,9 +61,7 @@ module ActiveRecord end def size - [@columns, @columns_hash, @primary_keys, @tables].map { |x| - x.size - }.inject :+ + [@columns, @columns_hash, @primary_keys, @tables].map(&:size).inject :+ end # Clear out internal caches for table with +table_name+. diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 0b8b6a2bf8..c3865a8fdd 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -88,11 +88,11 @@ module ActiveRecord include Comparable def initialize(version_string) - @version = version_string.split('.').map { |v| v.to_i } + @version = version_string.split('.').map(&:to_i) end def <=>(version_string) - @version <=> version_string.split('.').map { |v| v.to_i } + @version <=> version_string.split('.').map(&:to_i) end end @@ -556,7 +556,7 @@ module ActiveRecord end copy_table_indexes(from, to, options[:rename] || {}) copy_table_contents(from, to, - @definition.columns.map {|column| column.name}, + @definition.columns.map(&:name), options[:rename] || {}) end @@ -569,7 +569,7 @@ module ActiveRecord name = name[1..-1] end - to_column_names = columns(to).map { |c| c.name } + to_column_names = columns(to).map(&:name) columns = index.columns.map {|c| rename[c] || c }.select do |column| to_column_names.include?(column) end @@ -586,7 +586,7 @@ module ActiveRecord def copy_table_contents(from, to, columns, rename = {}) #:nodoc: column_mappings = Hash[columns.map {|name| [name, name]}] rename.each { |a| column_mappings[a.last] = a.first } - from_columns = columns(from).collect {|col| col.name} + from_columns = columns(from).collect(&:name) columns = columns.find_all{|col| from_columns.include?(column_mappings[col])} quoted_columns = columns.map { |col| quote_column_name(col) } * ',' diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 7c53ee0a9a..4732462b05 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -745,7 +745,7 @@ module ActiveRecord end def column_names - @column_names ||= @connection.columns(@table_name).collect { |c| c.name } + @column_names ||= @connection.columns(@table_name).collect(&:name) end def read_fixture_files(path, model_class) @@ -868,7 +868,7 @@ module ActiveRecord fixture_set_names = Dir["#{fixture_path}/{**,*}/*.{yml}"] fixture_set_names.map! { |f| f[(fixture_path.to_s.size + 1)..-5] } else - fixture_set_names = fixture_set_names.flatten.map { |n| n.to_s } + fixture_set_names = fixture_set_names.flatten.map(&:to_s) end self.fixture_table_names |= fixture_set_names @@ -908,7 +908,7 @@ module ActiveRecord def uses_transaction(*methods) @uses_transaction = [] unless defined?(@uses_transaction) - @uses_transaction.concat methods.map { |m| m.to_s } + @uses_transaction.concat methods.map(&:to_s) end def uses_transaction?(method) diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index f58145ab05..b91e9ac137 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -192,7 +192,7 @@ module ActiveRecord def type_condition(table = arel_table) sti_column = table[inheritance_column] - sti_names = ([self] + descendants).map { |model| model.sti_name } + sti_names = ([self] + descendants).map(&:sti_name) sti_column.in(sti_names) end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 92f2951f2d..3cac465440 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -647,7 +647,7 @@ module ActiveRecord end def method_missing(method, *arguments, &block) - arg_list = arguments.map{ |a| a.inspect } * ', ' + arg_list = arguments.map(&:inspect) * ', ' say_with_time "#{method}(#{arg_list})" do unless @connection.respond_to? :revert diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index adad7774b9..92ad9c9101 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -257,7 +257,7 @@ module ActiveRecord # Returns an array of column names as strings. def column_names - @column_names ||= columns.map { |column| column.name } + @column_names ||= columns.map(&:name) end # Returns an array of column objects where the primary id, all columns ending in "_id" or "_count", diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 21b1c3f721..6c09456c4d 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -366,7 +366,7 @@ namespace :railties do namespace :install do # desc "Copies missing migrations from Railties (e.g. engines). You can specify Railties to use with FROM=railtie1,railtie2" task :migrations => :'db:load_config' do - to_load = ENV['FROM'].blank? ? :all : ENV['FROM'].split(",").map {|n| n.strip } + to_load = ENV['FROM'].blank? ? :all : ENV['FROM'].split(",").map(&:strip) railties = {} Rails.application.migration_railties.each do |railtie| next unless to_load == :all || to_load.include?(railtie.railtie_name) diff --git a/activerecord/lib/active_record/readonly_attributes.rb b/activerecord/lib/active_record/readonly_attributes.rb index 85bbac43e4..ce78f1756d 100644 --- a/activerecord/lib/active_record/readonly_attributes.rb +++ b/activerecord/lib/active_record/readonly_attributes.rb @@ -11,7 +11,7 @@ module ActiveRecord # Attributes listed as readonly will be used to create a new record but update operations will # ignore these fields. def attr_readonly(*attributes) - self._attr_readonly = Set.new(attributes.map { |a| a.to_s }) + (self._attr_readonly || []) + self._attr_readonly = Set.new(attributes.map(&:to_s)) + (self._attr_readonly || []) end # Returns an array of all the attributes that have been specified as readonly. diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 4219632596..96520d1d49 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -791,7 +791,7 @@ module ActiveRecord def source_reflection_name # :nodoc: return @source_reflection_name if @source_reflection_name - names = [name.to_s.singularize, name].collect { |n| n.to_sym }.uniq + names = [name.to_s.singularize, name].collect(&:to_sym).uniq names = names.find_all { |n| through_reflection.klass._reflect_on_association(n) } diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 561ed222d1..7e1994086f 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -400,7 +400,7 @@ module ActiveRecord if conditions where(conditions).destroy_all else - to_a.each {|object| object.destroy }.tap { reset } + to_a.each(&:destroy).tap { reset } end end @@ -644,7 +644,7 @@ module ActiveRecord preloader.preload @records, associations end - @records.each { |record| record.readonly! } if readonly_value + @records.each(&:readonly!) if readonly_value @loaded = true @records @@ -666,7 +666,7 @@ module ActiveRecord joined_tables += [table.name, table.table_alias] # always convert table names to downcase as in Oracle quoted table names are in uppercase - joined_tables = joined_tables.flatten.compact.map { |t| t.downcase }.uniq + joined_tables = joined_tables.flatten.compact.map(&:downcase).uniq (references_values - joined_tables).any? end @@ -675,7 +675,7 @@ module ActiveRecord return [] if string.blank? # always convert table names to downcase as in Oracle quoted table names are in uppercase # ignore raw_sql_ that is used by Oracle adapter as alias for limit/offset subqueries - string.scan(/([a-zA-Z_][.\w]+).?\./).flatten.map{ |s| s.downcase }.uniq - ['raw_sql_'] + string.scan(/([a-zA-Z_][.\w]+).?\./).flatten.map(&:downcase).uniq - ['raw_sql_'] end end end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index eacae73ebb..5061940d82 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -415,7 +415,7 @@ module ActiveRecord end def using_limitable_reflections?(reflections) - reflections.none? { |r| r.collection? } + reflections.none?(&:collection?) end protected diff --git a/activerecord/lib/active_record/serialization.rb b/activerecord/lib/active_record/serialization.rb index bd9079b596..48c12dcf9f 100644 --- a/activerecord/lib/active_record/serialization.rb +++ b/activerecord/lib/active_record/serialization.rb @@ -11,7 +11,7 @@ module ActiveRecord #:nodoc: def serializable_hash(options = nil) options = options.try(:clone) || {} - options[:except] = Array(options[:except]).map { |n| n.to_s } + options[:except] = Array(options[:except]).map(&:to_s) options[:except] |= Array(self.class.inheritance_column) super(options) diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index c68990bf99..192a19f05d 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -79,7 +79,7 @@ module ActiveRecord end def bind(values) - bvs = @bind_values.map { |pair| pair.dup } + bvs = @bind_values.map(&:dup) @indexes.each_with_index { |offset,i| bvs[offset][1] = values[i] } bvs end diff --git a/activerecord/lib/active_record/validations/presence.rb b/activerecord/lib/active_record/validations/presence.rb index c7aa814ba8..61b30749d9 100644 --- a/activerecord/lib/active_record/validations/presence.rb +++ b/activerecord/lib/active_record/validations/presence.rb @@ -8,7 +8,7 @@ module ActiveRecord associated_records = Array.wrap(record.send(attribute)) # Superclass validates presence. Ensure present records aren't about to be destroyed. - if associated_records.present? && associated_records.all? { |r| r.marked_for_destruction? } + if associated_records.present? && associated_records.all?(&:marked_for_destruction?) record.errors.add(attribute, :blank, options) end end -- cgit v1.2.3 From de239066e32d654f6815a94cdf8ce4b67b058726 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Wed, 26 Nov 2014 13:22:11 -0700 Subject: Stop using `Arel::Table.engine` We never actually make use of it on the table, since we're constructing the select manager manually. It looks like if we ever actually were grabbing it from the table, we're grossly misusing it since it's meant to vary by AR class. Its existence on `Arel::Table` appears to be purely for convenience methods that are never used outside of tests. However, in production code it just complicates construction of the tables on the rails side, and the plan is to remove it from `Arel::Table` entirely. I'm not convinced it needs to live on `SelectManager`, etc either. --- activerecord/lib/active_record/core.rb | 2 +- activerecord/lib/active_record/relation/predicate_builder.rb | 4 ++-- activerecord/lib/active_record/relation/query_methods.rb | 2 +- activerecord/lib/active_record/sanitization.rb | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 89d8932e9e..c812e7842a 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -235,7 +235,7 @@ module ActiveRecord # scope :published_and_commented, -> { published.and(self.arel_table[:comments_count].gt(0)) } # end def arel_table # :nodoc: - @arel_table ||= Arel::Table.new(table_name, arel_engine) + @arel_table ||= Arel::Table.new(table_name) end # Returns the Arel engine. diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index e4b6b49087..938bf55c6d 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -25,7 +25,7 @@ module ActiveRecord if value.empty? queries << '1=0' else - table = Arel::Table.new(column, default_table.engine) + table = Arel::Table.new(column) association = klass._reflect_on_association(column) value.each do |k, v| @@ -37,7 +37,7 @@ module ActiveRecord if column.include?('.') table_name, column = column.split('.', 2) - table = Arel::Table.new(table_name, default_table.engine) + table = Arel::Table.new(table_name) end queries.concat expand(klass, table, column, value) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6e384facce..0edd2b1e48 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -858,7 +858,7 @@ module ActiveRecord private def build_arel - arel = Arel::SelectManager.new(table.engine, table) + arel = Arel::SelectManager.new(klass.arel_engine, table) build_joins(arel, joins_values.flatten) unless joins_values.empty? diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 6a130c145b..88dfddebcc 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -93,7 +93,7 @@ sanitize_sql_hash_for_conditions is deprecated, and will be removed in Rails 5.0 attrs = PredicateBuilder.resolve_column_aliases self, attrs attrs = expand_hash_conditions_for_aggregates(attrs) - table = Arel::Table.new(table_name, arel_engine).alias(default_table_name) + table = Arel::Table.new(table_name).alias(default_table_name) PredicateBuilder.build_from_hash(self, attrs, table).map { |b| connection.visitor.compile b }.join(' AND ') -- cgit v1.2.3 From a975407a0a4f8bb06f8d4696de6a8b5e5a086622 Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Sat, 29 Nov 2014 17:23:10 -0700 Subject: Update Arel usage for rails/arel#98fc259 `where_sql` now requires that we pass it an engine. None of the manager classes take an engine in their constructor. --- .../active_record/associations/has_many_through_association.rb | 2 +- activerecord/lib/active_record/relation.rb | 4 ++-- activerecord/lib/active_record/relation/finder_methods.rb | 8 ++++---- activerecord/lib/active_record/relation/query_methods.rb | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 5cfedcc009..3f4d3bfc08 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -165,7 +165,7 @@ module ActiveRecord arel = scope.arel - stmt = Arel::DeleteManager.new arel.engine + stmt = Arel::DeleteManager.new stmt.from scope.klass.arel_table stmt.wheres = arel.constraints diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 7e1994086f..7df50f7c97 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -327,7 +327,7 @@ module ActiveRecord def update_all(updates) raise ArgumentError, "Empty list of attributes to change" if updates.blank? - stmt = Arel::UpdateManager.new(arel.engine) + stmt = Arel::UpdateManager.new stmt.set Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)) stmt.table(table) @@ -465,7 +465,7 @@ module ActiveRecord if conditions where(conditions).delete_all else - stmt = Arel::DeleteManager.new(arel.engine) + stmt = Arel::DeleteManager.new stmt.from(table) if joins_values.any? diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 5061940d82..357861caaa 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -108,7 +108,7 @@ module ActiveRecord # Same as +take+ but raises ActiveRecord::RecordNotFound if no record # is found. Note that take! accepts no arguments. def take! - take or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql}]") + take or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") end # Find the first record (or first N records if a parameter is supplied). @@ -176,7 +176,7 @@ module ActiveRecord # Same as +last+ but raises ActiveRecord::RecordNotFound if no record # is found. Note that last! accepts no arguments. def last! - last or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql}]") + last or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") end # Find the second record. @@ -323,7 +323,7 @@ module ActiveRecord # the expected number of results should be provided in the +expected_size+ # argument. def raise_record_not_found_exception!(ids, result_size, expected_size) #:nodoc: - conditions = arel.where_sql + conditions = arel.where_sql(@klass.arel_engine) conditions = " [#{conditions}]" if conditions if Array(ids).size == 1 @@ -498,7 +498,7 @@ module ActiveRecord end def find_nth!(index) - find_nth(index, offset_index) or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql}]") + find_nth(index, offset_index) or raise RecordNotFound.new("Couldn't find #{@klass.name} with [#{arel.where_sql(@klass.arel_engine)}]") end def find_nth_with_limit(offset, limit) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 0edd2b1e48..d58af01339 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -858,7 +858,7 @@ module ActiveRecord private def build_arel - arel = Arel::SelectManager.new(klass.arel_engine, table) + arel = Arel::SelectManager.new(table) build_joins(arel, joins_values.flatten) unless joins_values.empty? -- cgit v1.2.3 From 6874133f9e5ca79f30f43632da4338d7b82ddeff Mon Sep 17 00:00:00 2001 From: Hendy Tanata Date: Wed, 26 Nov 2014 09:16:44 -0800 Subject: Fix grammar on ActiveRecord::AttributeMethods doc. [ci skip] --- activerecord/lib/active_record/attribute_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index d766996d37..83fcefa64d 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -218,7 +218,7 @@ module ActiveRecord # A Person object with a name attribute can ask person.respond_to?(:name), # person.respond_to?(:name=), and person.respond_to?(:name?) - # which will all return +true+. It also define the attribute methods if they have + # which will all return +true+. It also defines the attribute methods if they have # not been generated. # # class Person < ActiveRecord::Base -- cgit v1.2.3