Merge pull request #36483 from kamipo/no_allocation_to_sql_visit

No allocation `Arel::Visitors::ToSql#visit`

4 files changed, 34 insertions, 33 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb b/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb
index 1df4dea2d8..97d74df529 100644
--- a/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb
+++ b/activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb
@@ -5,7 +5,7 @@ module ActiveRecord
     module DetermineIfPreparableVisitor
       attr_accessor :preparable
 
-      def accept(*)
+      def accept(object, collector)
         @preparable = true
         super
       end
@@ -20,7 +20,7 @@ module ActiveRecord
         super
       end
 
-      def visit_Arel_Nodes_SqlLiteral(*)
+      def visit_Arel_Nodes_SqlLiteral(o, collector)
         @preparable = false
         super
       end
diff --git a/activerecord/lib/arel/visitors/depth_first.rb b/activerecord/lib/arel/visitors/depth_first.rb
index ca11d9c879..98c3f92cf1 100644
--- a/activerecord/lib/arel/visitors/depth_first.rb
+++ b/activerecord/lib/arel/visitors/depth_first.rb
@@ -9,7 +9,7 @@ module Arel # :nodoc: all
       end
 
       private
-        def visit(o)
+        def visit(o, _ = nil)
           super
           @block.call o
         end
diff --git a/activerecord/lib/arel/visitors/to_sql.rb b/activerecord/lib/arel/visitors/to_sql.rb
index a67a660fed..a1aa620f9d 100644
--- a/activerecord/lib/arel/visitors/to_sql.rb
+++ b/activerecord/lib/arel/visitors/to_sql.rb
@@ -51,10 +51,14 @@ module Arel # :nodoc: all
         def visit_Arel_Nodes_InsertStatement(o, collector)
           collector << "INSERT INTO "
           collector = visit o.relation, collector
-          if o.columns.any?
-            collector << " (#{o.columns.map { |x|
-              quote_column_name x.name
-            }.join ', '})"
+
+          unless o.columns.empty?
+            collector << " ("
+            o.columns.each_with_index do |x, i|
+              collector << ", " unless i == 0
+              collector << quote_column_name(x.name)
+            end
+            collector << ")"
           end
 
           if o.values
@@ -96,22 +100,20 @@ module Arel # :nodoc: all
         def visit_Arel_Nodes_ValuesList(o, collector)
           collector << "VALUES "
 
-          len = o.rows.length - 1
-          o.rows.each_with_index { |row, i|
+          o.rows.each_with_index do |row, i|
+            collector << ", " unless i == 0
             collector << "("
-            row_len = row.length - 1
             row.each_with_index do |value, k|
+              collector << ", " unless k == 0
               case value
               when Nodes::SqlLiteral, Nodes::BindParam
                 collector = visit(value, collector)
               else
                 collector << quote(value).to_s
               end
-              collector << ", " unless k == row_len
             end
             collector << ")"
-            collector << ", " unless i == len
-          }
+          end
           collector
         end
 
@@ -127,11 +129,10 @@ module Arel # :nodoc: all
 
           unless o.orders.empty?
             collector << " ORDER BY "
-            len = o.orders.length - 1
-            o.orders.each_with_index { |x, i|
+            o.orders.each_with_index do |x, i|
+              collector << ", " unless i == 0
               collector = visit(x, collector)
-              collector << ", " unless len == i
-            }
+            end
           end
 
           visit_Arel_Nodes_SelectOptions(o, collector)
@@ -505,7 +506,7 @@ module Arel # :nodoc: all
 
         def visit_Arel_Table(o, collector)
           if o.table_alias
-            collector << "#{quote_table_name o.name} #{quote_table_name o.table_alias}"
+            collector << quote_table_name(o.name) << " " << quote_table_name(o.table_alias)
           else
             collector << quote_table_name(o.name)
           end
@@ -681,13 +682,12 @@ module Arel # :nodoc: all
         end
 
         def visit_Arel_Nodes_UnqualifiedColumn(o, collector)
-          collector << "#{quote_column_name o.name}"
-          collector
+          collector << quote_column_name(o.name)
         end
 
         def visit_Arel_Attributes_Attribute(o, collector)
           join_name = o.relation.table_alias || o.relation.name
-          collector << "#{quote_table_name join_name}.#{quote_column_name o.name}"
+          collector << quote_table_name(join_name) << "." << quote_column_name(o.name)
         end
         alias :visit_Arel_Attributes_Integer :visit_Arel_Attributes_Attribute
         alias :visit_Arel_Attributes_Float :visit_Arel_Attributes_Attribute
@@ -784,14 +784,11 @@ module Arel # :nodoc: all
         end
 
         def inject_join(list, collector, join_str)
-          len = list.length - 1
-          list.each_with_index.inject(collector) { |c, (x, i)|
-            if i == len
-              visit x, c
-            else
-              visit(x, c) << join_str
-            end
-          }
+          list.each_with_index do |x, i|
+            collector << join_str unless i == 0
+            collector = visit(x, collector)
+          end
+          collector
         end
 
         def unboundable?(value)
diff --git a/activerecord/lib/arel/visitors/visitor.rb b/activerecord/lib/arel/visitors/visitor.rb
index 2d5a2b6681..d65ac820bc 100644
--- a/activerecord/lib/arel/visitors/visitor.rb
+++ b/activerecord/lib/arel/visitors/visitor.rb
@@ -7,8 +7,8 @@ module Arel # :nodoc: all
         @dispatch = get_dispatch_cache
       end
 
-      def accept(object, *args)
-        visit object, *args
+      def accept(object, collector = nil)
+        visit object, collector
       end
 
       private
@@ -24,9 +24,13 @@ module Arel # :nodoc: all
           self.class.dispatch_cache
         end
 
-        def visit(object, *args)
+        def visit(object, collector = nil)
           dispatch_method = dispatch[object.class]
-          send dispatch_method, object, *args
+          if collector
+            send dispatch_method, object, collector
+          else
+            send dispatch_method, object
+          end
         rescue NoMethodError => e
           raise e if respond_to?(dispatch_method, true)
           superklass = object.class.ancestors.find { |klass|