Refactor `substitute_binds` to perform substitution immediately

I'm honestly not sure if replacing bind params with their concrete
values is something that belongs in Arel at all, as it's something that
will need to be coupled to the quoting mechanism of the caller, and
could just be accomplished by using `Quoted` instead.

Still, with the new structure we can provide a much simpler API around
substitution. The expectation of the quoter responding to `quote` is a
reasonably minimal API.

I originally used `DelegateClass` here, with the one line override of
`add_bind`, but realized that we have some funky code going on where the
collector returns the next collector to use (in practice `self` is
always returned, and I don't see why we'd ever want to do this).
Removing that would likely be worthwhile, but would be a larger
refactoring

5 files changed, 14 insertions, 23 deletions

diff --git a/lib/arel/collectors/substitute_binds.rb b/lib/arel/collectors/substitute_binds.rb
index 62589c44e8..99d2215aaa 100644
--- a/lib/arel/collectors/substitute_binds.rb
+++ b/lib/arel/collectors/substitute_binds.rb
@@ -2,36 +2,27 @@
 module Arel
   module Collectors
     class SubstituteBinds
-      def initialize
-        @parts = []
+      def initialize(quoter, delegate_collector)
+        @quoter = quoter
+        @delegate = delegate_collector
       end
 
       def << str
-        @parts << str
+        delegate << str
         self
       end
 
       def add_bind bind
-        @parts << bind
-        self
+        self << quoter.quote(bind)
       end
 
-      def value; @parts; end
-
-      def substitute_binds bvs
-        bvs = bvs.dup
-        @parts.map do |val|
-          if Arel::Nodes::BindParam === val
-            bvs.shift
-          else
-            val
-          end
-        end
+      def value
+        delegate.value
       end
 
-      def compile bvs
-        substitute_binds(bvs).join
-      end
+      protected
+
+      attr_reader :quoter, :delegate
     end
   end
 end
diff --git a/lib/arel/visitors/oracle.rb b/lib/arel/visitors/oracle.rb
index 3b452836db..c1e41be66f 100644
--- a/lib/arel/visitors/oracle.rb
+++ b/lib/arel/visitors/oracle.rb
@@ -145,7 +145,7 @@ module Arel
       end
 
       def visit_Arel_Nodes_BindParam o, collector
-        collector.add_bind(o) { |i| ":a#{i}" }
+        collector.add_bind(o.value) { |i| ":a#{i}" }
       end
 
     end
diff --git a/lib/arel/visitors/oracle12.rb b/lib/arel/visitors/oracle12.rb
index ce90e994ae..648047ae61 100644
--- a/lib/arel/visitors/oracle12.rb
+++ b/lib/arel/visitors/oracle12.rb
@@ -53,7 +53,7 @@ module Arel
       end
 
       def visit_Arel_Nodes_BindParam o, collector
-        collector.add_bind(o) { |i| ":a#{i}" }
+        collector.add_bind(o.value) { |i| ":a#{i}" }
       end
     end
   end
diff --git a/lib/arel/visitors/postgresql.rb b/lib/arel/visitors/postgresql.rb
index f0991a2f11..2a935e4318 100644
--- a/lib/arel/visitors/postgresql.rb
+++ b/lib/arel/visitors/postgresql.rb
@@ -46,7 +46,7 @@ module Arel
       end
 
       def visit_Arel_Nodes_BindParam o, collector
-        collector.add_bind(o) { |i| "$#{i}" }
+        collector.add_bind(o.value) { |i| "$#{i}" }
       end
 
       def visit_Arel_Nodes_GroupingElement o, collector
diff --git a/lib/arel/visitors/to_sql.rb b/lib/arel/visitors/to_sql.rb
index 3f1e390dcc..3ed80c8f06 100644
--- a/lib/arel/visitors/to_sql.rb
+++ b/lib/arel/visitors/to_sql.rb
@@ -737,7 +737,7 @@ module Arel
       def literal o, collector; collector << o.to_s; end
 
       def visit_Arel_Nodes_BindParam o, collector
-        collector.add_bind(o) { "?" }
+        collector.add_bind(o.value) { "?" }
       end
 
       alias :visit_Arel_Nodes_SqlLiteral :literal