this SQL appears to be giving weird results with multiple JOINS, how do I correct it?

Question

I have several Rails Active Record Models: ContactEmails, ContactCalls, ContactPostalcards.

Each one represent a completed activity: a completed email, call, or postalcard to a specific Contact, and each Contact belongs to a Company.

I want a high-level summary by Company of all the Emails, Calls, and Postalcards within a specific date range:

def self.get_list(report_start_date, report_end_date)

    self.find :all,
      :select => "companies.name AS co_name, 
              companies.id AS comp_id, 
              COUNT(contact_emails.id) AS email_count, 
              COUNT(contact_calls.id) AS call_count, 
              COUNT(contact_letters.id) AS letter_count, 
              COUNT(contact_postalcards.id) AS postalcard_count",

      :conditions => ['contact_emails.date_sent < ? and contact_emails.date_sent > ? or
                       contact_calls.date_sent < ? and contact_calls.date_sent > ?', 
                  report_end_date, report_start_date, report_end_date, report_start_date],

      :joins => [
        "LEFT JOIN companies ON companies.id = contacts.company_id",
        "LEFT JOIN contact_emails ON contact_emails.contact_id = contacts.id",
        "LEFT JOIN contact_letters ON contact_letters.contact_id = contacts.id",
        "LEFT JOIN contact_postalcards ON contact_postalcards.contact_id = contacts.id",
        "LEFT JOIN contact_calls ON contact_calls.contact_id = contacts.id"
       ],
     #:group => "companies.id"
      :group => "companies.name"
  end

This is how I output the matrix in my View:

<% @matrix_summary.each do |item| %>      
<tr>
   <td><%= link_to item.co_name, company_path(item.comp_id) %></td>
   <td><%= item.email_count %> </td>
   <td><%= item.postalcard_count %></td>
   <td><%= item.call_count %></td>
<% end %>
</tr>

My expectation is each row should represent a Company, with the respective Email, Postalcard and Call counts for that one company (across all Contacts).

What I get is the same value (and a wrong value at that) for all Activities.

What am I doing that's not right?

Answer 1

Run your server log, (tail -f log/development.log) and then run your action in browser. See the query it generated in the logs. Copy that query, and execute it in a SQL console. Does it give the right answers?

Secondly, if you want to perform so heavy customization of Rails' find, I would recommend you rather use a) find_by_sql or b) ActiveRecord::Base.connection.execute() etc.

Answer 2

What your query appears to be doing (I know SQL but not Rails) is returning a line for each postcard, email etc which are then grouped. Count simply counts the number of lines in the group so the number you get will be the total number of the 4 types.

I suspect you will need to use a managed SQL loop to loop through the customers and then to count the records in each category for each customer and then summarize the information.

Answer 3

I suggest changing each of your COUNT(fieldname) to be COUNT(DISTINCT fieldname) - so for example, COUNT(contact_emails.id) AS email_count would become COUNT(DISTINCT contact_emails.id) AS email_count .

Answer 4

I assume that your models look like this:

# Company model
has_many :contacts
has_many :contact_emails, :through => :contacts
has_many :contact_calls, :through => :contacts
has_many :contact_letters, :through => :contacts
has_many :contact_postalcard, :through => :contacts

# Contact model
has_many :contact_emails
has_many :contact_calls
has_many :contact_letters
has_many :contact_postalcards

# ContactEmail model
scope :from, lambda {|start| where("date_sent > ?", start)}
scope :to, lambda {|stop| where("date_sent < ?", stop)}

# ContactCall model
scope :from, lambda {|start| where("date_sent > ?", start)}
scope :to, lambda {|stop| where("date_sent < ?", stop)}

Then you can load what you want for each company like this:

emails_count = @company.contact_emails.from(some_date).to(some_date2).size
letters_count = @company.contact_letters.size
etc.

This solution will run many sql queries, but is good and clean for begining.

You can also write queries that load all emails within date range and count it with grouping by company. In this solution you would only have 5 queries (to load company and each "count").

EDIT:

I think that 50 to 100 queries is "not so bad" if it is not run too often. Always you can start with this and when your application is in production, then just look where are the slowest parts and then optimize them. However this query looks like it will probably cause some problems, so I suggest that you go in the way @Tim proposed. I just took a look on it and it seems that it is what you are looking for. But you should also have code that is in my answer, because you will probably need most of it in your application and you can use it to see if @Tim's approach is giving correct results.

According to you question, which companies you should show. So, you have some @user which is current user and in User model you have has_many :companies, so:

# in controller
# this should load all companies that belongs to @user and has at least one contact
@companies = @user.companies.joins(:contacts).where("contacts.id is not null")

# in view
<% @companies.each do |item| %>
  <tr>
    <td><%= link_to item.co_name, company_path(item.comp_id) %></td>
    <td><%= item.contact_emails.from(some_date).to(some_date2).size %> </td>
    <td><%= item.contact_postalcards.size %></td>
    <td><%= item.contact_calls.from(some_date).to(some_date2).size %></td>
 </tr>
<% end %>

If one company doesn't have any calls, but has emails contacts, it will just display 0 in calls column and some number in emails column.

Answer 5

If you have few companies, than @klew's suggestion is probably the way to go. If you have many and only want to run one query, here's a gist which illustrates using subqueries to get all of the counts: http://gist.github.com/602944

It's split into two examples, the first is really introductory to how the subqueries will work, the second removes all the redundancy so it's easier to edit in the future.