Working on a report/metrics page and I need to optimize the queries as much as I can so I am using find_by_sql for efficiency.

One of my queries is doing some aggregate functions where I return a count and some sums. I'm assigning the results of this query into instance variables for a model.

I have working code, but the code scares me. I have read the official Ruby/Rails docs on the methods used, but I still think there's something wrong with what I'm doing.

def initialize(args)
  @loans_count           = stats.loans_count
  @total_fees_in_cents   = stats.total_fees_in_cents
  @total_amount_in_cents = stats.total_amount_in_cents
end

def stats
  @stats ||= find_stats
end

def find_stats
  if single_year?
    loans = Loan.find_by_sql(["SELECT count(*) as loans_count, sum(amount) as total_amount_in_cents, sum(fee) as total_fees_in_cents FROM loans WHERE account_id = ? AND year = ? LIMIT 1", @account_id, @year]).first
  else
    loans = Loan.find_by_sql(["SELECT count(*) as loans_count, sum(amount) as total_amount_in_cents, sum(fee) as total_fees_in_cents FROM loans WHERE account_id = ? LIMIT 1", @account_id]).first
  end

  # Store results in OpenStruct for ease of access later on
  OpenStruct.new(
    loans_count: loans.loans_count || 0,
    total_fees_in_cents: loans.total_fees_in_cents || 0,
    total_amount_in_cents: loans.total_amount_in_cents || 0
  )
end

Concerns

1. find_by_sql is supposed to return an array; the SQL will always return a row, even if no matches are found (null values, but a valid row). However, is there a reason I shouldn't call .first on the returned array? I'm afraid of hitting [].first => nil in some case I didn't anticipate.
2. Is my method of "caching" the result using the stats method an appropriate method of only querying the DB 1 time? It seems like a lot of code and methods just to get some aggregate data.

Dan,

There is two ways to look at this problem...

Method 1

I think I can address both questions with one answer here (^_-), and the key is to address your fear.

You're mainly worried about the whole [].first => nil. Second your worried about DB efficiency. Thirdly, you want to make this clean and re-factored (Good for you!).

Your answer...let PostgreSQL do this work for you, and force it to return a non-nil answer every time. Get it?

1. There's no need for the OpenStruct because your labels in your SQL query are the same.
2. Use the PostgreSQL function COALESCE() to force the null to be a zero.
3. You 'could' make all of this a one-liner, but let's chop it up a bit for ease of the reader.
4. This is already a summation query, you don't need the LIMIT 1.

Let's rewrite the code:

def initialize(args)
  @loans_count           = stats.loans_count
  @total_fees_in_cents   = stats.total_fees_in_cents
  @total_amount_in_cents = stats.total_amount_in_cents
end

def stats
  loans = Loan.select("count(*) as loans_count, COALESCE(sum(amount), 0) as total_amount_in_cents, COALESCE(sum(fee), 0) as total_fees_in_cents").where(account_id: @account_id)

  loans = loans.where(year: @year) if single_year?

  loans.first
end

Method 2

I personally think you are overly worried about the DB efficiency thing. You can always look at your development/production logs to read what is actually being outputted to the PSQL server, but I'm pretty sure it's close to the same thing your doing here.

Also, if I remember correctly, the DB query isn't actually executed UNTIL you want the data. During that time, ActiveRecord is just preparing the QUERY string.

.to_i will convert your null to zero.

Let's rewrite the code:

def initialize(args)
  stats = Loan.where(account_id: @account_id)
  stats = stats.where(year: @year) if single_year?
  stats.first

  @loans_count           = stats.count
  @total_fees_in_cents   = stats.sum(:fee).to_i
  @total_amount_in_cents = stats.sum(:amount).to_i
end