diff options
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/CHANGELOG.md | 10 | ||||
-rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb | 3 | ||||
-rw-r--r-- | activerecord/lib/active_record/model_schema.rb | 2 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation.rb | 10 | ||||
-rw-r--r-- | activerecord/lib/active_record/relation/query_methods.rb | 4 | ||||
-rw-r--r-- | activerecord/test/cases/base_test.rb | 10 | ||||
-rw-r--r-- | activerecord/test/cases/locking_test.rb | 4 | ||||
-rw-r--r-- | activerecord/test/cases/persistence_test.rb | 54 | ||||
-rw-r--r-- | activerecord/test/cases/relations_test.rb | 2 | ||||
-rw-r--r-- | activerecord/test/cases/reserved_word_test.rb | 9 | ||||
-rw-r--r-- | activerecord/test/fixtures/reserved_words/values.yml | 4 |
11 files changed, 83 insertions, 29 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c88a4e7695..73970c99d0 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Using subselect for `delete_all` with `limit` or `offset`. + + *Ryuta Kamizono* + +* Undefine attribute methods on descendants when resetting column + information. + + *Chris Salzberg* + * Log database query callers Add `verbose_query_logs` configuration option to display the caller @@ -548,4 +557,5 @@ *Kevin McPhillips* + Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/activerecord/CHANGELOG.md) for previous changes. 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 8569c76317..bff2f51efd 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -717,7 +717,8 @@ module ActiveRecord # to work with MySQL 5.7.6 which sets optimizer_switch='derived_merge=on' subselect.distinct unless select.limit || select.offset || select.orders.any? - Arel::SelectManager.new(subselect.as("__active_record_temp")).project(Arel.sql(key.name)) + key_name = quote_column_name(key.name) + Arel::SelectManager.new(subselect.as("__active_record_temp")).project(Arel.sql(key_name)) end def supports_rename_index? diff --git a/activerecord/lib/active_record/model_schema.rb b/activerecord/lib/active_record/model_schema.rb index 773b0f6cde..fa7537c1a3 100644 --- a/activerecord/lib/active_record/model_schema.rb +++ b/activerecord/lib/active_record/model_schema.rb @@ -425,7 +425,7 @@ module ActiveRecord # end def reset_column_information connection.clear_cache! - undefine_attribute_methods + ([self] + descendants).each(&:undefine_attribute_methods) connection.schema_cache.clear_data_source_cache!(table_name) reload_schema_from_cache diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index dd0d52ad1c..4df3864d07 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -10,7 +10,7 @@ module ActiveRecord SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering, :reverse_order, :distinct, :create_with, :skip_query_cache] CLAUSE_METHODS = [:where, :having, :from] - INVALID_METHODS_FOR_DELETE_ALL = [:limit, :distinct, :offset, :group, :having] + INVALID_METHODS_FOR_DELETE_ALL = [:distinct, :group, :having] VALUE_METHODS = MULTI_VALUE_METHODS + SINGLE_VALUE_METHODS + CLAUSE_METHODS @@ -317,7 +317,7 @@ module ActiveRecord stmt.set Arel.sql(@klass.sanitize_sql_for_assignment(updates)) stmt.table(table) - if has_join_values? + if has_join_values? || offset_value @klass.connection.join_to_update(stmt, arel, arel_attribute(primary_key)) else stmt.key = arel_attribute(primary_key) @@ -365,8 +365,8 @@ module ActiveRecord # # If an invalid method is supplied, #delete_all raises an ActiveRecordError: # - # Post.limit(100).delete_all - # # => ActiveRecord::ActiveRecordError: delete_all doesn't support limit + # Post.distinct.delete_all + # # => ActiveRecord::ActiveRecordError: delete_all doesn't support distinct def delete_all invalid_methods = INVALID_METHODS_FOR_DELETE_ALL.select do |method| value = get_value(method) @@ -384,7 +384,7 @@ module ActiveRecord stmt = Arel::DeleteManager.new stmt.from(table) - if has_join_values? + if has_join_values? || has_limit_or_offset? @klass.connection.join_to_delete(stmt, arel, arel_attribute(primary_key)) else stmt.wheres = arel.constraints diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6b3ff3d610..2056f9bb73 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1040,8 +1040,8 @@ module ActiveRecord def build_select(arel) if select_values.any? arel.project(*arel_columns(select_values.uniq)) - elsif @klass.ignored_columns.any? - arel.project(*arel_columns(@klass.column_names.map(&:to_sym))) + elsif klass.ignored_columns.any? + arel.project(*klass.column_names.map { |field| arel_attribute(field) }) else arel.project(table[Arel.star]) end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index 7fc4a396e4..b076b452e7 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1498,10 +1498,14 @@ class BasicsTest < ActiveRecord::TestCase test "column names are quoted when using #from clause and model has ignored columns" do refute_empty Developer.ignored_columns - query = Developer.from("`developers`").to_sql - quoted_id = Developer.connection.quote_table_name("id") + query = Developer.from("developers").to_sql + quoted_id = "#{Developer.quoted_table_name}.#{Developer.quoted_primary_key}" - assert_match(/SELECT #{quoted_id}.* FROM `developers`/, query) + assert_match(/SELECT #{quoted_id}.* FROM developers/, query) + end + + test "using table name qualified column names unless having SELECT list explicitly" do + assert_equal developers(:david), Developer.from("developers").joins(:shared_computers).take end test "protected environments by default is an array with production" do diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 437a5a38a3..3701be4b11 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -406,7 +406,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 0, car.lock_version previously_car_updated_at = car.updated_at - travel(1.second) do + travel(2.second) do Wheel.create!(wheelable: car) end @@ -422,7 +422,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 2, car.lock_version previously_car_updated_at = car.updated_at - travel(1.second) do + travel(2.second) do car.wheels.first.destroy! end diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb index c0b9a3ef76..07c4a17fb1 100644 --- a/activerecord/test/cases/persistence_test.rb +++ b/activerecord/test/cases/persistence_test.rb @@ -59,13 +59,42 @@ class PersistenceTest < ActiveRecord::TestCase def test_update_all_with_order_and_limit_updates_subset_only author = authors(:david) - assert_nothing_raised do - assert_equal 1, author.posts_sorted_by_id_limited.size - assert_equal 2, author.posts_sorted_by_id_limited.limit(2).to_a.size - assert_equal 1, author.posts_sorted_by_id_limited.update_all([ "body = ?", "bulk update!" ]) - assert_equal "bulk update!", posts(:welcome).body - assert_not_equal "bulk update!", posts(:thinking).body - end + limited_posts = author.posts_sorted_by_id_limited + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.update_all([ "body = ?", "bulk update!" ]) + assert_equal "bulk update!", posts(:welcome).body + assert_not_equal "bulk update!", posts(:thinking).body + end + + def test_update_all_with_order_and_limit_and_offset_updates_subset_only + author = authors(:david) + limited_posts = author.posts_sorted_by_id_limited.offset(1) + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.update_all([ "body = ?", "bulk update!" ]) + assert_equal "bulk update!", posts(:thinking).body + assert_not_equal "bulk update!", posts(:welcome).body + end + + def test_delete_all_with_order_and_limit_deletes_subset_only + author = authors(:david) + limited_posts = Post.where(author: author).order(:id).limit(1) + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.delete_all + assert_raise(ActiveRecord::RecordNotFound) { posts(:welcome) } + assert posts(:thinking) + end + + def test_delete_all_with_order_and_limit_and_offset_deletes_subset_only + author = authors(:david) + limited_posts = Post.where(author: author).order(:id).limit(1).offset(1) + assert_equal 1, limited_posts.size + assert_equal 2, limited_posts.limit(2).size + assert_equal 1, limited_posts.delete_all + assert_raise(ActiveRecord::RecordNotFound) { posts(:thinking) } + assert posts(:welcome) end end @@ -1106,13 +1135,18 @@ class PersistenceTest < ActiveRecord::TestCase end def test_reset_column_information_resets_children - child = Class.new(Topic) - child.new # force schema to load + child_class = Class.new(Topic) + child_class.new # force schema to load ActiveRecord::Base.connection.add_column(:topics, :foo, :string) Topic.reset_column_information - assert_equal "bar", child.new(foo: :bar).foo + # this should redefine attribute methods + child_class.new + + assert child_class.instance_methods.include?(:foo) + assert child_class.instance_methods.include?(:foo_changed?) + assert_equal "bar", child_class.new(foo: :bar).foo ensure ActiveRecord::Base.connection.remove_column(:topics, :foo) Topic.reset_column_information diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 675aafabda..915ff03c2c 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -896,11 +896,9 @@ class RelationTest < ActiveRecord::TestCase end def test_delete_all_with_unpermitted_relation_raises_error - assert_raises(ActiveRecord::ActiveRecordError) { Author.limit(10).delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.distinct.delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.group(:name).delete_all } assert_raises(ActiveRecord::ActiveRecordError) { Author.having("SUM(id) < 3").delete_all } - assert_raises(ActiveRecord::ActiveRecordError) { Author.offset(10).delete_all } end def test_select_with_aggregates diff --git a/activerecord/test/cases/reserved_word_test.rb b/activerecord/test/cases/reserved_word_test.rb index 0214dbec17..4f8ca392b9 100644 --- a/activerecord/test/cases/reserved_word_test.rb +++ b/activerecord/test/cases/reserved_word_test.rb @@ -39,7 +39,7 @@ class ReservedWordTest < ActiveRecord::TestCase t.string :order t.belongs_to :select end - @connection.create_table :values, force: true do |t| + @connection.create_table :values, primary_key: :as, force: true do |t| t.belongs_to :group end end @@ -88,6 +88,13 @@ class ReservedWordTest < ActiveRecord::TestCase assert_equal x, Group.find(x.id) end + def test_delete_all_with_subselect + create_test_fixtures :values + assert_equal 1, Values.order(:as).limit(1).offset(1).delete_all + assert_raise(ActiveRecord::RecordNotFound) { Values.find(2) } + assert Values.find(1) + end + def test_has_one_associations create_test_fixtures :group, :values v = Group.find(1).values diff --git a/activerecord/test/fixtures/reserved_words/values.yml b/activerecord/test/fixtures/reserved_words/values.yml index 7d109609ab..9ed9e5edc5 100644 --- a/activerecord/test/fixtures/reserved_words/values.yml +++ b/activerecord/test/fixtures/reserved_words/values.yml @@ -1,7 +1,7 @@ values1: - id: 1 + as: 1 group_id: 2 values2: - id: 2 + as: 2 group_id: 1 |