This project is archived and is in readonly mode.

#3296 ✓invalid
Morgan Grubb

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|
 def self.down
  drop_table :examples
class CreateExampleUrls < ActiveRecord::Migration
 def self.up
  create_table :example_urls do |t|
   t.string :url
   t.integer :example_id
 def self.down
  drop_table :example_urls

And these models

class Example < ActiveRecord::Base
  has_many :example_urls
class ExampleUrl < ActiveRecord::Base
  belongs_to :example

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 = 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)
            attr_table_name = table_name

          attribute_condition(&quot;#{attr_table_name}.#{connection.quote_column_name(attr)}&quot;, value)
          sanitize_sql_hash_for_conditions(value, connection.quote_table_name(attr.to_s))
      end.join(' AND ')

      replace_bind_variables(conditions, expand_range_bind_variables(attrs.values))

    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) }

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

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=""></a>