0

I have a table where the user can filter in a variety of ways using many different types of combinations. With all the input data I'm getting from the user it was important for me to escape the user data from the SQL which leads into the problem that I'm now having. I have two arrays that are dynamically built based on the params sent to the action, one array contains the SQL clauses and the other contains the values to be paired with it's respective caluse... so for instance...

def results

  sql_clauses = Array.new
  sql_args = Array.new

  unless params[:elapsed_time].nil?
    sql_clauses << "elapsed_time = ?"
    sql_args << params[:elaped_time] 
  end
  unless params[:age_greater_than].nil?
    sql_clauses << "age > ?"
    sql_args << params[:age_greater_than]
  end
  .....
  @results = Model.where(sql_clauses.join(" and "), sql_args.join(", "))

end

Now this sends the sql_clauses array to the where method no problem. But it bombs out on the second argument because it returns a single string and it's expecting individual variables in correspondence with each of the "?" fields that appear in the sql_clauses array. I've tried the solutions offered by KandadaBoggu on Comine arrays of conditions in rails. Neither of these options worked for me though but it might be because I'm using 2 arrays instead of 1.

Does anybody know of a solution to my problem?

Community
  • 1
  • 1
Noz
  • 6,216
  • 3
  • 47
  • 82

2 Answers2

5

There is no need to use strings to build dynamic queries in Rails 3. ActiveRecord methods like select, where, order, limit, etc, return ActiveRecord::Relation objects which can be chained without triggering multiple database calls:

cars = Car.where(:colour => 'black') # No database queries are generated here.
rich_ppls_cars = cars.order('cars.price DESC').limit(10) # Still no db queries.

The db will be queried when we call .all, .first, .last, or .each on an ActiveRecord::Relation object.

Example Code

Let's assume you are querying a model with the following columns:

  • name
  • elapsed_time
  • age

And you have a params hash that looks like this:

{ :elapsed_time => 34, :age_greater_than => 14, :max_rows => 20 }

Your controller action could look like this:

def results
    query = ModelName.select([:name, :elapsed_time, :age])
    query = query.where(:elapsed_time => params[:elapsed_time])  if params[:elapsed_time].present?
    query = query.where('age > ?', params[:age_greater_than])  if params[:age_greater_then].present?
    query = query.limit(params[:max_rows])  if params[:max_rows].present?
    @dynamic_query = query
end

Note that I am testing for the presence of the params keys using .present?. This prevents us from misinterpreting a blank string in the params hash as a valid data.

Reference Material

Wally Glutton
  • 76
  • 3
  • Yup, this is exactly what I ended up implementing. Thanks for the very detailed post. – Noz May 28 '12 at 14:20
1

Try something like this:

def results
  sql_clauses = []
  sql_clauses << "elapsed_time = :elapsed_time" if params[:elapsed_time]
  sql_clauses << "age > :age_greater_than" if params[:age_greater_than]
  .....
  @results = Model.where(sql_clauses.join(" and "), params)
end

where supports a variety of difference syntaxes for building conditions.

If you have a lot of possible fileds to include I would make a mapping dictionary somehow, like this:

map = {
   :elapsed_time => "elapsed_time =",
   :age_greater_than => "age >"
}

And then loop thru the params keys and build the clauses if they exist in map.

Mattias Wadman
  • 11,172
  • 2
  • 42
  • 57
  • Is this safe from SQL injections? – Noz May 25 '12 at 15:28
  • Yeah i think so, why not try? – Mattias Wadman May 25 '12 at 16:04
  • User.where(":a", {:a => "\"a\""}) -> `SELECT `users`.* FROM `users` WHERE ('\"a\"')` – Mattias Wadman May 25 '12 at 16:07
  • Hey Mattias, unfortunately for some of the params some additional processing needs to take place and I can't accept them into my query as an argument as is. But I didn't know you could pass the entire params hash like that I think it's certainly useful to know. A colleague of mine recommended an option that works better for me, he'll share his answer shortly. – Noz May 25 '12 at 17:05