From f41825809cf3d27f98467bd7bc005e5d6ed94828 Mon Sep 17 00:00:00 2001 From: Matt Yoho Date: Thu, 14 Mar 2019 12:57:14 -0700 Subject: Add Relation#annotate for SQL commenting This patch has two main portions: 1. Add SQL comment support to Arel via Arel::Nodes::Comment. 2. Implement a Relation#annotate method on top of that. == Adding SQL comment support Adds a new Arel::Nodes::Comment node that represents an optional SQL comment and teachers the relevant visitors how to handle it. Comment nodes may be added to the basic CRUD statement nodes and set through any of the four (Select|Insert|Update|Delete)Manager objects. For example: manager = Arel::UpdateManager.new manager.table table manager.comment("annotation") manager.to_sql # UPDATE "users" /* annotation */ This new node type will be used by ActiveRecord::Relation to enable query annotation via SQL comments. == Implementing the Relation#annotate method Implements `ActiveRecord::Relation#annotate`, which accepts a comment string that will be appeneded to any queries generated by the relation. Some examples: relation = Post.where(id: 123).annotate("metadata string") relation.first # SELECT "posts".* FROM "posts" WHERE "posts"."id" = 123 # LIMIT 1 /* metadata string */ class Tag < ActiveRecord::Base scope :foo_annotated, -> { annotate("foo") } end Tag.foo_annotated.annotate("bar").first # SELECT "tags".* FROM "tags" LIMIT 1 /* foo */ /* bar */ Also wires up the plumbing so this works with `#update_all` and `#delete_all` as well. This feature is useful for instrumentation and general analysis of queries generated at runtime. --- activerecord/CHANGELOG.md | 13 +++ .../associations/preloader/through_association.rb | 6 +- activerecord/lib/active_record/querying.rb | 2 +- activerecord/lib/active_record/relation.rb | 5 +- .../lib/active_record/relation/query_methods.rb | 23 ++++- activerecord/lib/arel/nodes.rb | 2 + activerecord/lib/arel/nodes/comment.rb | 29 +++++++ activerecord/lib/arel/nodes/delete_statement.rb | 9 +- activerecord/lib/arel/nodes/insert_statement.rb | 9 +- activerecord/lib/arel/nodes/select_core.rb | 9 +- activerecord/lib/arel/nodes/update_statement.rb | 9 +- activerecord/lib/arel/select_manager.rb | 9 ++ activerecord/lib/arel/tree_manager.rb | 5 ++ activerecord/lib/arel/visitors/depth_first.rb | 4 + activerecord/lib/arel/visitors/dot.rb | 4 + activerecord/lib/arel/visitors/to_sql.rb | 12 ++- .../test/cases/adapters/mysql2/annotate_test.rb | 37 ++++++++ .../cases/adapters/postgresql/annotate_test.rb | 37 ++++++++ .../test/cases/adapters/sqlite3/annotate_test.rb | 37 ++++++++ .../test/cases/arel/delete_manager_test.rb | 18 ++++ activerecord/test/cases/arel/nodes/comment_test.rb | 22 +++++ .../test/cases/arel/nodes/delete_statement_test.rb | 8 ++ .../test/cases/arel/nodes/insert_statement_test.rb | 8 ++ .../test/cases/arel/nodes/select_core_test.rb | 8 ++ .../test/cases/arel/nodes/update_statement_test.rb | 8 ++ .../test/cases/arel/select_manager_test.rb | 23 +++++ .../test/cases/arel/support/fake_record.rb | 4 + .../test/cases/arel/update_manager_test.rb | 24 ++++++ .../test/cases/arel/visitors/depth_first_test.rb | 6 ++ activerecord/test/cases/associations_test.rb | 99 ++++++++++++++++++++++ .../test/cases/relation/delete_all_test.rb | 19 +++++ activerecord/test/cases/relation/merging_test.rb | 12 +++ .../test/cases/relation/update_all_test.rb | 27 ++++++ activerecord/test/cases/relation_test.rb | 52 ++++++++++++ .../test/cases/scoping/named_scoping_test.rb | 10 +++ .../test/cases/scoping/relation_scoping_test.rb | 38 +++++++++ activerecord/test/models/pirate.rb | 16 ++++ 37 files changed, 644 insertions(+), 19 deletions(-) create mode 100644 activerecord/lib/arel/nodes/comment.rb create mode 100644 activerecord/test/cases/adapters/mysql2/annotate_test.rb create mode 100644 activerecord/test/cases/adapters/postgresql/annotate_test.rb create mode 100644 activerecord/test/cases/adapters/sqlite3/annotate_test.rb create mode 100644 activerecord/test/cases/arel/nodes/comment_test.rb (limited to 'activerecord') 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 -- cgit v1.2.3