0

I have this huge ugly query below, i'd like to sort by it in a catalog view. Thinking something like http://wow.dev:3000/catalog_items?&order=deals. A million thank yous in advance for any comments or answers.

select 100 - round((current_price / item.estimated_price)*100) as percent, item.cached_thumbnail_url, item.item_id, it.name,
          ci.current_price, ci.close_date
           from item
           join catalog_item ci on ci.item_id = item.item_id
           join item_translations as it on (it.item_id = item.item_id)
           where  (100 - round((current_price / item.estimated_price)*100)) > 49 and 
           item.estimated_price > 0 and ci.current_price > 0 and ci.close_date > now() and item.active = 1 and ci.active = 1 and 
           (current_price / estimated_price) < 1
           order by (ci.close_date < DATE_ADD(now(), INTERVAL 17 hour))  and (item.estimated_price - current_price) desc
           limit 12
jahrichie
  • 1,215
  • 3
  • 17
  • 26
  • Are you building the catalog page using the information returned by this query? Or something else? – octern Jul 26 '12 at 05:46

2 Answers2

0

Not sure how this could be related to ROR, but anyways:

  1. You have 100 - round((current_price / item.estimated_price)*100) as percent in your SELECT clause, yet anyway using the same expression in WHERE conditions.

  2. Can item.estimated_price be less than zero? If not the item.estimated_price > 0 condition is excessive, if it's zero the (100 - round((current_price / item.estimated_price)*100)) > 49 condition will be false

  3. The (current_price / estimated_price) < 1 is excessive for same reasons

So you query can be rewritten a bit more clearly like this:

select (100 - round((current_price / item.estimated_price)*100)) as percent,
   item.cached_thumbnail_url, item.item_id, it.name,
   ci.current_price, ci.close_date
from item
   join catalog_item ci on ci.item_id = item.item_id
   join item_translations as it on (it.item_id = item.item_id)
where
   percent > 49
   and ci.current_price > 0
   and ci.close_date > now()
   and item.active = 1
   and ci.active = 1
order by
   (ci.close_date < DATE_ADD(now(), INTERVAL 17 hour))
   and (item.estimated_price - current_price) desc
limit 12

This doesn't improve the situation a lot, but I can't say more without knowing any more reasons about your DB architecture.

BTW the link in your question doesn't work (it's you local link obviously)

binarycode
  • 1,788
  • 15
  • 15
0

Building up on binarycode's answer, you can try wrapping your query in ARel, and then in your controller action you'd order on the field passed in the params hash, something like this:

class ItemsController < ApplicationController
  ...
  def index
    @items = Item.select(%{(100 - round((current_price / item.estimated_price)*100)) as percent,
               item.cached_thumbnail_url, item.item_id, it.name,
               ci.current_price, ci.close_date}).
             joins('join catalog_item ci on ci.item_id = item.item_id').
             joins('join item_translations as it on (it.item_id = item.item_id)').
             where('percent > 49 and ci.current_price > 0 and ci.close_date > now() and item.active = 1 and ci.active = 1').
             order(%{(ci.close_date < DATE_ADD(now(), INTERVAL 17 hour))
               and (item.estimated_price - current_price) desc)}).limit(12)
    @items = @items.order(params[:order]) if params[:order]
    ...
end

EDIT

As binarycode points out below, you might want to make the controller code cleaner by moving the main query outside the action, probably into a method in the Item model, and making sure you still return a Relation object from it (like the current statement does), so you could chain the extra order later:

def index
  @items = Item.by_price_ratio
  @items = @items.order(params[:order]) if params[:order]
end
HargrimmTheBleak
  • 2,147
  • 1
  • 19
  • 19
  • 1
    Please don't recommend placing such ugly queries (in fact any SQL queries) in controllers. They should be placed in the model instead. – binarycode Jul 26 '12 at 07:03
  • Agreed, it's just a dirty example to outline the basic idea, long queries like that don't belong in controllers. – HargrimmTheBleak Jul 26 '12 at 07:16