thr3ads.net - Rails - [Rails] Reduce Number of Queries When Using ActiveRecord [Apr 2006]

If this information is useful, please help other people find it:
Share via:

Eden Brandeis

2006-Apr-11 18:11 UTC

[Rails] Reduce Number of Queries When Using ActiveRecord

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://wrath.rubyonrails.org/pipermail/rails/attachments/20060411/741ad5cc/attachment.html

Jeff Coleman

2006-Apr-11 18:41 UTC

head link

[Rails] Re: Reduce Number of Queries When Using ActiveRecord

Eden Brandeis 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 Coleman

-- 
Posted via http://www.ruby-forum.com/.

Eden Brandeis

2006-Apr-11 19:41 UTC

head link

[Rails] Re: Reduce Number of Queries When Using ActiveRecord

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

On 4/11/06, Jeff Coleman <progressions@gmail.com>
wrote:>
> Eden Brandeis 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 Coleman
>
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> Rails mailing list
> Rails@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://wrath.rubyonrails.org/pipermail/rails/attachments/20060411/f2e27167/attachment-0001.html

Jeff Coleman

2006-Apr-11 20:34 UTC

head link

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

Eden Brandeis 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 Coleman

-- 
Posted via http://www.ruby-forum.com/.

Jeff Coleman

2006-Apr-11 21:02 UTC

head link

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

Eden Brandeis 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 Coleman

-- 
Posted via http://www.ruby-forum.com/.

Eden Brandeis

2006-Apr-12 20:32 UTC

head link

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

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

On 4/11/06, Jeff Coleman <progressions@gmail.com>
wrote:>
> Eden Brandeis 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 Coleman
>
> --
> Posted via http://www.ruby-forum.com/.
> _______________________________________________
> Rails mailing list
> Rails@lists.rubyonrails.org
> http://lists.rubyonrails.org/mailman/listinfo/rails
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://wrath.rubyonrails.org/pipermail/rails/attachments/20060412/e61b2934/attachment-0001.html

Eden Brandeis

2006-Apr-16 15:08 UTC

head link

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

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

On 4/12/06, Eden Brandeis <ebrandeis@gmail.com>
wrote:>
> Thank you for your help Jeff.  I will look at reducing the number of
> queries again when I start optimizing the application.
>
> On 4/11/06, Jeff Coleman < progressions@gmail.com> wrote:
>
> > Eden Brandeis 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 Coleman
> >
> > --
> > Posted via http://www.ruby-forum.com/ .
> > _______________________________________________
> > Rails mailing list
> > Rails@lists.rubyonrails.org
> > http://lists.rubyonrails.org/mailman/listinfo/rails
> >
>
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://wrath.rubyonrails.org/pipermail/rails/attachments/20060416/9376ee8b/attachment-0001.html

Maybe Matching Threads

Search for more maybe matching threads

Rails - Apr 2006 - Reduce Number of Queries When Using ActiveRecord

[Rails] Reduce Number of Queries When Using ActiveRecord

[Rails] Re: Reduce Number of Queries When Using ActiveRecord

[Rails] Re: Reduce Number of Queries When Using ActiveRecord

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

[Rails] Re: Re: Reduce Number of Queries When Using ActiveRecord

Maybe Matching Threads