7

I am working on migrating a legacy database into my Rails application (3.2.3). The original database comes with quite a few long sql queries for reports. For now, what I would like to do it use the sql queries in the Rails application and then one by one (when time allows) swap the sql queries to 'proper' Rails queries.

I have a clinical model and the controller has the following code:

 @clinical_income_by_year = Clinical.find_all_by_sql(SELECT date_format(c.transactiondate,'%Y') as Year, 
                                                 date_format(c.transactiondate,'%b') as Month,
                                                 sum(c.LineBalance) as "Income"
                                                 FROM clinical c
                                                 WHERE c.Payments = 0 AND c.LineBalance <> 0
                                                 AND c.analysiscode <> 213
                                                 GROUP BY c.MonthYear;)

However, when I run that code I get a few errors to do with the formatting.

Started GET "/clinicals" for 127.0.0.1 at 2012-04-29 18:00:45 +0100

SyntaxError (/Users/dannymcclelland/Projects/premvet/app/controllers/clinicals_controller.rb:6: syntax error, unexpected tIDENTIFIER, expecting ')'
...rmat(c.transactiondate,'%Y') as Year, 
...                               ^
/Users/dannymcclelland/Projects/premvet/app/controllers/clinicals_controller.rb:7: syntax error, unexpected tIDENTIFIER, expecting keyword_end
...rmat(c.transactiondate,'%b') as Month,
...                               ^
/Users/dannymcclelland/Projects/premvet/app/controllers/clinicals_controller.rb:8: syntax error, unexpected tIDENTIFIER, expecting keyword_end
...          sum(c.LineBalance) as "Income"
...                               ^
/Users/dannymcclelland/Projects/premvet/app/controllers/clinicals_controller.rb:10: syntax error, unexpected tCONSTANT, expecting keyword_end
...       WHERE c.Payments = 0 AND c.LineBalance <> 0
...                               ^
/Users/dannymcclelland/Projects/premvet/app/controllers/clinicals_controller.rb:10: syntax error, unexpected '>'
...yments = 0 AND c.LineBalance <> 0
...                               ^
/Users/dannymcclelland/Projects/premvet/app/controllers/clinicals_controller.rb:11: syntax error, unexpected '>'
...          AND c.analysiscode <> 213
...                               ^

Is there something I should be doing to the sql query before importing it into the controller? Although it's possible there is something wrong with the query (It was written quite some time ago), it does work as expected when run directly within the database. It returns an array like this:

----------------------------------------------
|  Year      |    Month     |     Income     |
----------------------------------------------
----------------------------------------------
|  2012      |    January   |   20,000       |
|  2012      |    February  |   20,000       |
|  2012      |    March     |   20,000       |
|  2012      |    April     |   20,000       |
----------------------------------------------
etc..

Any help, advice or general pointers would be appreciated!

I'm reading through http://guides.rubyonrails.org/active_record_querying.html trying to convert the sql query to a correct Rails query.

So far I have matched the second to last line:

AND c.analysiscode <> 213

with

@clinical_income_by_year = Clinical.where("AnalysisCode != 213")

baby steps!

UPDATE

I've got the filtering sorted now, thanks to the Rails guide site but I'm stuck on the grouping and sum part of the sql query. I have the following so far:

@clinical_income_by_year = Clinical.where("AnalysisCode != 213 AND Payments != 0 AND LineBalance != 0").page(params[:page]).per_page(15)

I'm struggling to build in the following two lines of the sql query:

sum(c.LineBalance) as "Income"

and

GROUP BY c.MonthYear;)

My view code looks like this:

<% @clinical_income_by_year.each do |clinical| %>
  <tr>
    <td><%= clinical.TransactionDate.strftime("%Y") %></td>
    <td><%= clinical.TransactionDate.strftime("%B") %></td>
    <td><%= Clinical.sum(:LineBalance) %></td>
  </tr>    
  <% end %>
</table>
  <%= will_paginate @clinical_income_by_year %>
mu is too short
  • 426,620
  • 70
  • 833
  • 800
dannymcc
  • 3,744
  • 12
  • 52
  • 85

1 Answers1

14

The Ruby parser doesn't understand SQL, you need to use a string:

