rm `reorder_bind_params`

Arel handles this for us automatically. Updated tests, as BindParam is
no longer a subclass of SqlLiteral. We should remove the second argument
to substitute_at entirely, as it's no longer used

9 files changed, 7 insertions, 25 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 582dd360f0..a2df76259b 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -265,7 +265,7 @@ module ActiveRecord
       # Returns a bind substitution value given a bind +index+ and +column+
       # NOTE: The column param is currently being used by the sqlserver-adapter
       def substitute_at(column, index = 0)
-        Arel::Nodes::BindParam.new '?'
+        Arel::Nodes::BindParam.new
       end
 
       # REFERENTIAL INTEGRITY ====================================
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
index cf379ab210..d09468329a 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb
@@ -156,10 +156,6 @@ module ActiveRecord
           end
         end
 
-        def substitute_at(column, index = 0)
-          Arel::Nodes::BindParam.new "$#{index + 1}"
-        end
-
         def exec_query(sql, name = 'SQL', binds = [])
           execute_and_clear(sql, name, binds) do |result|
             types = {}
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 03bce4f5b7..3aaf037d31 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -84,7 +84,6 @@ module ActiveRecord
       um = relation
         .arel
         .compile_update(substitutes, @klass.primary_key)
-      reorder_bind_params(um.ast, bvs)
 
       @klass.connection.update(
         um,
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index a686e3263b..cb4e33f1b1 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -881,19 +881,9 @@ module ActiveRecord
       arel.from(build_from) if from_value
       arel.lock(lock_value) if lock_value
 
-      # Reorder bind indexes if joins produced bind values
-      bvs = arel.bind_values + bind_values
-      reorder_bind_params(arel.ast, bvs)
       arel
     end
 
-    def reorder_bind_params(ast, bvs)
-      ast.grep(Arel::Nodes::BindParam).each_with_index do |bp, i|
-        column = bvs[i].first
-        bp.replace connection.substitute_at(column, i)
-      end
-    end
-
     def symbol_unscoping(scope)
       if !VALID_UNSCOPING_VALUES.include?(scope)
         raise ArgumentError, "Called unscope() with invalid unscoping argument ':#{scope}'. Valid arguments are :#{VALID_UNSCOPING_VALUES.to_a.join(", :")}."
diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb
index 3dabb1104a..9353607f5d 100644
--- a/activerecord/test/cases/adapters/mysql/connection_test.rb
+++ b/activerecord/test/cases/adapters/mysql/connection_test.rb
@@ -70,7 +70,7 @@ class MysqlConnectionTest < ActiveRecord::TestCase
 
   def test_bind_value_substitute
     bind_param = @connection.substitute_at('foo', 0)
-    assert_equal Arel.sql('?'), bind_param
+    assert_equal Arel.sql('?'), bind_param.to_sql
   end
 
   def test_exec_no_binds
diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
index a71c0dfb26..7d6c15c826 100644
--- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb
@@ -305,10 +305,7 @@ module ActiveRecord
 
       def test_substitute_at
         bind = @connection.substitute_at(nil, 0)
-        assert_equal Arel.sql('$1'), bind
-
-        bind = @connection.substitute_at(nil, 1)
-        assert_equal Arel.sql('$2'), bind
+        assert_equal Arel.sql('$1'), bind.to_sql
       end
 
       def test_partial_index
diff --git a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
index 8c1c22d3bf..54e0309167 100644
--- a/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
+++ b/activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb
@@ -134,7 +134,7 @@ module ActiveRecord
 
       def test_bind_value_substitute
         bind_param = @conn.substitute_at('foo', 0)
-        assert_equal Arel.sql('?'), bind_param
+        assert_equal Arel.sql('?'), bind_param.to_sql
       end
 
       def test_exec_no_binds
diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb
index 0bc7ee6d64..d1fdfb2458 100644
--- a/activerecord/test/cases/bind_parameter_test.rb
+++ b/activerecord/test/cases/bind_parameter_test.rb
@@ -33,7 +33,7 @@ module ActiveRecord
       def test_binds_are_logged
         sub   = @connection.substitute_at(@pk, 0)
         binds = [[@pk, 1]]
-        sql   = "select * from topics where id = #{sub}"
+        sql   = "select * from topics where id = #{sub.to_sql}"
 
         @connection.exec_query(sql, 'SQL', binds)
 
@@ -44,7 +44,7 @@ module ActiveRecord
       def test_binds_are_logged_after_type_cast
         sub   = @connection.substitute_at(@pk, 0)
         binds = [[@pk, "3"]]
-        sql   = "select * from topics where id = #{sub}"
+        sql   = "select * from topics where id = #{sub.to_sql}"
 
         @connection.exec_query(sql, 'SQL', binds)
 
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index 8ac4a4ca36..ca86d58b35 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1703,7 +1703,7 @@ class RelationTest < ActiveRecord::TestCase
   end
 
   def test_unscope_removes_binds
-    left  = Post.where(id: Arel::Nodes::BindParam.new('?'))
+    left  = Post.where(id: Arel::Nodes::BindParam.new)
     column = Post.columns_hash['id']
     left.bind_values += [[column, 20]]