Add order to BindParams in the ToSql collector

This removes the need for us to do the re-ordering by walking the AST in
ActiveRecord. We're using a block to communicate with the collector,
since the collector needs to be the thing which knows about the index,
while the visitor is the thing that needs to know the syntax. The
BindParam needs to know about neither of these things, so it's been
changed to stop being a subclass of SqlLiteral

I could also see an alternative implementation using format strings if
for some reason blocks cause a problem.

2 files changed, 7 insertions, 2 deletions

diff --git a/lib/arel/visitors/postgresql.rb b/lib/arel/visitors/postgresql.rb
index 60878ddd20..f55aaf30fe 100644
--- a/lib/arel/visitors/postgresql.rb
+++ b/lib/arel/visitors/postgresql.rb
@@ -23,6 +23,10 @@ module Arel
         collector << "DISTINCT ON ( "
         visit(o.expr, collector) << " )"
       end
+
+      def visit_Arel_Nodes_BindParam o, collector
+        collector.add_bind(o) { |i| "$#{i}" }
+      end
     end
   end
 end
diff --git a/lib/arel/visitors/to_sql.rb b/lib/arel/visitors/to_sql.rb
index a3f8cb565d..884076d987 100644
--- a/lib/arel/visitors/to_sql.rb
+++ b/lib/arel/visitors/to_sql.rb
@@ -186,7 +186,8 @@ module Arel
 
         len = o.expressions.length - 1
         o.expressions.zip(o.columns).each_with_index { |(value, attr), i|
-          if Nodes::SqlLiteral === value
+          case value
+          when Nodes::SqlLiteral, Nodes::BindParam
             collector = visit value, collector
           else
             collector << quote(value, attr && column_for(attr)).to_s
@@ -713,7 +714,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) { "?" }
       end
 
       alias :visit_Arel_Nodes_SqlLiteral :literal