@clinical_income_by_year = Clinical.find_by_sql(%q{ ... })

I'd recommend using %q or %Q (if you need interpolation) for this so that you don't have to worry about embedded quotes so much. You should also move that into a class method in the model to keep your controllers from worrying about things that aren't their business, this will also give you easy access to connection.quote and friends so that you can properly use string interpolation:

find_by_sql(%Q{
    select ...
    from ...
    where x = #{connection.quote(some_string)}
})

Also, the semicolon in your SQL:

GROUP BY c.MonthYear;})

isn't necessary. Some databases will let it through but you should get rid of it anyway.

Depending on your database, the identifiers (table names, column names, ...) should be case insensitive (unless some hateful person quoted them when they were created) so you might be able to use lower case column names to make things fit into Rails better.

Also note that some databases won't like that GROUP BY as you have columns in your SELECT that aren't aggregated or grouped so there is ambiguity about which c.transactiondate to use for each group.


A more "Railsy" version of your query would look something like this:

@c = Clinical.select(%q{date_format(transactiondate, '%Y') as year, date_format(transactiondate, '%b') as month, sum(LineBalance) as income})
             .where(:payments => 0)
             .where('linebalance <> ?', 0)
             .where('analysiscode <> ?', 213)
             .group(:monthyear)

Then you could do things like this:

@c.each do |c|
    puts c.year
    puts c.month
    puts c.income
end

to access the results. You could also simplify a little bit by pushing the date mangling into Ruby:

@c = Clinical.select(%q{c.transactiondate, sum(c.LineBalance) as income})
             .where(:payments => 0)
             .where('linebalance <> ?', 0)
             .where('analysiscode <> ?', 213)
             .group(:monthyear)

Then pull apart c.transactiondate in Ruby rather than calling c.year and c.month.

mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • As I've started down the route of converting the sql query to a Rails query, would you recommend continuing or is using sql queries a common thing within models and controllers? – dannymcc Apr 29 '12 at 17:43
  • 1
    @dannymcc: Using the Rails methods is usually recommended but sometimes SQL is necessary, if you used "advanced" database features such as stored procedures, window functions, derived tables, CTEs, etc. then you will have to write a lot of raw SQL to get the job done; if all your queries are simple `select * from t1 where...` queries then you're probably better using the ActiveRecord methods. It is a judgement call, use whatever is clearer to you and the people maintaining your code. – mu is too short Apr 29 '12 at 17:54
  • I think using ActiveRecord methods are clearer for me when reading them, but writing them is just as complex as writing raw sql queries for me! Thanks for the advice. – dannymcc Apr 29 '12 at 17:56
  • +1, but I'd also recommend using parameters over interpolation. – dwerner Apr 29 '12 at 17:56
  • @dwerner: You're stuck with interpolation if you have to use raw SQL since AR has such a bad attitude about databases. – mu is too short Apr 29 '12 at 18:04
  • I just looked that up. Apparently you need to use the base adapter/connection? Still, I'd go to the extra effort, but I see where you're coming from. – dwerner Apr 29 '12 at 18:06
  • @dwerner: You can use proper prepared statements and placeholders if you bypass ActiveRecord completely and talk straight to the database driver. But then you end up slinging strings and hashes around rather than ActiveRecord objects. AR is okay as long as you stay within the narrow confines of what it thinks you should be doing but things get ugly when you need to do more interesting database things. – mu is too short Apr 29 '12 at 18:18
  • @dannymcc: I added a more Railsy version of your query that might help you out. – mu is too short Apr 29 '12 at 18:19
  • i don't know if it's a mistake or not, but once you used `find_all_by_sql` and the next time you used `find_by_sql`. The first method doesn't exist in Rails 3, but the second one does. maybe i'm missing something, but just letting you know in case you want to correct yourself. also thnx for the answer :) – Zippie Apr 13 '13 at 12:23
  • @Zippie: That `find_all_by_sql` probably came from the question, I don't think I've ever actually used it. Might have been one of the `find_all_by_X`/`find_by_X` magical methods from the olden times. Anyway, thanks for the heads up, I switched it to `find_by_sql` to avoid confusion. – mu is too short Apr 13 '13 at 17:08