Refactor `preparable` to be had the state in Arel collectors

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

@@ -14,31 +14,32 @@ def to_sql(arel_or_sql_string, binds = [])
         sql
       end
 
-      def to_sql_and_binds(arel_or_sql_string, binds = []) # :nodoc:
+      def to_sql_and_binds(arel_or_sql_string, binds = [], preparable = nil) # :nodoc:
         if arel_or_sql_string.respond_to?(:ast)
           unless binds.empty?
             raise "Passing bind parameters with an arel AST is forbidden. " \
               "The values must be stored on the AST directly"
           end
 
+          collector = collector()
+
           if prepared_statements
+            collector.preparable = true
             sql, binds = visitor.compile(arel_or_sql_string.ast, collector)
 
             if binds.length > bind_params_length
               unprepared_statement do
-                sql, binds = to_sql_and_binds(arel_or_sql_string)
-                visitor.preparable = false
+                return to_sql_and_binds(arel_or_sql_string)
               end
             end
+            preparable = collector.preparable
           else
             sql = visitor.compile(arel_or_sql_string.ast, collector)
           end
-          [sql.freeze, binds]
+          [sql.freeze, binds, preparable]
         else
-          visitor.preparable = false if prepared_statements
-
           arel_or_sql_string = arel_or_sql_string.dup.freeze unless arel_or_sql_string.frozen?
-          [arel_or_sql_string, binds]
+          [arel_or_sql_string, binds, preparable]
         end
       end
       private :to_sql_and_binds
@@ -60,11 +61,7 @@ def cacheable_query(klass, arel) # :nodoc:
       # Returns an ActiveRecord::Result instance.
       def select_all(arel, name = nil, binds = [], preparable: nil)
         arel = arel_from_relation(arel)
-        sql, binds = to_sql_and_binds(arel, binds)
-
-        if preparable.nil?
-          preparable = prepared_statements ? visitor.preparable : false
-        end
+        sql, binds, preparable = to_sql_and_binds(arel, binds, preparable)
 
         if prepared_statements && preparable
           select_prepared(sql, name, binds)

activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb

@@ -96,11 +96,7 @@ def clear_query_cache
       def select_all(arel, name = nil, binds = [], preparable: nil)
         if @query_cache_enabled && !locked?(arel)
           arel = arel_from_relation(arel)
-          sql, binds = to_sql_and_binds(arel, binds)
-
-          if preparable.nil?
-            preparable = prepared_statements ? visitor.preparable : false
-          end
+          sql, binds, preparable = to_sql_and_binds(arel, binds, preparable)
 
           cache_sql(sql, name, binds) { super(sql, name, binds, preparable: preparable) }
         else

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

 # frozen_string_literal: true
 
 require "set"
-require "active_record/connection_adapters/determine_if_preparable_visitor"
 require "active_record/connection_adapters/schema_cache"
 require "active_record/connection_adapters/sql_type_metadata"
 require "active_record/connection_adapters/abstract/schema_dumper"
@@ -95,12 +94,9 @@ def initialize(connection, logger = nil, config = {}) # :nodoc:
         @statements = build_statement_pool
         @lock = ActiveSupport::Concurrency::LoadInterlockAwareMonitor.new
 
-        if self.class.type_cast_config_to_boolean(config.fetch(:prepared_statements) { true })
-          @prepared_statements = true
-          @visitor.extend(DetermineIfPreparableVisitor)
-        else
-          @prepared_statements = false
-        end
+        @prepared_statements = self.class.type_cast_config_to_boolean(
+          config.fetch(:prepared_statements, true)
+        )
 
         @advisory_locks_enabled = self.class.type_cast_config_to_boolean(
           config.fetch(:advisory_locks, true)

activerecord/lib/active_record/connection_adapters/determine_if_preparable_visitor.rb

-# frozen_string_literal: true
-
-module ActiveRecord
-  module ConnectionAdapters
-    module DetermineIfPreparableVisitor
-      attr_accessor :preparable
-
-      def accept(object, collector)
-        @preparable = true
-        super
-      end
-
-      def visit_Arel_Nodes_In(o, collector)
-        @preparable = false
-        super
-      end
-
-      def visit_Arel_Nodes_NotIn(o, collector)
-        @preparable = false
-        super
-      end
-
-      def visit_Arel_Nodes_SqlLiteral(o, collector)
-        @preparable = false
-        super
-      end
-    end
-  end
-end

activerecord/lib/active_record/statement_cache.rb

@@ -57,6 +57,8 @@ def sql_for(binds, connection)
     end
 
     class PartialQueryCollector
+      attr_accessor :preparable
+
       def initialize
         @parts = []
         @binds = []

activerecord/lib/arel/collectors/composite.rb

@@ -3,6 +3,8 @@
 module Arel # :nodoc: all
   module Collectors
     class Composite
+      attr_accessor :preparable
+
       def initialize(left, right)
         @left = left
         @right = right

activerecord/lib/arel/collectors/sql_string.rb

@@ -5,6 +5,8 @@
 module Arel # :nodoc: all
   module Collectors
     class SQLString < PlainString
+      attr_accessor :preparable
+
       def initialize(*)
         super
         @bind_index = 1

activerecord/lib/arel/collectors/substitute_binds.rb

@@ -3,6 +3,8 @@
 module Arel # :nodoc: all
   module Collectors
     class SubstituteBinds
+      attr_accessor :preparable
+
       def initialize(quoter, delegate_collector)
         @quoter = quoter
         @delegate = delegate_collector

activerecord/lib/arel/visitors/to_sql.rb

@@ -510,6 +510,7 @@ def visit_Arel_Table(o, collector)
         end
 
         def visit_Arel_Nodes_In(o, collector)
+          collector.preparable = false
           attr, values = o.left, o.right
 
           if Array === values
@@ -525,6 +526,7 @@ def visit_Arel_Nodes_In(o, collector)
         end
 
         def visit_Arel_Nodes_NotIn(o, collector)
+          collector.preparable = false
           attr, values = o.left, o.right
 
           if Array === values
@@ -656,14 +658,18 @@ def visit_Arel_Attributes_Attribute(o, collector)
           collector << quote_table_name(join_name) << "." << quote_column_name(o.name)
         end
 
-        def literal(o, collector); collector << o.to_s; end
-
         def visit_Arel_Nodes_BindParam(o, collector)
           collector.add_bind(o.value) { "?" }
         end
 
-        alias :visit_Arel_Nodes_SqlLiteral :literal
-        alias :visit_Integer               :literal
+        def visit_Arel_Nodes_SqlLiteral(o, collector)
+          collector.preparable = false
+          collector << o.to_s
+        end
+
+        def visit_Integer(o, collector)
+          collector << o.to_s
+        end
 
         def unsupported(o, collector)
           raise UnsupportedVisitError.new(o)