Merge pull request #36483 from kamipo/no_allocation_to_sql_visit

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

activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb

@@ -5,7 +5,7 @@ module ConnectionAdapters
     module DetermineIfPreparableVisitor
       attr_accessor :preparable
 
-      def accept(*)
+      def accept(object, collector)
         @preparable = true
         super
       end
@@ -20,7 +20,7 @@ def visit_Arel_Nodes_NotIn(o, collector)
         super
       end
 
-      def visit_Arel_Nodes_SqlLiteral(*)
+      def visit_Arel_Nodes_SqlLiteral(o, collector)
         @preparable = false
         super
       end

activerecord/lib/arel/visitors/depth_first.rb

@@ -10,7 +10,7 @@ def initialize(block = nil)
 
       private
 
-        def visit(o)
+        def visit(o, _ = nil)
           super
           @block.call o
         end

activerecord/lib/arel/visitors/to_sql.rb

@@ -52,10 +52,14 @@ def visit_Arel_Nodes_UpdateStatement(o, collector)
         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
@@ -97,22 +101,20 @@ def visit_Arel_Nodes_False(o, collector)
         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
 
@@ -128,11 +130,10 @@ def visit_Arel_Nodes_SelectStatement(o, collector)
 
           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)
@@ -506,7 +507,7 @@ def visit_Arel_Nodes_Not(o, collector)
 
         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
@@ -682,13 +683,12 @@ def visit_Arel_Nodes_Else(o, collector)
         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
@@ -785,14 +785,11 @@ def maybe_visit(thing, collector)
         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)

activerecord/lib/arel/visitors/visitor.rb

@@ -7,8 +7,8 @@ def initialize
         @dispatch = get_dispatch_cache
       end
 
-      def accept(object, *args)
-        visit object, *args
+      def accept(object, collector = nil)
+        visit object, collector
       end
 
       private
@@ -25,9 +25,13 @@ def get_dispatch_cache
           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|