Permalink
Browse files

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
  • Loading branch information...
sgrif committed Nov 17, 2014
1 parent bf14967 commit c01b20b658c9fe4b7d54f4a227a09cb090b5763d
@@ -265,7 +265,7 @@ def index_algorithms
# 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 ====================================
@@ -156,10 +156,6 @@ def execute(sql, name = nil)
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 = {}
@@ -84,7 +84,6 @@ def _update_record(values, id, id_was) # :nodoc:
um = relation
.arel
.compile_update(substitutes, @klass.primary_key)
reorder_bind_params(um.ast, bvs)

@klass.connection.update(
um,
@@ -881,19 +881,9 @@ def build_arel
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(", :")}."
@@ -70,7 +70,7 @@ def test_successful_reconnection_after_timeout_with_verify

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
@@ -305,10 +305,7 @@ def test_exec_typecasts_bind_vals

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
@@ -134,7 +134,7 @@ def test_encoding

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
@@ -33,7 +33,7 @@ def setup
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 @@ def test_binds_are_logged
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)

@@ -1703,7 +1703,7 @@ def test_presence
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]]

2 comments on commit c01b20b

@tommeier

This comment has been minimized.

Copy link
Contributor

tommeier replied Nov 22, 2014

This has caused bugs in test runs for me, before this commit passing, after this commit unable to add data on seed commands:

ActiveRecord::StatementInvalid: PG::SyntaxError: ERROR:  syntax error at or near ","
LINE 1: ...ers" ("name", "created_at", "updated_at") VALUES (, , ) RETU...
                                                             ^
: INSERT INTO "players" ("name", "created_at", "updated_at") VALUES (, , ) RETURNING "id"
/Users/tom/src/company/test-rails/db/seeds.rb:1:in `<top (required)>'
Tasks: TOP => db:setup => db:seed
(See full trace by running task with --trace)
@sgrif

This comment has been minimized.

Copy link
Member Author

sgrif replied Nov 22, 2014

Please sign in to comment.