Reduce Number of Queries When Using ActiveRecord

Rails
1

I have a controller method called update_all that grabs parameters for
about
30 form fields and saves them to the database. My current code looks to
see
if there is a string in params[] for each form field (two fields for
each
SelfEvaluationItem) and saves the updated self_evaluation_answer if a
string
is available. Since this is iterated code, I am almost certain that
this
generates 30 or so SQL queries.

Is there a way to build up all of the self_evaluation_answers into a
larger
unit (model array?) and then save them all at once? The goal of course
would be to reduce the number of queries to just 1. I guess the
alternate
would be to build a SQL query to do this and execute it, but I prefer to
work in Ruby and Rails if possible.

Any help is very much appreciated!

I have included the source for reference below:

def update_all
for item in SelfEvaluationItem.find_all
id = params[:self_evaluation_answer][item.id.to_s][:id]
comment = params[:self_evaluation_answer][item.id.to_s][:comment]
response =
params[:self_evaluation_answer][item.id.to_s][:response]
if !comment.empty? or !response.empty?
@self_evaluation_answer = SelfEvaluationAnswer.find(id)
@self_evaluation_answer.comment = comment unless comment.empty?
@self_evaluation_answer.response = response unless
response.empty?
@self_evaluation_answer.save
end
end
flash[:notice] = “Save Completed”
end

2

Eden B. wrote:

I have a controller method called update_all that grabs parameters for
about
30 form fields and saves them to the database. My current code looks to
see
if there is a string in params[] for each form field (two fields for
each
SelfEvaluationItem) and saves the updated self_evaluation_answer if a
string
is available. Since this is iterated code, I am almost certain that
this
generates 30 or so SQL queries.

Is there a way to build up all of the self_evaluation_answers into a
larger
unit (model array?) and then save them all at once? The goal of course
would be to reduce the number of queries to just 1. I guess the
alternate
would be to build a SQL query to do this and execute it, but I prefer to
work in Ruby and Rails if possible.

Any help is very much appreciated!

I have included the source for reference below:

def update_all
for item in SelfEvaluationItem.find_all
id = params[:self_evaluation_answer][item.id.to_s][:id]
comment = params[:self_evaluation_answer][item.id.to_s][:comment]
response =
params[:self_evaluation_answer][item.id.to_s][:response]
if !comment.empty? or !response.empty?
@self_evaluation_answer = SelfEvaluationAnswer.find(id)
@self_evaluation_answer.comment = comment unless comment.empty?
@self_evaluation_answer.response = response unless
response.empty?
@self_evaluation_answer.save
end
end
flash[:notice] = “Save Completed”
end

If I’m reading this correctly, what you’re doing is cycling through
every single item in your SelfEvaluationItem table to compare each item
against the entries in params[:self_evaluation_answer]. Wouldn’t it be
more efficient to cycle through params[:self_evaluation_answer] and
refer to evaluation items based on the results?

For example:

def update_all
params[:self_evaluation_answer].each do |answer|
answer.each_key do |id_from_answer|
item = SelfEvaluationAnswer.find(id_from_answer.to_i)
new_comment = answer[id_from_answer][:comment]
new_response = answer[id_from_answer][:response]
unless new_comment.empty? and new_response.empty?
@self_evaluation_answer = item
@self_evaluation_answer.comment = new_comment unless
new_comment.empty?
@self_evaluation_answer.response = new_response unless
new_response.empty?
@self_evaluation_answer.save
end
end
end
flash[:notice] = “Save Completed”
end

Again I’m hoping I have your read your code correctly, but what I
attemped to do here is to loop through the
params[:self_evaluation_answer] hash, which seems to have keys that are
String versions of your SelfEvaluationItem primary key ids.

So we take each key and use it to find the SelfEvaluationItemAnswer
record which matches that id. Then if there’s a new comment or response
we add them to the record, and save it.

I added descriptive flash messages indicating when an error prevented
the record from saving or when the record didn’t need to be updated.

I’m not sure this really addresses the gist of your question, but I
thought I’d offer it as an alternative that might be slightly more
efficient. Feel free to disregard if this doesn’t suit your needs for
whatever reason.

