How do I optimize this query?

rubyonrails-talk
1

Right now I am wasting a query and also losing the order of Users. Was wondering what t he proper way to do this query is?

@top_tipper_ids = Tip.find_by_sql("SELECT SUM(tips.amount_cents) total_tip, tips.client_id FROM tips WHERE tips.vendor_id = #{@user.id} GROUP BY tips.client_id ORDER BY total_tip").collect {|e| e.client_id}

@top_tippers = User.find_all_by_id(@top_tipper_ids)

Trying to find the top tippers (Users) for a specific vendor.

2

Sprite, I would propose a custom view that exploits SQL powers. I do not know how much info you would like to syphon from the user model. Here is what I think you can do: Code (In User model): sql_query = "SELECT total_tippers.total_tips AS total_tips, total_tippers.client_id AS client_id

users.first_name AS first_name, users.last_name AS surname,

users.add_all_other_fields_you_require AS each_required_field FROM (

SELECT SUM(tips.amount_cents)AS total_tips, tips.client_id AS client_id
FROM tips

WHERE tips.vendor_id = #{@[user.id](http://user.id)}
GROUP BY tips.client_id

ORDER BY total_tip ) AS top_tippers, users

WHERE top_tippers.client_id = users.id"

@top_tippers = self.find_by_sql(sql_query) Expected/Sample Output (tabulated for the sake of crarity):

total_tips client_id first_name surname each_required_field

25 4 Aake Gregertsen data_1 23 1 Edmond Kachale data_1

Points to note:

  • I chose to use User model. This is to reflect that the top_tipper is a user not the tip itself. (for sense and readability’s sake). In addition, I think it’s a user who has (many) tips and not the other way round (B06.gif).

  • If you only want to use the user (without making use of the other data e.g. total_tips, client_id), as depicted by the “.collect” operation on your Tip model, then you can just scrap off the fields in the outer query so that it appears like this: sql_query = "SELECT * FROM (

     SELECT SUM(tips.amount_cents)AS

    total_tips, tips.client_id AS client_id

    FROM tips

WHERE tips.vendor_id = #{@[user.id](http://user.id)}

GROUP BY tips.client_id

ORDER BY total_tip ) AS top_tippers, users

    WHERE top_tippers.client_id = users.id" . But be ready to hassle a bit in order to find the fields you want. May be you can use script/console to figure out, though most geeks discourage testing codes from script/console.

  • Caution: I haven’t tested the code; expect a bug if you use as it is. You may need to customize it to fit your models and tables details. (nice one, isn’t it? 360.gif) Sorry for my long folk-tale.

Regards,

3

sprite wrote:

Right now I am wasting a query and also losing the order of Users. Was wondering what t he proper way to do this query is?

@top_tipper_ids = Tip.find_by_sql("SELECT SUM(tips.amount_cents) total_tip, tips.client_id FROM tips WHERE tips.vendor_id = #{@user.id} GROUP BY tips.client_id ORDER BY total_tip").collect {|e| e.client_id}

@top_tippers = User.find_all_by_id(@top_tipper_ids)

Trying to find the top tippers (Users) for a specific vendor.

Will the Calculations module help you here?

If not, you could add another join to get the user info in one query.

And never -- but never -- interpolate the user ID in the string the way you're doing. Use placeholders, or you leave yourself open for SQL injection.

Best,

4

Thanks for leading me on the right path. Fixed a few errors and it worked great:

  def top_tippers     sql_query = "SELECT top_tippers.total_tips AS total_tips,                  top_tippers.client_id AS client_id,                  users.login_slug AS login_slug,                  users.login AS login                  FROM (                  SELECT SUM(tips.amount_cents) AS total_tips, tips.client_id AS client_id                  FROM tips                  WHERE tips.vendor_id = #{self.id}                  GROUP BY tips.client_id                  ORDER BY total_tips DESC) AS top_tippers, users                  WHERE top_tippers.client_id = users.id"                  @top_tippers = User.find_by_sql(sql_query)   end

What's the danger of interpolating the id directly? It is not passed in any way from the user. It is the primary_key integer ID of the user. How do I use placeholders when constructing the query?

5

sprite wrote:

Thanks for leading me on the right path.

Whom are you addressing? Please quote when replying in future.

Fixed a few errors and it worked great:

  def top_tippers     sql_query = "SELECT top_tippers.total_tips AS total_tips,                  top_tippers.client_id AS client_id,                  users.login_slug AS login_slug,                  users.login AS login                  FROM (                  SELECT SUM(tips.amount_cents) AS total_tips, tips.client_id AS client_id                  FROM tips                  WHERE tips.vendor_id = #{self.id}                  GROUP BY tips.client_id                  ORDER BY total_tips DESC) AS top_tippers, users                  WHERE top_tippers.client_id = users.id"                  @top_tippers = User.find_by_sql(sql_query)   end

You probably don't need to write that much SQL. Again, see if the Calculations module will help.

What's the danger of interpolating the id directly? It is not passed in any way from the user. It is the primary_key integer ID of the user.

How sure can you be that you won't get passed a bogus ID?

How do I use placeholders when constructing the query?

Read the find_by_sql documentation.

Best,

6

Sprite, I propose a small change. Add a space between the sql_query string and the @top_tippers variable (for readability sake): def top_tippers sql_query = “SELECT top_tippers.total_tips AS total_tips, top_tippers.client_id AS client_id, users.login_slug AS login_slug, users.login AS login FROM ( SELECT SUM(tips.amount_cents) AS total_tips, tips.client_id AS client_id FROM tips WHERE tips.vendor_id = #{self.id} GROUP BY tips.client_id ORDER BY total_tips DESC) AS top_tippers, users WHERE top_tippers.client_id = users.id”

@top_tippers = User.find_by_sql(sql_query) end And this blog post may be helpful too as you code: Top 10 Things That Annoy Programmers. Regards,