diff options
Diffstat (limited to 'activerecord')
23 files changed, 312 insertions, 37 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0d3852f199..0e8fe55e84 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,4 +1,73 @@ ## unreleased ## +* Fix mysql2 adapter raises the correct exception when executing a query on a + closed connection. + + *Yves Senn* + +* Fixes bug where `Company.new.contract_ids` would incorrectly load + all non-associated contracts. + + Example: + + company = Company.new # Company has many :contracts + + # before + company.contract_ids # => SELECT ... WHERE `contracts`.`company_id` IS NULL + + # after + company.contract_ids # => [] + + *Jared Armstrong* + +* Fix the `:primary_key` option for `has_many` associations. + Fixes #10693. + + *Yves Senn* + +* fixes bug introduced by #3329. Now, when autosaving associations, + deletions happen before inserts and saves. This prevents a 'duplicate + unique value' database error that would occur if a record being created had + the same value on a unique indexed field as that of a record being destroyed. + + Backport of #10417 + + *Johnny Holton* + +* Fix that under some conditions, Active Record could produce invalid SQL of the sort: + "SELECT DISTINCT DISTINCT". + + Backport of #6792. + + *Ben Woosley* + +* Require `ActiveRecord::Base` in railtie hooks for rake_tasks, console and runner to + avoid circular constant loading issues. + + Backport #7695. + + Fixes #7683 and #882 + + *Ben Holley* + +* Maintain context for joins within ActiveRecord::Relation merges. + Backport #10164. + + *Neeraj Singh + Andrew Horner* + +* Make sure the `EXPLAIN` command is never triggered by a `select_db` call. + + *Daniel Schierbeck* + +* Revert changes on `pluck` that was ignoring the select clause when the relation already + has one. This caused a regression since it changed the behavior in a stable release. + + Fixes #9777. + + *Rafael Mendonça França* + +* Confirm a record has not already been destroyed before decrementing counter cache. + + *Ben Tucker* * Default values for PostgreSQL bigint types now get parsed and dumped to the schema correctly. diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 1759a41d93..d78717704c 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -34,7 +34,10 @@ module ActiveRecord::Associations::Builder method_name = "belongs_to_counter_cache_before_destroy_for_#{name}" mixin.redefine_method(method_name) do record = send(name) - record.class.decrement_counter(cache_column, record.id) unless record.nil? + + if record && !self.destroyed? + record.class.decrement_counter(cache_column, record.id) + end end model.before_destroy(method_name) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 65e882867e..baddb9852f 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -43,7 +43,7 @@ module ActiveRecord # Implements the ids reader method, e.g. foo.item_ids for Foo.has_many :items def ids_reader - if loaded? || options[:finder_sql] + if owner.new_record? || loaded? || options[:finder_sql] load_target.map do |record| record.send(reflection.association_primary_key) end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 059e6c77bc..5296cb7282 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -89,8 +89,7 @@ module ActiveRecord records.each { |r| r.destroy } update_counter(-records.length) unless inverse_updates_counter_cache? else - keys = records.map { |r| r[reflection.association_primary_key] } - scope = scoped.where(reflection.association_primary_key => keys) + scope = self.scoped.where(reflection.klass.primary_key => records) if method == :delete_all update_counter(-scope.delete_all) diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index cd366ac8b7..e3d8356f49 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -109,7 +109,7 @@ module ActiveRecord case associations when Symbol, String reflection = parent.reflections[associations.to_s.intern] or - raise ConfigurationError, "Association named '#{ associations }' was not found; perhaps you misspelled it?" + raise ConfigurationError, "Association named '#{ associations }' was not found on #{parent.active_record.name}; perhaps you misspelled it?" unless join_association = find_join_association(reflection, parent) @reflections << reflection join_association = build_join_association(reflection, parent) 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 03963ab060..becf1a3f62 100644 --- a/activerecord/lib/active_record/associations/join_dependency/join_association.rb +++ b/activerecord/lib/active_record/associations/join_dependency/join_association.rb @@ -55,7 +55,12 @@ module ActiveRecord def find_parent_in(other_join_dependency) other_join_dependency.join_parts.detect do |join_part| - parent == join_part + case parent + when JoinBase + parent.active_record == join_part.active_record + else + parent == join_part + end end end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index e1499fc3b0..3fc9d307bc 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -332,16 +332,16 @@ module ActiveRecord if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) begin - records_to_destroy = [] + if autosave + records_to_destroy = records.select(&:marked_for_destruction?) + records_to_destroy.each { |record| association.proxy.destroy(record) } + records -= records_to_destroy + end records.each do |record| - next if record.destroyed? - saved = true - if autosave && record.marked_for_destruction? - records_to_destroy << record - elsif autosave != false && (@new_record_before_save || record.new_record?) + if autosave != false && (@new_record_before_save || record.new_record?) if autosave saved = association.insert_record(record, false) else @@ -353,19 +353,14 @@ module ActiveRecord raise ActiveRecord::Rollback unless saved end - - records_to_destroy.each do |record| - association.proxy.destroy(record) - end rescue records.each {|x| IdentityMap.remove(x) } if IdentityMap.enabled? raise end - end # reconstruct the scope now that we know the owner's id - association.send(:reset_scope) if association.respond_to?(:reset_scope) + association.reset_scope if association.respond_to?(:reset_scope) end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 524a7d30fc..c690b982a1 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -204,9 +204,11 @@ module ActiveRecord # Executes the SQL statement in the context of this connection. def execute(sql, name = nil) - # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been - # made since we established the connection - @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone + if @connection + # make sure we carry over any changes to ActiveRecord::Base.default_timezone that have been + # made since we established the connection + @connection.query_options[:database_timezone] = ActiveRecord::Base.default_timezone + end super end diff --git a/activerecord/lib/active_record/explain_subscriber.rb b/activerecord/lib/active_record/explain_subscriber.rb index 859c8edfc5..1d861a57db 100644 --- a/activerecord/lib/active_record/explain_subscriber.rb +++ b/activerecord/lib/active_record/explain_subscriber.rb @@ -15,7 +15,7 @@ module ActiveRecord # On the other hand, we want to monitor the performance of our real database # queries, not the performance of the access to the query cache. IGNORED_PAYLOADS = %w(SCHEMA EXPLAIN CACHE) - EXPLAINED_SQLS = /\A\s*(select|update|delete|insert)/i + EXPLAINED_SQLS = /\A\s*(select|update|delete|insert)\b/i def ignore_payload?(payload) payload[:exception] || IGNORED_PAYLOADS.include?(payload[:name]) || payload[:sql] !~ EXPLAINED_SQLS end diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 13b7c6e214..4e39654e5b 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -30,6 +30,7 @@ module ActiveRecord ) rake_tasks do + require "active_record/base" load "active_record/railties/databases.rake" end @@ -38,9 +39,14 @@ module ActiveRecord # first time. Also, make it output to STDERR. console do |app| require "active_record/railties/console_sandbox" if app.sandbox? + require "active_record/base" ActiveRecord::Base.logger = Logger.new(STDERR) end + runner do |app| + require "active_record/base" + end + initializer "active_record.initialize_timezone" do ActiveSupport.on_load(:active_record) do self.time_zone_aware_attributes = true diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 1f9dbdc3d4..afab793a0c 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -179,14 +179,13 @@ module ActiveRecord def pluck(column_name) if column_name.is_a?(Symbol) && column_names.include?(column_name.to_s) column_name = "#{connection.quote_table_name(table_name)}.#{connection.quote_column_name(column_name)}" - else - column_name = column_name.to_s end - relation = clone - relation.select_values = [column_name] - klass.connection.select_all(relation.arel).map! do |attributes| - klass.type_cast_attribute(attributes.keys.first, klass.initialize_attributes(attributes)) + result = klass.connection.exec_query(select(column_name).to_sql) + last_column = result.columns.last + + result.map do |attributes| + klass.type_cast_attribute(last_column, klass.initialize_attributes(attributes)) end end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 08cfe4f70d..cdf18f8080 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -254,6 +254,7 @@ module ActiveRecord values = @klass.connection.distinct("#{@klass.connection.quote_table_name table_name}.#{primary_key}", orders) relation = relation.dup.select(values) + relation.uniq_value = nil id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values) ids_array = id_rows.map {|row| row[primary_key]} diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index b8e384c2f3..90f2ac3cde 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -27,7 +27,7 @@ module ActiveRecord merge_relation_method(merged_relation, method, value) if value.present? end - merged_relation.joins_values += r.joins_values + merge_joins(merged_relation, r) merged_wheres = @where_values + r.where_values @@ -146,6 +146,32 @@ module ActiveRecord private + def merge_joins(relation, other) + values = other.joins_values + return if values.blank? + + if other.klass == relation.klass + relation.joins_values += values + else + joins_dependency, rest = values.partition do |join| + case join + when Hash, Symbol, Array + true + else + false + end + end + + join_dependency = ActiveRecord::Associations::JoinDependency.new( + other.klass, + joins_dependency, + [] + ) + + relation.joins_values += join_dependency.join_associations + rest + end + end + def merge_relation_method(relation, method, value) relation.send(:"#{method}_values=", relation.send(:"#{method}_values") + value) end diff --git a/activerecord/test/cases/adapters/postgresql/bytea_test.rb b/activerecord/test/cases/adapters/postgresql/bytea_test.rb new file mode 100644 index 0000000000..5ed2d8aba2 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/bytea_test.rb @@ -0,0 +1,46 @@ +# encoding: utf-8 + +require "cases/helper" +require 'active_record/base' +require 'active_record/connection_adapters/postgresql_adapter' + +class PostgresqlByteaTest < ActiveRecord::TestCase + class ByteaDataType < ActiveRecord::Base + self.table_name = 'bytea_data_type' + end + + def setup + @connection = ActiveRecord::Base.connection + begin + @connection.transaction do + @connection.create_table('bytea_data_type') do |t| + t.binary 'payload' + t.binary 'serialized' + end + end + end + @column = ByteaDataType.columns.find { |c| c.name == 'payload' } + assert(@column.is_a?(ActiveRecord::ConnectionAdapters::PostgreSQLColumn)) + end + + def teardown + @connection.execute 'drop table if exists bytea_data_type' + end + + class Serializer + def load(str); str; end + def dump(str); str; end + end + + def test_serialize + serializer = Serializer.new + klass = Class.new(ByteaDataType) { + serialize :serialized, Serializer.new + } + obj = klass.new + obj.serialized = "hello world" + obj.save! + obj.reload + assert_equal "hello world", obj.serialized + end +end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index c9b26895ae..58c788e42d 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -387,6 +387,26 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal 15, topic.replies.size end + def test_counter_cache_double_destroy + topic = Topic.create :title => "Zoom-zoom-zoom" + + 5.times do + topic.replies.create(:title => "re: zoom", :content => "speedy quick!") + end + + assert_equal 5, topic.reload[:replies_count] + assert_equal 5, topic.replies.size + + reply = topic.replies.first + + reply.destroy + assert_equal 4, topic.reload[:replies_count] + + reply.destroy + assert_equal 4, topic.reload[:replies_count] + assert_equal 4, topic.replies.size + end + def test_custom_counter_cache reply = Reply.create(:title => "re: zoom", :content => "speedy quick!") assert_equal 0, reply[:replies_count] diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 869ec1e4b8..d94f5d3207 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1311,6 +1311,33 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !company.clients.loaded? end + def test_get_ids_for_association_on_new_record_does_not_try_to_find_records + Company.columns # Load schema information so we don't query below + Contract.columns # if running just this test. + + company = Company.new + assert_queries(0) do + company.contract_ids + end + + assert_equal [], company.contract_ids + end + + def test_set_ids_for_association_on_new_record_applies_association_correctly + contract_a = Contract.create! + contract_b = Contract.create! + Contract.create! # another contract + company = Company.new(:name => "Some Company") + + company.contract_ids = [contract_a.id, contract_b.id] + assert_equal [contract_a.id, contract_b.id], company.contract_ids + assert_equal [contract_a, contract_b], company.contracts + + company.save! + assert_equal company, contract_a.reload.company + assert_equal company, contract_b.reload.company + end + def test_get_ids_ignores_include_option assert_equal [readers(:michael_welcome).id], posts(:welcome).readers_with_person_ids end @@ -1492,6 +1519,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal david.essays, Essay.find_all_by_writer_id("David") end + def test_has_many_assignment_with_custom_primary_key + david = people(:david) + + assert_equal ["A Modest Proposal"], david.essays.map(&:name) + david.essays = [Essay.create!(:name => "Remote Work" )] + assert_equal ["Remote Work"], david.essays.map(&:name) + end + def test_blank_custom_primary_key_on_new_record_should_not_run_queries author = Author.new assert !author.essays.loaded? diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index e6b881389b..f7697fa77b 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -583,7 +583,7 @@ class TestDefaultAutosaveAssociationOnNewRecord < ActiveRecord::TestCase end class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase - self.use_transactional_fixtures = false unless supports_savepoints? + self.use_transactional_fixtures = false def setup @pirate = Pirate.create(:catchphrase => "Don' botharrr talkin' like one, savvy?") @@ -780,6 +780,20 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase assert_equal 2, @pirate.birds.reload.length end + def test_should_save_new_record_that_has_same_value_as_existing_record_marked_for_destruction_on_field_that_has_unique_index + Bird.connection.add_index :birds, :name, :unique => true + + 3.times { |i| @pirate.birds.create(:name => "unique_birds_#{i}") } + + @pirate.birds[0].mark_for_destruction + @pirate.birds.build(:name => @pirate.birds[0].name) + @pirate.save! + + assert_equal 3, @pirate.birds.reload.length + ensure + Bird.connection.remove_index :birds, :column => :name + end + # Add and remove callbacks tests for association collections. %w{ method proc }.each do |callback_type| define_method("test_should_run_add_callback_#{callback_type}s_for_has_many") do @@ -862,8 +876,10 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase @pirate.parrots.each { |parrot| parrot.mark_for_destruction } assert @pirate.save - assert_queries(0) do - assert @pirate.save + Pirate.transaction do + assert_queries(0) do + assert @pirate.save + end end end diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index a1dc1de38d..8755e1f580 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -493,10 +493,10 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal [1,2,3,4], Topic.order(:id).pluck("topics.id") end - def test_pluck_replaces_select_clause - taks_relation = Topic.select([:approved, :id]).order(:id) - assert_equal [1,2,3,4], taks_relation.pluck(:id) - assert_equal [false, true, true, true], taks_relation.pluck(:approved) + def test_pluck_does_not_replace_select_clause + taks_relation = Topic.select("approved, id, id AS foo_id").order('foo_id DESC') + assert_equal [4,3,2,1], taks_relation.pluck(:id) + assert_equal [true, true, true, false], taks_relation.pluck(:approved) end def test_pluck_auto_table_name_prefix diff --git a/activerecord/test/cases/disconnected_test.rb b/activerecord/test/cases/disconnected_test.rb new file mode 100644 index 0000000000..cc2c1f6489 --- /dev/null +++ b/activerecord/test/cases/disconnected_test.rb @@ -0,0 +1,26 @@ +require "cases/helper" + +class TestRecord < ActiveRecord::Base +end + +class TestDisconnectedAdapter < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + def setup + @connection = ActiveRecord::Base.connection + end + + def teardown + spec = ActiveRecord::Base.connection_config + ActiveRecord::Base.establish_connection(spec) + @connection = nil + end + + test "can't execute statements while disconnected" do + @connection.execute "SELECT count(*) from products" + @connection.disconnect! + assert_raises(ActiveRecord::StatementInvalid) do + @connection.execute "SELECT count(*) from products" + end + end +end diff --git a/activerecord/test/cases/explain_subscriber_test.rb b/activerecord/test/cases/explain_subscriber_test.rb index 7b852a625d..0546e793fe 100644 --- a/activerecord/test/cases/explain_subscriber_test.rb +++ b/activerecord/test/cases/explain_subscriber_test.rb @@ -38,6 +38,13 @@ if ActiveRecord::Base.connection.supports_explain? end end + def test_collects_nothing_if_the_statement_is_only_partially_matched + with_queries([]) do |queries| + SUBSCRIBER.call(:name => 'SQL', :sql => 'select_db yo_mama') + assert queries.empty? + end + end + def test_collects_nothing_if_unexplained_sqls with_queries([]) do |queries| SUBSCRIBER.call(:name => 'SQL', :sql => 'SHOW max_identifier_length') diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 21bd7b51d5..2efafe5c24 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -93,6 +93,18 @@ class FinderTest < ActiveRecord::TestCase assert !Topic.includes(:replies).limit(1).where('0 = 1').exists? end + def test_exists_with_distinct_association_includes_and_limit + author = Author.first + assert !author.unique_categorized_posts.includes(:special_comments).limit(0).exists? + assert author.unique_categorized_posts.includes(:special_comments).limit(1).exists? + end + + def test_exists_with_distinct_association_includes_limit_and_order + author = Author.first + assert !author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(0).exists? + assert author.unique_categorized_posts.includes(:special_comments).order('comments.taggings_count DESC').limit(1).exists? + end + def test_exists_with_empty_table_and_no_args_given Topic.delete_all assert !Topic.exists? diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 98fe6b7611..f14eee2eb8 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -4,6 +4,7 @@ require 'models/tagging' require 'models/post' require 'models/topic' require 'models/comment' +require 'models/rating' require 'models/reply' require 'models/author' require 'models/comment' @@ -19,7 +20,7 @@ require 'models/minivan' class RelationTest < ActiveRecord::TestCase fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments, - :tags, :taggings, :cars, :minivans + :ratings, :tags, :taggings, :cars, :minivans def test_do_not_double_quote_string_id van = Minivan.last @@ -696,6 +697,12 @@ class RelationTest < ActiveRecord::TestCase assert_equal 1, comments.count end + def test_relation_merging_with_merged_joins + special_comments_with_ratings = SpecialComment.joins(:ratings) + posts_with_special_comments_with_ratings = Post.group('posts.id').joins(:special_comments).merge(special_comments_with_ratings) + assert_equal 1, authors(:david).posts.merge(posts_with_special_comments_with_ratings).count.length + end + def test_count posts = Post.scoped diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index 07c529d685..d316a0b992 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -27,6 +27,7 @@ class Person < ActiveRecord::Base has_many :agents_posts, :through => :agents, :source => :posts has_many :agents_posts_authors, :through => :agents_posts, :source => :author + has_many :essays, :primary_key => "first_name", :foreign_key => "writer_id" scope :males, :conditions => { :gender => 'M' } scope :females, :conditions => { :gender => 'F' } |