This project is archived and is in readonly mode.
Failing functional test of ActiveRecord conditions merging
Reported by Morgan Grubb | September 28th, 2009 @ 05:54 PM
With Rails 2.3.4:
Given this schema
class CreateExamples < ActiveRecord::Migration
def self.up
create_table :examples do |t|
end
end
def self.down
drop_table :examples
end
end
class CreateExampleUrls < ActiveRecord::Migration
def self.up
create_table :example_urls do |t|
t.string :url
t.integer :example_id
end
end
def self.down
drop_table :example_urls
end
end
And these models
class Example < ActiveRecord::Base
has_many :example_urls
end
class ExampleUrl < ActiveRecord::Base
belongs_to :example
end
This query passes
ExampleUrl.first :conditions => { :url => "this will not explode?" }
While this one fails
Example.first :joins => :example_urls, :conditions => { :example_urls => { :url => "this will explode?" } }
This one, however, also passes
Example.first :joins => :example_urls, :conditions => ["example_urls.url = ?", "this will not explode?"]
I tracked the problem down to a couple of methods in
ActiveRecord::Base
def sanitize_sql_hash_for_conditions(attrs, default_table_name = quoted_table_name)
attrs = expand_hash_conditions_for_aggregates(attrs)
conditions = attrs.map do |attr, value|
table_name = default_table_name
unless value.is_a?(Hash)
attr = attr.to_s
# Extract table name from qualified attribute names.
if attr.include?('.')
attr_table_name, attr = attr.split('.', 2)
attr_table_name = connection.quote_table_name(attr_table_name)
else
attr_table_name = table_name
end
attribute_condition("#{attr_table_name}.#{connection.quote_column_name(attr)}", value)
else
sanitize_sql_hash_for_conditions(value, connection.quote_table_name(attr.to_s))
end
end.join(' AND ')
replace_bind_variables(conditions, expand_range_bind_variables(attrs.values))
end
def replace_bind_variables(statement, values) #:nodoc:
raise_if_bind_arity_mismatch(statement, statement.count('?'), values.size)
bound = values.dup
statement.gsub('?') { quote_bound_value(bound.shift) }
end</code>
What's happening is the recursive call to sanitize_sql_hash_for_conditions is calling replace_bind_variables on the inner conditions hash, thus interpolating the value into the string query. Then the outer call attempts to interpolate it's variables which is where the problem starts.
replace_bind_variables counts the number of question marks in the [previously] interpolated condition string, finds the one in the "example_urls.url = 'this will explode?'" piece and, not seeing any variables to bind to, raises PreparedStatementInvalid.
When considering how to fix this issue I've ruled out replacing question marks with strings at the point of interpolation and replacing them just before finalizing the query as there is no guarantee that whatever replacement string I used will not legitimately appear in the query itself. This leaves two other options: count the question marks in the already interpolated strings and don't include those when binding variables (difficult but not impossible), or put off interpolating until after the string is completed (much more difficult but definitely the right thing to do).
I don't have the time to fix this myself so I'm hoping someone else can pick this up and run with it.
Comments and changes to this ticket
-
Martin Luder June 11th, 2010 @ 11:35 AM
We just ran into this issue with Rails 2.3.5. Does nobody care?
-
Neeraj Singh July 22nd, 2010 @ 07:28 PM
- State changed from new to invalid
- Importance changed from to
Marking it as invalid. If you are able to reproduce this problem with rails 2-3-stable or rails master then it could be reopened.
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>