diff options
Diffstat (limited to 'activerecord')
37 files changed, 644 insertions, 19 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 148b3800a8..adf41cd872 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,16 @@ +* Add `ActiveRecord::Relation#annotate` for adding SQL comments to its queries. + + For example: + + ``` + Post.where(id: 123).annotate("this is a comment").to_sql + # SELECT "posts".* FROM "posts" WHERE "posts"."id" = 123 /* this is a comment */ + ``` + + This can be useful in instrumentation or other analysis of issued queries. + + *Matt Yoho* + * Support Optimizer Hints. In most databases, there is a way to control the optimizer is by using optimizer hints, diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 32653956b2..db89b77629 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -61,11 +61,15 @@ module ActiveRecord scope = through_reflection.klass.unscoped options = reflection.options + values = reflection_scope.values + if annotations = values[:annotate] + scope.annotate!(*annotations) + end + if options[:source_type] scope.where! reflection.foreign_type => options[:source_type] elsif !reflection_scope.where_clause.empty? scope.where_clause = reflection_scope.where_clause - values = reflection_scope.values if includes = values[:includes] scope.includes!(source_reflection.name => includes) diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index 86021f0a80..81ab502824 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -15,7 +15,7 @@ module ActiveRecord :select, :reselect, :order, :reorder, :group, :limit, :offset, :joins, :left_joins, :left_outer_joins, :where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly, :extending, :or, :having, :create_with, :distinct, :references, :none, :unscope, :optimizer_hints, :merge, :except, :only, - :count, :average, :minimum, :maximum, :sum, :calculate, + :count, :average, :minimum, :maximum, :sum, :calculate, :annotate, :pluck, :pick, :ids ].freeze # :nodoc: delegate(*QUERYING_METHODS, to: :all) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 6d954a2b2e..36c2422d84 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -5,7 +5,7 @@ module ActiveRecord class Relation MULTI_VALUE_METHODS = [:includes, :eager_load, :preload, :select, :group, :order, :joins, :left_outer_joins, :references, - :extending, :unscope, :optimizer_hints] + :extending, :unscope, :optimizer_hints, :annotate] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :reordering, :reverse_order, :distinct, :create_with, :skip_query_cache] @@ -389,6 +389,8 @@ 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 @@ -504,6 +506,7 @@ 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/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 05ea0850d3..6f0f2125dc 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -349,7 +349,7 @@ module ActiveRecord end VALID_UNSCOPING_VALUES = Set.new([:where, :select, :group, :order, :lock, - :limit, :offset, :joins, :left_outer_joins, + :limit, :offset, :joins, :left_outer_joins, :annotate, :includes, :from, :readonly, :having, :optimizer_hints]) # Removes an unwanted relation that is already defined on a chain of relations. @@ -948,6 +948,26 @@ module ActiveRecord self end + # Adds an SQL comment to queries generated from this relation. For example: + # + # User.annotate("selecting user names").select(:name) + # # SELECT "users"."name" FROM "users" /* selecting user names */ + # + # User.annotate("selecting", "user", "names").select(:name) + # # SELECT "users"."name" FROM "users" /* selecting */ /* user */ /* names */ + # + # The SQL block comment delimiters, "/*" and "*/", will be added automatically. + def annotate(*args) + check_if_method_has_arguments!(:annotate, args) + spawn.annotate!(*args) + end + + # Like #annotate, but modifies relation in place. + def annotate!(*args) # :nodoc: + self.annotate_values += args + self + end + # Returns the Arel object associated with the relation. def arel(aliases = nil) # :nodoc: @arel ||= build_arel(aliases) @@ -1004,6 +1024,7 @@ module ActiveRecord arel.distinct(distinct_value) arel.from(build_from) unless from_clause.empty? arel.lock(lock_value) if lock_value + arel.comment(*annotate_values) unless annotate_values.empty? arel end diff --git a/activerecord/lib/arel/nodes.rb b/activerecord/lib/arel/nodes.rb index 2f6dd9bc45..f994754620 100644 --- a/activerecord/lib/arel/nodes.rb +++ b/activerecord/lib/arel/nodes.rb @@ -61,6 +61,8 @@ require "arel/nodes/outer_join" require "arel/nodes/right_outer_join" require "arel/nodes/string_join" +require "arel/nodes/comment" + require "arel/nodes/sql_literal" require "arel/nodes/casted" diff --git a/activerecord/lib/arel/nodes/comment.rb b/activerecord/lib/arel/nodes/comment.rb new file mode 100644 index 0000000000..237ff27e7e --- /dev/null +++ b/activerecord/lib/arel/nodes/comment.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Arel # :nodoc: all + module Nodes + class Comment < Arel::Nodes::Node + attr_reader :values + + def initialize(values) + super() + @values = values + end + + def initialize_copy(other) + super + @values = @values.clone + end + + def hash + [@values].hash + end + + def eql?(other) + self.class == other.class && + self.values == other.values + end + alias :== :eql? + end + end +end diff --git a/activerecord/lib/arel/nodes/delete_statement.rb b/activerecord/lib/arel/nodes/delete_statement.rb index a419975335..56249b2bad 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 + attr_accessor :left, :right, :orders, :limit, :offset, :key, :comment alias :relation :left alias :relation= :left= @@ -18,16 +18,18 @@ 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].hash + [self.class, @left, @right, @orders, @limit, @offset, @key, @comment].hash end def eql?(other) @@ -37,7 +39,8 @@ module Arel # :nodoc: all self.orders == other.orders && self.limit == other.limit && self.offset == other.offset && - self.key == other.key + self.key == other.key && + self.comment == other.comment end alias :== :eql? end diff --git a/activerecord/lib/arel/nodes/insert_statement.rb b/activerecord/lib/arel/nodes/insert_statement.rb index d28fd1f6c8..8430dd23da 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 + attr_accessor :relation, :columns, :values, :select, :comment def initialize super() @@ -11,6 +11,7 @@ module Arel # :nodoc: all @columns = [] @values = nil @select = nil + @comment = nil end def initialize_copy(other) @@ -18,10 +19,11 @@ 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].hash + [@relation, @columns, @values, @select, @comment].hash end def eql?(other) @@ -29,7 +31,8 @@ module Arel # :nodoc: all self.relation == other.relation && self.columns == other.columns && self.select == other.select && - self.values == other.values + self.values == other.values && + self.comment == other.comment end alias :== :eql? end diff --git a/activerecord/lib/arel/nodes/select_core.rb b/activerecord/lib/arel/nodes/select_core.rb index 5df6ac8412..b6154b7ff4 100644 --- a/activerecord/lib/arel/nodes/select_core.rb +++ b/activerecord/lib/arel/nodes/select_core.rb @@ -3,7 +3,7 @@ module Arel # :nodoc: all module Nodes class SelectCore < Arel::Nodes::Node - attr_accessor :projections, :wheres, :groups, :windows + attr_accessor :projections, :wheres, :groups, :windows, :comment attr_accessor :havings, :source, :set_quantifier, :optimizer_hints def initialize @@ -18,6 +18,7 @@ module Arel # :nodoc: all @groups = [] @havings = [] @windows = [] + @comment = nil end def from @@ -39,12 +40,13 @@ module Arel # :nodoc: all @groups = @groups.clone @havings = @havings.clone @windows = @windows.clone + @comment = @comment.clone if @comment end def hash [ @source, @set_quantifier, @projections, @optimizer_hints, - @wheres, @groups, @havings, @windows + @wheres, @groups, @havings, @windows, @comment ].hash end @@ -57,7 +59,8 @@ module Arel # :nodoc: all self.wheres == other.wheres && self.groups == other.groups && self.havings == other.havings && - self.windows == other.windows + self.windows == other.windows && + self.comment == other.comment end alias :== :eql? end diff --git a/activerecord/lib/arel/nodes/update_statement.rb b/activerecord/lib/arel/nodes/update_statement.rb index cfaa19e392..015bcd7613 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 + attr_accessor :relation, :wheres, :values, :orders, :limit, :offset, :key, :comment def initialize @relation = nil @@ -13,16 +13,18 @@ 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].hash + [@relation, @wheres, @values, @orders, @limit, @offset, @key, @comment].hash end def eql?(other) @@ -33,7 +35,8 @@ module Arel # :nodoc: all self.orders == other.orders && self.limit == other.limit && self.offset == other.offset && - self.key == other.key + self.key == other.key && + self.comment == other.comment end alias :== :eql? end diff --git a/activerecord/lib/arel/select_manager.rb b/activerecord/lib/arel/select_manager.rb index 32286b67f4..4e9f527235 100644 --- a/activerecord/lib/arel/select_manager.rb +++ b/activerecord/lib/arel/select_manager.rb @@ -244,6 +244,15 @@ module Arel # :nodoc: all @ctx.source end + def comment(*values) + @ctx.comment = Nodes::Comment.new(values) + 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 0476399618..326c4f995c 100644 --- a/activerecord/lib/arel/tree_manager.rb +++ b/activerecord/lib/arel/tree_manager.rb @@ -36,6 +36,11 @@ 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/depth_first.rb b/activerecord/lib/arel/visitors/depth_first.rb index 109afb7402..d696edc507 100644 --- a/activerecord/lib/arel/visitors/depth_first.rb +++ b/activerecord/lib/arel/visitors/depth_first.rb @@ -181,6 +181,10 @@ module Arel # :nodoc: all visit o.limit end + def visit_Arel_Nodes_Comment(o) + visit o.values + end + def visit_Array(o) o.each { |i| visit i } end diff --git a/activerecord/lib/arel/visitors/dot.rb b/activerecord/lib/arel/visitors/dot.rb index 37803ce0c0..ecc386de07 100644 --- a/activerecord/lib/arel/visitors/dot.rb +++ b/activerecord/lib/arel/visitors/dot.rb @@ -234,6 +234,10 @@ module Arel # :nodoc: all end alias :visit_Set :visit_Array + def visit_Arel_Nodes_Comment(o) + visit_edge(o, "values") + end + def visit_edge(o, method) edge(method) { visit o.send(method) } end diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb index 1630226085..4192d9efdc 100644 --- a/activerecord/lib/arel/visitors/to_sql.rb +++ b/activerecord/lib/arel/visitors/to_sql.rb @@ -35,6 +35,7 @@ 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) @@ -47,6 +48,7 @@ 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) @@ -62,9 +64,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) @@ -162,7 +164,7 @@ module Arel # :nodoc: all collect_nodes_for o.havings, collector, " HAVING ", " AND " collect_nodes_for o.windows, collector, " WINDOW " - collector + maybe_visit o.comment, collector end def visit_Arel_Nodes_OptimizerHints(o, collector) @@ -170,6 +172,10 @@ module Arel # :nodoc: all collector << "/*+ #{hints} */" end + def visit_Arel_Nodes_Comment(o, collector) + collector << o.values.map { |v| "/* #{sanitize_as_sql_comment(v)} */" }.join(" ") + end + def collect_nodes_for(nodes, collector, spacer, connector = ", ") unless nodes.empty? collector << spacer diff --git a/activerecord/test/cases/adapters/mysql2/annotate_test.rb b/activerecord/test/cases/adapters/mysql2/annotate_test.rb new file mode 100644 index 0000000000..b512540073 --- /dev/null +++ b/activerecord/test/cases/adapters/mysql2/annotate_test.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/post" + +class Mysql2AnnotateTest < ActiveRecord::Mysql2TestCase + fixtures :posts + + def test_annotate_wraps_content_in_an_inline_comment + assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do + posts = Post.select(:id).annotate("foo") + assert posts.first + end + end + + def test_annotate_is_sanitized + assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do + posts = Post.select(:id).annotate("*/foo/*") + assert posts.first + end + + assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/}) do + posts = Post.select(:id).annotate("**//foo//**") + assert posts.first + end + + assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* foo \*/ /\* bar \*/}) do + posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") + assert posts.first + end + + assert_sql(%r{\ASELECT `posts`\.`id` FROM `posts` /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do + posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") + assert posts.first + end + end +end diff --git a/activerecord/test/cases/adapters/postgresql/annotate_test.rb b/activerecord/test/cases/adapters/postgresql/annotate_test.rb new file mode 100644 index 0000000000..42a2861511 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/annotate_test.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/post" + +class PostgresqlAnnotateTest < ActiveRecord::PostgreSQLTestCase + fixtures :posts + + def test_annotate_wraps_content_in_an_inline_comment + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do + posts = Post.select(:id).annotate("foo") + assert posts.first + end + end + + def test_annotate_is_sanitized + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do + posts = Post.select(:id).annotate("*/foo/*") + assert posts.first + end + + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do + posts = Post.select(:id).annotate("**//foo//**") + assert posts.first + end + + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/ /\* bar \*/}) do + posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") + assert posts.first + end + + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do + posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") + assert posts.first + end + end +end diff --git a/activerecord/test/cases/adapters/sqlite3/annotate_test.rb b/activerecord/test/cases/adapters/sqlite3/annotate_test.rb new file mode 100644 index 0000000000..6567a5eca3 --- /dev/null +++ b/activerecord/test/cases/adapters/sqlite3/annotate_test.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require "cases/helper" +require "models/post" + +class SQLite3AnnotateTest < ActiveRecord::SQLite3TestCase + fixtures :posts + + def test_annotate_wraps_content_in_an_inline_comment + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do + posts = Post.select(:id).annotate("foo") + assert posts.first + end + end + + def test_annotate_is_sanitized + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do + posts = Post.select(:id).annotate("*/foo/*") + assert posts.first + end + + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/}) do + posts = Post.select(:id).annotate("**//foo//**") + assert posts.first + end + + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* foo \*/ /\* bar \*/}) do + posts = Post.select(:id).annotate("*/foo/*").annotate("*/bar") + assert posts.first + end + + assert_sql(%r{\ASELECT "posts"\."id" FROM "posts" /\* \+ MAX_EXECUTION_TIME\(1\) \*/}) do + posts = Post.select(:id).annotate("+ MAX_EXECUTION_TIME(1)") + assert posts.first + end + end +end diff --git a/activerecord/test/cases/arel/delete_manager_test.rb b/activerecord/test/cases/arel/delete_manager_test.rb index 0bad02f4d2..63cd1bffe3 100644 --- a/activerecord/test/cases/arel/delete_manager_test.rb +++ b/activerecord/test/cases/arel/delete_manager_test.rb @@ -49,5 +49,23 @@ 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/comment_test.rb b/activerecord/test/cases/arel/nodes/comment_test.rb new file mode 100644 index 0000000000..bf5eaf4c5a --- /dev/null +++ b/activerecord/test/cases/arel/nodes/comment_test.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require_relative "../helper" +require "yaml" + +module Arel + module Nodes + class CommentTest < Arel::Spec + describe "equality" do + it "is equal with equal contents" do + array = [Comment.new(["foo"]), Comment.new(["foo"])] + assert_equal 1, array.uniq.size + end + + it "is not equal with different contents" do + array = [Comment.new(["foo"]), Comment.new(["bar"])] + assert_equal 2, array.uniq.size + end + 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 3f078063a4..8ba268653d 100644 --- a/activerecord/test/cases/arel/nodes/delete_statement_test.rb +++ b/activerecord/test/cases/arel/nodes/delete_statement_test.rb @@ -18,8 +18,10 @@ 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 @@ -27,8 +29,14 @@ 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 252a0d0d0b..036576b231 100644 --- a/activerecord/test/cases/arel/nodes/insert_statement_test.rb +++ b/activerecord/test/cases/arel/nodes/insert_statement_test.rb @@ -23,9 +23,11 @@ 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 @@ -34,9 +36,15 @@ 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/select_core_test.rb b/activerecord/test/cases/arel/nodes/select_core_test.rb index 0b698205ff..6860f2a395 100644 --- a/activerecord/test/cases/arel/nodes/select_core_test.rb +++ b/activerecord/test/cases/arel/nodes/select_core_test.rb @@ -37,6 +37,7 @@ module Arel core1.groups = %w[j k l] core1.windows = %w[m n o] core1.havings = %w[p q r] + core1.comment = Arel::Nodes::Comment.new(["comment"]) core2 = SelectCore.new core2.froms = %w[a b c] core2.projections = %w[d e f] @@ -44,6 +45,7 @@ module Arel core2.groups = %w[j k l] core2.windows = %w[m n o] core2.havings = %w[p q r] + core2.comment = Arel::Nodes::Comment.new(["comment"]) array = [core1, core2] assert_equal 1, array.uniq.size end @@ -56,6 +58,7 @@ module Arel core1.groups = %w[j k l] core1.windows = %w[m n o] core1.havings = %w[p q r] + core1.comment = Arel::Nodes::Comment.new(["comment"]) core2 = SelectCore.new core2.froms = %w[a b c] core2.projections = %w[d e f] @@ -63,6 +66,11 @@ module Arel core2.groups = %w[j k l] core2.windows = %w[m n o] core2.havings = %w[l o l] + core2.comment = Arel::Nodes::Comment.new(["comment"]) + array = [core1, core2] + assert_equal 2, array.uniq.size + core2.havings = %w[p q r] + core2.comment = Arel::Nodes::Comment.new(["other"]) array = [core1, core2] 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 a83ce32f68..f133ddf7eb 100644 --- a/activerecord/test/cases/arel/nodes/update_statement_test.rb +++ b/activerecord/test/cases/arel/nodes/update_statement_test.rb @@ -27,6 +27,7 @@ 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 @@ -34,6 +35,7 @@ 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 @@ -46,6 +48,7 @@ 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 @@ -53,6 +56,11 @@ 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/select_manager_test.rb b/activerecord/test/cases/arel/select_manager_test.rb index 5220950905..e6c49cd429 100644 --- a/activerecord/test/cases/arel/select_manager_test.rb +++ b/activerecord/test/cases/arel/select_manager_test.rb @@ -1221,5 +1221,28 @@ module Arel manager.distinct_on(false).must_equal manager end end + + describe "comment" do + it "chains" do + manager = Arel::SelectManager.new + manager.comment("selecting").must_equal manager + end + + it "appends a comment to the generated query" do + manager = Arel::SelectManager.new + table = Table.new :users + manager.from(table).project(table["id"]) + + manager.comment("selecting") + manager.to_sql.must_be_like %{ + SELECT "users"."id" FROM "users" /* selecting */ + } + + manager.comment("selecting", "with", "comment") + manager.to_sql.must_be_like %{ + SELECT "users"."id" FROM "users" /* selecting */ /* with */ /* comment */ + } + end + end end end diff --git a/activerecord/test/cases/arel/support/fake_record.rb b/activerecord/test/cases/arel/support/fake_record.rb index 559ff5d4e6..18e6c10c9d 100644 --- a/activerecord/test/cases/arel/support/fake_record.rb +++ b/activerecord/test/cases/arel/support/fake_record.rb @@ -58,6 +58,10 @@ module FakeRecord "\"#{name}\"" end + def sanitize_as_sql_comment(comment) + comment + end + def schema_cache self end diff --git a/activerecord/test/cases/arel/update_manager_test.rb b/activerecord/test/cases/arel/update_manager_test.rb index cc1b9ac5b3..e13cb6aa52 100644 --- a/activerecord/test/cases/arel/update_manager_test.rb +++ b/activerecord/test/cases/arel/update_manager_test.rb @@ -122,5 +122,29 @@ 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/arel/visitors/depth_first_test.rb b/activerecord/test/cases/arel/visitors/depth_first_test.rb index 4a57608411..106be2311d 100644 --- a/activerecord/test/cases/arel/visitors/depth_first_test.rb +++ b/activerecord/test/cases/arel/visitors/depth_first_test.rb @@ -101,6 +101,12 @@ module Arel assert_equal [:a, :b, join], @collector.calls end + def test_comment + comment = Nodes::Comment.new ["foo"] + @visitor.accept comment + assert_equal ["foo", ["foo"], comment], @collector.calls + end + [ Arel::Nodes::Assignment, Arel::Nodes::Between, diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 081da95df7..84130ec208 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -21,6 +21,11 @@ require "models/molecule" require "models/electron" require "models/man" require "models/interest" +require "models/pirate" +require "models/parrot" +require "models/bird" +require "models/treasure" +require "models/price_estimate" class AssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :developers_projects, @@ -368,3 +373,97 @@ class GeneratedMethodsTest < ActiveRecord::TestCase assert_equal :none, MyArticle.new.comments end end + +class WithAnnotationsTest < ActiveRecord::TestCase + fixtures :pirates, :parrots + + def test_belongs_to_with_annotation_includes_a_query_comment + pirate = SpacePirate.where.not(parrot_id: nil).first + assert pirate, "should have a Pirate record" + + log = capture_sql do + pirate.parrot + end + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? + + assert_sql(%r{/\* that tells jokes \*/}) do + pirate.parrot_with_annotation + end + end + + def test_has_and_belongs_to_many_with_annotation_includes_a_query_comment + pirate = SpacePirate.first + assert pirate, "should have a Pirate record" + + log = capture_sql do + pirate.parrots.first + end + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? + + assert_sql(%r{/\* that are very colorful \*/}) do + pirate.parrots_with_annotation.first + end + end + + def test_has_one_with_annotation_includes_a_query_comment + pirate = SpacePirate.first + assert pirate, "should have a Pirate record" + + log = capture_sql do + pirate.ship + end + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? + + assert_sql(%r{/\* that is a rocket \*/}) do + pirate.ship_with_annotation + end + end + + def test_has_many_with_annotation_includes_a_query_comment + pirate = SpacePirate.first + assert pirate, "should have a Pirate record" + + log = capture_sql do + pirate.birds.first + end + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? + + assert_sql(%r{/\* that are also parrots \*/}) do + pirate.birds_with_annotation.first + end + end + + def test_has_many_through_with_annotation_includes_a_query_comment + pirate = SpacePirate.first + assert pirate, "should have a Pirate record" + + log = capture_sql do + pirate.treasure_estimates.first + end + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? + + assert_sql(%r{/\* yarrr \*/}) do + pirate.treasure_estimates_with_annotation.first + end + end + + def test_has_many_through_with_annotation_includes_a_query_comment_when_eager_loading + pirate = SpacePirate.first + assert pirate, "should have a Pirate record" + + log = capture_sql do + pirate.treasure_estimates.first + end + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? + + assert_sql(%r{/\* yarrr \*/}) do + SpacePirate.includes(:treasure_estimates_with_annotation, :treasures).first + end + end +end diff --git a/activerecord/test/cases/relation/delete_all_test.rb b/activerecord/test/cases/relation/delete_all_test.rb index d1c13fa1b5..9b76936b7e 100644 --- a/activerecord/test/cases/relation/delete_all_test.rb +++ b/activerecord/test/cases/relation/delete_all_test.rb @@ -99,4 +99,23 @@ 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/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 224e4f39a8..5c5e760e34 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -135,6 +135,18 @@ class RelationMergingTest < ActiveRecord::TestCase relation = Post.all.merge(Post.order(Arel.sql("title LIKE '%?'"))) assert_equal ["title LIKE '%?'"], relation.order_values end + + def test_merging_annotations_respects_merge_order + assert_sql(%r{/\* foo \*/ /\* bar \*/}) do + Post.annotate("foo").merge(Post.annotate("bar")).first + end + assert_sql(%r{/\* bar \*/ /\* foo \*/}) do + Post.annotate("bar").merge(Post.annotate("foo")).first + end + assert_sql(%r{/\* foo \*/ /\* bar \*/ /\* baz \*/ /\* qux \*/}) do + Post.annotate("foo").annotate("bar").merge(Post.annotate("baz").annotate("qux")).first + end + end end class MergingDifferentRelationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relation/update_all_test.rb b/activerecord/test/cases/relation/update_all_test.rb index 0500574f28..526f926841 100644 --- a/activerecord/test/cases/relation/update_all_test.rb +++ b/activerecord/test/cases/relation/update_all_test.rb @@ -241,6 +241,33 @@ 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 diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 68161f6a84..00a7b3841f 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -307,6 +307,58 @@ module ActiveRecord assert_equal 3, ratings.count end + def test_relation_with_annotation_includes_comment_in_to_sql + post_with_annotation = Post.where(id: 1).annotate("foo") + assert_match %r{= 1 /\* foo \*/}, post_with_annotation.to_sql + end + + def test_relation_with_annotation_includes_comment_in_sql + post_with_annotation = Post.where(id: 1).annotate("foo") + assert_sql(%r{/\* foo \*/}) do + assert post_with_annotation.first, "record should be found" + end + end + + def test_relation_with_annotation_chains_sql_comments + post_with_annotation = Post.where(id: 1).annotate("foo").annotate("bar") + assert_sql(%r{/\* foo \*/ /\* bar \*/}) do + assert post_with_annotation.first, "record should be found" + end + end + + def test_relation_with_annotation_filters_sql_comment_delimiters + post_with_annotation = Post.where(id: 1).annotate("**//foo//**") + assert_match %r{= 1 /\* foo \*/}, post_with_annotation.to_sql + end + + def test_relation_with_annotation_includes_comment_in_count_query + post_with_annotation = Post.annotate("foo") + all_count = Post.all.to_a.count + assert_sql(%r{/\* foo \*/}) do + assert_equal all_count, post_with_annotation.count + end + end + + def test_relation_without_annotation_does_not_include_an_empty_comment + log = capture_sql do + Post.where(id: 1).first + end + + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\*}) }, :empty? + end + + def test_relation_with_optimizer_hints_filters_sql_comment_delimiters + post_with_hint = Post.where(id: 1).optimizer_hints("**//BADHINT//**") + assert_match %r{BADHINT}, post_with_hint.to_sql + assert_no_match %r{\*/BADHINT}, post_with_hint.to_sql + assert_no_match %r{\*//BADHINT}, post_with_hint.to_sql + assert_no_match %r{BADHINT/\*}, post_with_hint.to_sql + assert_no_match %r{BADHINT//\*}, post_with_hint.to_sql + post_with_hint = Post.where(id: 1).optimizer_hints("/*+ BADHINT */") + assert_match %r{/\*\+ BADHINT \*/}, post_with_hint.to_sql + end + class EnsureRoundTripTypeCasting < ActiveRecord::Type::Value def type :string diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb index 8b08e40468..3488442cab 100644 --- a/activerecord/test/cases/scoping/named_scoping_test.rb +++ b/activerecord/test/cases/scoping/named_scoping_test.rb @@ -602,4 +602,14 @@ class NamedScopingTest < ActiveRecord::TestCase Topic.create! assert_predicate Topic, :one? end + + def test_scope_with_annotation + Topic.class_eval do + scope :including_annotate_in_scope, Proc.new { annotate("from-scope") } + end + + assert_sql(%r{/\* from-scope \*/}) do + assert Topic.including_annotate_in_scope.to_a, Topic.all.to_a + end + end end diff --git a/activerecord/test/cases/scoping/relation_scoping_test.rb b/activerecord/test/cases/scoping/relation_scoping_test.rb index b1f2ffe29c..a95ab0f429 100644 --- a/activerecord/test/cases/scoping/relation_scoping_test.rb +++ b/activerecord/test/cases/scoping/relation_scoping_test.rb @@ -130,6 +130,44 @@ class RelationScopingTest < ActiveRecord::TestCase end end + def test_scoped_find_with_annotation + Developer.annotate("scoped").scoping do + developer = nil + assert_sql(%r{/\* scoped \*/}) do + developer = Developer.where("name = 'David'").first + end + assert_equal "David", developer.name + end + end + + def test_find_with_annotation_unscoped + Developer.annotate("scoped").unscoped do + developer = nil + log = capture_sql do + developer = Developer.where("name = 'David'").first + end + + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\* scoped \*/}) }, :empty? + + assert_equal "David", developer.name + end + end + + def test_find_with_annotation_unscope + developer = nil + log = capture_sql do + developer = Developer.annotate("unscope"). + where("name = 'David'"). + unscope(:annotate).first + end + + assert_not_predicate log, :empty? + assert_predicate log.select { |query| query.match?(%r{/\* unscope \*/}) }, :empty? + + assert_equal "David", developer.name + end + def test_scoped_find_include # with the include, will retrieve only developers for the given project scoped_developers = Developer.includes(:projects).scoping do diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index fd5083e597..8733398697 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -98,3 +98,19 @@ class FamousPirate < ActiveRecord::Base has_many :famous_ships validates_presence_of :catchphrase, on: :conference end + +class SpacePirate < ActiveRecord::Base + self.table_name = "pirates" + + belongs_to :parrot + belongs_to :parrot_with_annotation, -> { annotate("that tells jokes") }, class_name: :Parrot, foreign_key: :parrot_id + has_and_belongs_to_many :parrots, foreign_key: :pirate_id + has_and_belongs_to_many :parrots_with_annotation, -> { annotate("that are very colorful") }, class_name: :Parrot, foreign_key: :pirate_id + has_one :ship, foreign_key: :pirate_id + has_one :ship_with_annotation, -> { annotate("that is a rocket") }, class_name: :Ship, foreign_key: :pirate_id + has_many :birds, foreign_key: :pirate_id + has_many :birds_with_annotation, -> { annotate("that are also parrots") }, class_name: :Bird, foreign_key: :pirate_id + has_many :treasures, as: :looter + has_many :treasure_estimates, through: :treasures, source: :price_estimates + has_many :treasure_estimates_with_annotation, -> { annotate("yarrr") }, through: :treasures, source: :price_estimates +end |