Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

SqlLiterals quoting fix #159

Merged
merged 1 commit into from

3 participants

@olliwer

Added SqlLiteral check for quote method.

@ernie
Collaborator

This seems not altogether unreasonable. That being said, where are you putting SqlLiterals that they're ending up being passed to quote?

@tenderlove
Owner

@ernie this is to support passing bind values to update statements. Update statements use the Assignment node, which quotes things all the time. Obviously a quoted bind param won't work so well. ;-)

You think putting it in quote is good, or should we move it to the visitor method?

@ernie
Collaborator

@tenderlove That's what I was wondering -- only because elsewhere, we're handling these cases in the visitor. Then again, if we could safely pass SqlLiterals through all the time in quote, we could avoid branching so much in the visitors, right?

@tenderlove
Owner

@ernie it looks like we're totally inconsistent. We have some in the quote methods and some in the visitor methods. One of the visitor methods is able to use the is_a? check to avoid calculating a column.

Not 100% sure what to do. Thoughts?

@ernie
Collaborator

Oh, well then. Blergh. I guess I'm leaning toward doing it in quote, but paranoid that someone, somewhere, will create SqlLiterals and then blame us for not quoting their SQL LITERALS.

Because... Well... yeah.

@tenderlove
Owner

@ernie. meh. There's other places they can inject the literals. I'll pull in this PR, we can fix it later if we have to.

@tenderlove tenderlove merged commit 789d059 into rails:master
@ernie
Collaborator

Cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 29, 2013
  1. @olliwer
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 0 deletions.
  1. +1 −0  lib/arel/visitors/to_sql.rb
  2. +8 −0 test/test_update_manager.rb
View
1  lib/arel/visitors/to_sql.rb
@@ -589,6 +589,7 @@ def visit_Array o
end
def quote value, column = nil
+ return value if Arel::Nodes::SqlLiteral === value
@connection.quote value, column
end
View
8 test/test_update_manager.rb
@@ -8,6 +8,14 @@ module Arel
end
end
+ it "should not quote sql literals" do
+ table = Table.new(:users)
+ um = Arel::UpdateManager.new Table.engine
+ um.table table
+ um.set [[table[:name], (Arel::Nodes::BindParam.new '?')]]
+ um.to_sql.must_be_like %{ UPDATE "users" SET "name" = ? }
+ end
+
it 'handles limit properly' do
table = Table.new(:users)
um = Arel::UpdateManager.new Table.engine
Something went wrong with that request. Please try again.