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.

3 files changed, 21 insertions, 4 deletions

diff --git a/test/visitors/test_bind_visitor.rb b/test/visitors/test_bind_visitor.rb
index 333636ed51..79d340e5cd 100644
--- a/test/visitors/test_bind_visitor.rb
+++ b/test/visitors/test_bind_visitor.rb
@@ -18,7 +18,7 @@ module Arel
       def test_assignment_binds_are_substituted
         table = Table.new(:users)
         um = Arel::UpdateManager.new Table.engine
-        bp = Nodes::BindParam.new '?'
+        bp = Nodes::BindParam.new
         um.set [[table[:name], bp]]
         visitor = Class.new(Arel::Visitors::ToSql) {
           include Arel::Visitors::BindVisitor
@@ -38,7 +38,7 @@ module Arel
           include Arel::Visitors::BindVisitor
         }.new nil
 
-        bp = Nodes::BindParam.new 'omg'
+        bp = Nodes::BindParam.new
         called = false
         visitor.accept(bp, collector) { called = true }
         assert called
diff --git a/test/visitors/test_postgres.rb b/test/visitors/test_postgres.rb
index 3d646a7324..368feb5977 100644
--- a/test/visitors/test_postgres.rb
+++ b/test/visitors/test_postgres.rb
@@ -117,6 +117,16 @@ module Arel
           }
         end
       end
+
+      describe "Nodes::BindParam" do
+        it "increments each bind param" do
+          query = @table[:name].eq(Arel::Nodes::BindParam.new)
+            .and(@table[:id].eq(Arel::Nodes::BindParam.new))
+          compile(query).must_be_like %{
+            "users"."name" = $1 AND "users"."id" = $2
+          }
+        end
+      end
     end
   end
 end
diff --git a/test/visitors/test_to_sql.rb b/test/visitors/test_to_sql.rb
index 9c18d74827..7895866809 100644
--- a/test/visitors/test_to_sql.rb
+++ b/test/visitors/test_to_sql.rb
@@ -16,9 +16,16 @@ module Arel
       end
 
       it 'works with BindParams' do
-        node = Nodes::BindParam.new 'omg'
+        node = Nodes::BindParam.new
         sql = compile node
-        sql.must_be_like 'omg'
+        sql.must_be_like '?'
+      end
+
+      it 'does not quote BindParams used as part of a Values' do
+        bp = Nodes::BindParam.new
+        values = Nodes::Values.new([bp])
+        sql = compile values
+        sql.must_be_like 'VALUES (?)'
       end
 
       it 'can define a dispatch method' do