Jeff C.man

3

Jeff,

Thank you for the suggestions. I really like your approach better than
mine
for aesthetic reasons, but it seems that you have introduced an extra
SQL
query with each iteration:

item = SelfEvaluationAnswer.find(id_from_answer.to_i)

Since the form will return a valid key for every field (even when left
blank), I think this is guaranteed to more than double the number of
queries
versus the code I have now. Currently I only generate a query before
entering the loop and once for each form item that has content (blank
form
items don’t generate and Update query).

To be honest, it is a little early in the development of this
application to
worry much about optimization, but I realized that using SQL I could
probably do all the UPDATE queries in my loop in a single statement and
thought I could nip this problem in the bud while still using
ActiveRecord.

Thanks again for your contribution. I will probably at least reorganize
some of my code based on your suggestions.

Eden

4

Eden B. wrote:

Jeff,

Thank you for the suggestions. I really like your approach better than
mine
for aesthetic reasons, but it seems that you have introduced an extra
SQL
query with each iteration:

item = SelfEvaluationAnswer.find(id_from_answer.to_i)

Since the form will return a valid key for every field (even when left
blank), I think this is guaranteed to more than double the number of
queries
versus the code I have now. Currently I only generate a query before
entering the loop and once for each form item that has content (blank
form
items don’t generate and Update query).

To be honest, it is a little early in the development of this
application to
worry much about optimization, but I realized that using SQL I could
probably do all the UPDATE queries in my loop in a single statement and
thought I could nip this problem in the bud while still using
ActiveRecord.

Thanks again for your contribution. I will probably at least reorganize
some of my code based on your suggestions.

Eden

Eden,

I see what you mean abou the SQL query–it was my mistake, but the line

item = SelfEvaluationAnswer.find(id_from_answer.to_i)

could be placed at the top of the conditional block, the way you had it
in your original. That would at least make it equal to your original
version in SQL calls, if we exclude your original find_all.

I’m not a SQL expert but I’ve been thinking about how to optimize your
overall number of SQL queries. I’m guessing with enough thought, that a
sufficient :condition statement could be amalgamated from your
parameters that would do what you’re looking for.

I’ll let you know if anything comes up.

Jeff C.man

5

Eden B. wrote:

To be honest, it is a little early in the development of this
application to
worry much about optimization, but I realized that using SQL I could
probably do all the UPDATE queries in my loop in a single statement and
thought I could nip this problem in the bud while still using
ActiveRecord.

Eden,

Try this. What I’ve done is basically make a new updated_answers Hash
using only the answers which have had comment or response updated. I’ve
converted the index from a string to an integer so we can use it to find
all the related SelfEvaluationItem records.

I believe you’ll still need to cycle through the array and save each
record individually, but at least the query can be reduced to a single
statement.

def update_all
updated_answers = {}
params[:self_evaluation_answer].each do |answer|
answer.each_key do |id_from_answer|
new_comment = answer[id_from_answer][:comment]
new_response = answer[id_from_answer][:response]

  updated_answers[id_from_answer.to_i] = answer[id_from_answer]

unless (new_comment.empty? and new_response.empty?)
end
end
updated_items = SelfEvaluationAnswer.find(updated_answers.keys)
updated_items.each do |item|
new_comment = updated_answers[item.id][:comment]
new_response = updated_answers[item.id][:response]

item.comment = new_comment unless new_comment.empty?
item.response = new_response unless new_response.empty?
item.save

end
flash[:notice] = “Save completed.”
end

There may be a more efficient way of doing this, but this is what I’ve
come up with. Hope it helps you some,

Jeff C.man

6

Thank you for your help Jeff. I will look at reducing the number of
queries
again when I start optimizing the application.

7

Jeff and others following this thread,

I found what looks like a good solution to this problem over at Rails
Weenie:

http://rails.techno-weenie.net/tip/2006/3/15/speed_up_inserts_with_transactions

I think if I wrap my updates in a transaction, then I should get a
single
query to the db. I will need to do some testing to make sure that I get
the
expected results if the transaction fails, but I think this is a
possible
solution to my original question (reduce queries from 32->2).

Thanks again,

Eden