aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2019-04-02 00:10:09 +0900
committerGitHub <noreply@github.com>2019-04-02 00:10:09 +0900
commite0a9e0259c56700fc83b3e886cdf7c04f6a83495 (patch)
tree1a5155bb6d334e31d9f42476372f1d7334a9b80b
parentd3a6277fd60b73ecdf1ad52db97cd9808c2ab2a8 (diff)
parent6e43a207c6d3072d10b494d605b7bb6635043e30 (diff)
downloadrails-e0a9e0259c56700fc83b3e886cdf7c04f6a83495.tar.gz
rails-e0a9e0259c56700fc83b3e886cdf7c04f6a83495.tar.bz2
rails-e0a9e0259c56700fc83b3e886cdf7c04f6a83495.zip
Merge pull request #35816 from kamipo/partly_revert_query_annotation_for_update_and_delete
Revert unused code and re-using query annotation for `update_all` and `delete_all`
-rw-r--r--activerecord/lib/active_record/relation.rb3
-rw-r--r--activerecord/lib/arel/nodes/delete_statement.rb9
-rw-r--r--activerecord/lib/arel/nodes/insert_statement.rb9
-rw-r--r--activerecord/lib/arel/nodes/select_core.rb1
-rw-r--r--activerecord/lib/arel/nodes/update_statement.rb9
-rw-r--r--activerecord/lib/arel/select_manager.rb4
-rw-r--r--activerecord/lib/arel/tree_manager.rb5
-rw-r--r--activerecord/lib/arel/visitors/to_sql.rb6
-rw-r--r--activerecord/test/cases/arel/delete_manager_test.rb18
-rw-r--r--activerecord/test/cases/arel/nodes/delete_statement_test.rb8
-rw-r--r--activerecord/test/cases/arel/nodes/insert_statement_test.rb8
-rw-r--r--activerecord/test/cases/arel/nodes/update_statement_test.rb8
-rw-r--r--activerecord/test/cases/arel/update_manager_test.rb24
-rw-r--r--activerecord/test/cases/relation/delete_all_test.rb19
-rw-r--r--activerecord/test/cases/relation/update_all_test.rb27
15 files changed, 11 insertions, 147 deletions
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 36c2422d84..dd821431e1 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -389,8 +389,6 @@ module ActiveRecord
stmt.set Arel.sql(klass.sanitize_sql_for_assignment(updates, table.name))
end
- stmt.comment(*arel.comment_node.values) if arel.comment_node
-
@klass.connection.update stmt, "#{@klass} Update All"
end
@@ -506,7 +504,6 @@ module ActiveRecord
stmt.offset(arel.offset)
stmt.order(*arel.orders)
stmt.wheres = arel.constraints
- stmt.comment(*arel.comment_node.values) if arel.comment_node
affected = @klass.connection.delete(stmt, "#{@klass} Destroy")
diff --git a/activerecord/lib/arel/nodes/delete_statement.rb b/activerecord/lib/arel/nodes/delete_statement.rb
index 56249b2bad..a419975335 100644
--- a/activerecord/lib/arel/nodes/delete_statement.rb
+++ b/activerecord/lib/arel/nodes/delete_statement.rb
@@ -3,7 +3,7 @@
module Arel # :nodoc: all
module Nodes
class DeleteStatement < Arel::Nodes::Node
- attr_accessor :left, :right, :orders, :limit, :offset, :key, :comment
+ attr_accessor :left, :right, :orders, :limit, :offset, :key
alias :relation :left
alias :relation= :left=
@@ -18,18 +18,16 @@ module Arel # :nodoc: all
@limit = nil
@offset = nil
@key = nil
- @comment = nil
end
def initialize_copy(other)
super
@left = @left.clone if @left
@right = @right.clone if @right
- @comment = @comment.clone if @comment
end
def hash
- [self.class, @left, @right, @orders, @limit, @offset, @key, @comment].hash
+ [self.class, @left, @right, @orders, @limit, @offset, @key].hash
end
def eql?(other)
@@ -39,8 +37,7 @@ module Arel # :nodoc: all
self.orders == other.orders &&
self.limit == other.limit &&
self.offset == other.offset &&
- self.key == other.key &&
- self.comment == other.comment
+ self.key == other.key
end
alias :== :eql?
end
diff --git a/activerecord/lib/arel/nodes/insert_statement.rb b/activerecord/lib/arel/nodes/insert_statement.rb
index 8430dd23da..d28fd1f6c8 100644
--- a/activerecord/lib/arel/nodes/insert_statement.rb
+++ b/activerecord/lib/arel/nodes/insert_statement.rb
@@ -3,7 +3,7 @@
module Arel # :nodoc: all
module Nodes
class InsertStatement < Arel::Nodes::Node
- attr_accessor :relation, :columns, :values, :select, :comment
+ attr_accessor :relation, :columns, :values, :select
def initialize
super()
@@ -11,7 +11,6 @@ module Arel # :nodoc: all
@columns = []
@values = nil
@select = nil
- @comment = nil
end
def initialize_copy(other)
@@ -19,11 +18,10 @@ module Arel # :nodoc: all
@columns = @columns.clone
@values = @values.clone if @values
@select = @select.clone if @select
- @comment = @comment.clone if @comment
end
def hash
- [@relation, @columns, @values, @select, @comment].hash
+ [@relation, @columns, @values, @select].hash
end
def eql?(other)
@@ -31,8 +29,7 @@ module Arel # :nodoc: all
self.relation == other.relation &&
self.columns == other.columns &&
self.select == other.select &&
- self.values == other.values &&
- self.comment == other.comment
+ self.values == other.values
end
alias :== :eql?
end
diff --git a/activerecord/lib/arel/nodes/select_core.rb b/activerecord/lib/arel/nodes/select_core.rb
index b6154b7ff4..11b4f39ece 100644
--- a/activerecord/lib/arel/nodes/select_core.rb
+++ b/activerecord/lib/arel/nodes/select_core.rb
@@ -40,7 +40,6 @@ module Arel # :nodoc: all
@groups = @groups.clone
@havings = @havings.clone
@windows = @windows.clone
- @comment = @comment.clone if @comment
end
def hash
diff --git a/activerecord/lib/arel/nodes/update_statement.rb b/activerecord/lib/arel/nodes/update_statement.rb
index 015bcd7613..cfaa19e392 100644
--- a/activerecord/lib/arel/nodes/update_statement.rb
+++ b/activerecord/lib/arel/nodes/update_statement.rb
@@ -3,7 +3,7 @@
module Arel # :nodoc: all
module Nodes
class UpdateStatement < Arel::Nodes::Node
- attr_accessor :relation, :wheres, :values, :orders, :limit, :offset, :key, :comment
+ attr_accessor :relation, :wheres, :values, :orders, :limit, :offset, :key
def initialize
@relation = nil
@@ -13,18 +13,16 @@ module Arel # :nodoc: all
@limit = nil
@offset = nil
@key = nil
- @comment = nil
end
def initialize_copy(other)
super
@wheres = @wheres.clone
@values = @values.clone
- @comment = @comment.clone if @comment
end
def hash
- [@relation, @wheres, @values, @orders, @limit, @offset, @key, @comment].hash
+ [@relation, @wheres, @values, @orders, @limit, @offset, @key].hash
end
def eql?(other)
@@ -35,8 +33,7 @@ module Arel # :nodoc: all
self.orders == other.orders &&
self.limit == other.limit &&
self.offset == other.offset &&
- self.key == other.key &&
- self.comment == other.comment
+ self.key == other.key
end
alias :== :eql?
end
diff --git a/activerecord/lib/arel/select_manager.rb b/activerecord/lib/arel/select_manager.rb
index 4e9f527235..ddc9e394dd 100644
--- a/activerecord/lib/arel/select_manager.rb
+++ b/activerecord/lib/arel/select_manager.rb
@@ -249,10 +249,6 @@ module Arel # :nodoc: all
self
end
- def comment_node
- @ctx.comment
- end
-
private
def collapse(exprs)
exprs = exprs.compact
diff --git a/activerecord/lib/arel/tree_manager.rb b/activerecord/lib/arel/tree_manager.rb
index 326c4f995c..0476399618 100644
--- a/activerecord/lib/arel/tree_manager.rb
+++ b/activerecord/lib/arel/tree_manager.rb
@@ -36,11 +36,6 @@ module Arel # :nodoc: all
@ast.wheres << expr
self
end
-
- def comment(*values)
- @ast.comment = Nodes::Comment.new(values)
- self
- end
end
attr_reader :ast
diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index 4192d9efdc..277d553e6c 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -35,7 +35,6 @@ module Arel # :nodoc: all
collect_nodes_for o.wheres, collector, " WHERE ", " AND "
collect_nodes_for o.orders, collector, " ORDER BY "
maybe_visit o.limit, collector
- maybe_visit o.comment, collector
end
def visit_Arel_Nodes_UpdateStatement(o, collector)
@@ -48,7 +47,6 @@ module Arel # :nodoc: all
collect_nodes_for o.wheres, collector, " WHERE ", " AND "
collect_nodes_for o.orders, collector, " ORDER BY "
maybe_visit o.limit, collector
- maybe_visit o.comment, collector
end
def visit_Arel_Nodes_InsertStatement(o, collector)
@@ -64,9 +62,9 @@ module Arel # :nodoc: all
maybe_visit o.values, collector
elsif o.select
maybe_visit o.select, collector
+ else
+ collector
end
-
- maybe_visit o.comment, collector
end
def visit_Arel_Nodes_Exists(o, collector)
diff --git a/activerecord/test/cases/arel/delete_manager_test.rb b/activerecord/test/cases/arel/delete_manager_test.rb
index 63cd1bffe3..0bad02f4d2 100644
--- a/activerecord/test/cases/arel/delete_manager_test.rb
+++ b/activerecord/test/cases/arel/delete_manager_test.rb
@@ -49,23 +49,5 @@ module Arel
dm.where(table[:id].eq(10)).must_equal dm
end
end
-
- describe "comment" do
- it "chains" do
- manager = Arel::DeleteManager.new
- manager.comment("deleting").must_equal manager
- end
-
- it "appends a comment to the generated query" do
- table = Table.new(:users)
- dm = Arel::DeleteManager.new
- dm.from table
- dm.comment("deletion")
- assert_match(%r{DELETE FROM "users" /\* deletion \*/}, dm.to_sql)
-
- dm.comment("deletion", "with", "comment")
- assert_match(%r{DELETE FROM "users" /\* deletion \*/ /\* with \*/ /\* comment \*/}, dm.to_sql)
- end
- end
end
end
diff --git a/activerecord/test/cases/arel/nodes/delete_statement_test.rb b/activerecord/test/cases/arel/nodes/delete_statement_test.rb
index 8ba268653d..3f078063a4 100644
--- a/activerecord/test/cases/arel/nodes/delete_statement_test.rb
+++ b/activerecord/test/cases/arel/nodes/delete_statement_test.rb
@@ -18,10 +18,8 @@ describe Arel::Nodes::DeleteStatement do
it "is equal with equal ivars" do
statement1 = Arel::Nodes::DeleteStatement.new
statement1.wheres = %w[a b c]
- statement1.comment = Arel::Nodes::Comment.new(["comment"])
statement2 = Arel::Nodes::DeleteStatement.new
statement2.wheres = %w[a b c]
- statement2.comment = Arel::Nodes::Comment.new(["comment"])
array = [statement1, statement2]
assert_equal 1, array.uniq.size
end
@@ -29,14 +27,8 @@ describe Arel::Nodes::DeleteStatement do
it "is not equal with different ivars" do
statement1 = Arel::Nodes::DeleteStatement.new
statement1.wheres = %w[a b c]
- statement1.comment = Arel::Nodes::Comment.new(["comment"])
statement2 = Arel::Nodes::DeleteStatement.new
statement2.wheres = %w[1 2 3]
- statement2.comment = Arel::Nodes::Comment.new(["comment"])
- array = [statement1, statement2]
- assert_equal 2, array.uniq.size
- statement2.wheres = %w[a b c]
- statement2.comment = Arel::Nodes::Comment.new(["other"])
array = [statement1, statement2]
assert_equal 2, array.uniq.size
end
diff --git a/activerecord/test/cases/arel/nodes/insert_statement_test.rb b/activerecord/test/cases/arel/nodes/insert_statement_test.rb
index 036576b231..252a0d0d0b 100644
--- a/activerecord/test/cases/arel/nodes/insert_statement_test.rb
+++ b/activerecord/test/cases/arel/nodes/insert_statement_test.rb
@@ -23,11 +23,9 @@ describe Arel::Nodes::InsertStatement do
statement1 = Arel::Nodes::InsertStatement.new
statement1.columns = %w[a b c]
statement1.values = %w[x y z]
- statement1.comment = Arel::Nodes::Comment.new(["comment"])
statement2 = Arel::Nodes::InsertStatement.new
statement2.columns = %w[a b c]
statement2.values = %w[x y z]
- statement2.comment = Arel::Nodes::Comment.new(["comment"])
array = [statement1, statement2]
assert_equal 1, array.uniq.size
end
@@ -36,15 +34,9 @@ describe Arel::Nodes::InsertStatement do
statement1 = Arel::Nodes::InsertStatement.new
statement1.columns = %w[a b c]
statement1.values = %w[x y z]
- statement1.comment = Arel::Nodes::Comment.new(["comment"])
statement2 = Arel::Nodes::InsertStatement.new
statement2.columns = %w[a b c]
statement2.values = %w[1 2 3]
- statement2.comment = Arel::Nodes::Comment.new(["comment"])
- array = [statement1, statement2]
- assert_equal 2, array.uniq.size
- statement2.values = %w[x y z]
- statement2.comment = Arel::Nodes::Comment.new("other")
array = [statement1, statement2]
assert_equal 2, array.uniq.size
end
diff --git a/activerecord/test/cases/arel/nodes/update_statement_test.rb b/activerecord/test/cases/arel/nodes/update_statement_test.rb
index f133ddf7eb..a83ce32f68 100644
--- a/activerecord/test/cases/arel/nodes/update_statement_test.rb
+++ b/activerecord/test/cases/arel/nodes/update_statement_test.rb
@@ -27,7 +27,6 @@ describe Arel::Nodes::UpdateStatement do
statement1.orders = %w[x y z]
statement1.limit = 42
statement1.key = "zomg"
- statement1.comment = Arel::Nodes::Comment.new(["comment"])
statement2 = Arel::Nodes::UpdateStatement.new
statement2.relation = "zomg"
statement2.wheres = 2
@@ -35,7 +34,6 @@ describe Arel::Nodes::UpdateStatement do
statement2.orders = %w[x y z]
statement2.limit = 42
statement2.key = "zomg"
- statement2.comment = Arel::Nodes::Comment.new(["comment"])
array = [statement1, statement2]
assert_equal 1, array.uniq.size
end
@@ -48,7 +46,6 @@ describe Arel::Nodes::UpdateStatement do
statement1.orders = %w[x y z]
statement1.limit = 42
statement1.key = "zomg"
- statement1.comment = Arel::Nodes::Comment.new(["comment"])
statement2 = Arel::Nodes::UpdateStatement.new
statement2.relation = "zomg"
statement2.wheres = 2
@@ -56,11 +53,6 @@ describe Arel::Nodes::UpdateStatement do
statement2.orders = %w[x y z]
statement2.limit = 42
statement2.key = "wth"
- statement2.comment = Arel::Nodes::Comment.new(["comment"])
- array = [statement1, statement2]
- assert_equal 2, array.uniq.size
- statement2.key = "zomg"
- statement2.comment = Arel::Nodes::Comment.new(["other"])
array = [statement1, statement2]
assert_equal 2, array.uniq.size
end
diff --git a/activerecord/test/cases/arel/update_manager_test.rb b/activerecord/test/cases/arel/update_manager_test.rb
index e13cb6aa52..cc1b9ac5b3 100644
--- a/activerecord/test/cases/arel/update_manager_test.rb
+++ b/activerecord/test/cases/arel/update_manager_test.rb
@@ -122,29 +122,5 @@ module Arel
@um.key.must_equal @table[:foo]
end
end
-
- describe "comment" do
- it "chains" do
- manager = Arel::UpdateManager.new
- manager.comment("updating").must_equal manager
- end
-
- it "appends a comment to the generated query" do
- table = Table.new :users
-
- manager = Arel::UpdateManager.new
- manager.table table
-
- manager.comment("updating")
- manager.to_sql.must_be_like %{
- UPDATE "users" /* updating */
- }
-
- manager.comment("updating", "with", "comment")
- manager.to_sql.must_be_like %{
- UPDATE "users" /* updating */ /* with */ /* comment */
- }
- end
- end
end
end
diff --git a/activerecord/test/cases/relation/delete_all_test.rb b/activerecord/test/cases/relation/delete_all_test.rb
index 9b76936b7e..d1c13fa1b5 100644
--- a/activerecord/test/cases/relation/delete_all_test.rb
+++ b/activerecord/test/cases/relation/delete_all_test.rb
@@ -99,23 +99,4 @@ class DeleteAllTest < ActiveRecord::TestCase
assert_raise(ActiveRecord::RecordNotFound) { posts(:thinking) }
assert posts(:welcome)
end
-
- def test_delete_all_with_annotation_includes_a_query_comment
- davids = Author.where(name: "David").annotate("deleting all")
-
- assert_sql(%r{/\* deleting all \*/}) do
- assert_difference("Author.count", -1) { davids.delete_all }
- end
- end
-
- def test_delete_all_without_annotation_does_not_include_an_empty_comment
- davids = Author.where(name: "David")
-
- log = capture_sql do
- assert_difference("Author.count", -1) { davids.delete_all }
- end
-
- assert_not_predicate log, :empty?
- assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
- end
end
diff --git a/activerecord/test/cases/relation/update_all_test.rb b/activerecord/test/cases/relation/update_all_test.rb
index 526f926841..0500574f28 100644
--- a/activerecord/test/cases/relation/update_all_test.rb
+++ b/activerecord/test/cases/relation/update_all_test.rb
@@ -241,33 +241,6 @@ class UpdateAllTest < ActiveRecord::TestCase
end
end
- def test_update_all_with_annotation_includes_a_query_comment
- tag = Tag.first
-
- assert_sql(%r{/\* updating all \*/}) do
- Post.tagged_with(tag.id).annotate("updating all").update_all(title: "rofl")
- end
-
- posts = Post.tagged_with(tag.id).all.to_a
- assert_operator posts.length, :>, 0
- posts.each { |post| assert_equal "rofl", post.title }
- end
-
- def test_update_all_without_annotation_does_not_include_an_empty_comment
- tag = Tag.first
-
- log = capture_sql do
- Post.tagged_with(tag.id).update_all(title: "rofl")
- end
-
- assert_not_predicate log, :empty?
- assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty?
-
- posts = Post.tagged_with(tag.id).all.to_a
- assert_operator posts.length, :>, 0
- posts.each { |post| assert_equal "rofl", post.title }
- end
-
# Oracle UPDATE does not support ORDER BY
unless current_adapter?(:OracleAdapter)
def test_update_all_ignores_order_without_limit_from_association