Marius
Marius

Reputation: 47

INNER JOIN on 3 tables with SUM()

I am having a problem trying to JOIN across a total of three tables:

I want to have 1 query that returns a set of all users, their cap, their used bandwidth for this month and their adhoc purchases for this month:

< TABLE 1 ><TABLE2><TABLE3>
User   | Cap | Adhoc | Used
marius | 3   | 1     | 3.34
bob    | 1   | 2     | 1.15
(simplified)

Here is the query I am working on:

SELECT
        `msi_adsl`.`id`,
        `msi_adsl`.`username`,
        `msi_adsl`.`realm`,
        `msi_adsl`.`cap_size` AS cap,
        SUM(`adsl_adhoc`.`value`) AS adhoc,
        SUM(`radacct`.`AcctInputOctets` + `radacct`.`AcctOutputOctets`) AS used
FROM
        `msi_adsl`
INNER JOIN
        (`radacct`, `adsl_adhoc`)
ON
        (CONCAT(`msi_adsl`.`username`,'@',`msi_adsl`.`realm`) 
           = `radacct`.`UserName` AND `msi_adsl`.`id`=`adsl_adhoc`.`id`)

WHERE
        `canceled` = '0000-00-00'
AND
        `radacct`.`AcctStartTime`
BETWEEN
        '2010-11-01'
AND
        '2010-11-31'
AND
        `adsl_adhoc`.`time`
BETWEEN
        '2010-11-01 00:00:00'
AND
        '2010-11-31 00:00:00'
GROUP BY
        `radacct`.`UserName`, `adsl_adhoc`.`id` LIMIT 10

The query works, but it returns wrong values for both adhoc and used; my guess would be a logical error in my joins, but I can't see it. Any help is very much appreciated.

Upvotes: 1

Views: 3756

Answers (2)

Jonathan Leffler
Jonathan Leffler

Reputation: 754190

Your query layout is too spread out for my taste. In particular, the BETWEEN/AND conditions should be on 1 line each, not 5 lines each. I've also removed the backticks, though you might need them for the 'time' column.

Since your table layouts don't match your sample query, it makes life very difficult. However, the table layouts all include a UserID (which is sensible), so I've written the query to do the relevant joins using the UserID. As I noted in a comment, if your design makes it necessary to use a CONCAT operation to join two tables, then you have a recipe for a performance disaster. Update your actual schema so that the tables can be joined by UserID, as your table layouts suggest should be possible. Obviously, you can use functions results in joins, but (unless your DBMS supports 'functional indexes' and you create appropriate indexes) the DBMS won't be able to use indexes on the table where the function is evaluated to speed the queries. For a one-off query, that may not matter; for production queries, it often does matter a lot.

There's a chance this will do the job you want. Since you are aggregating over two tables, you need the two sub-queries in the FROM clause.

SELECT u.UserID,
       u.username,
       u.realm,
       u.cap_size AS cap,
       h.AdHoc,
       a.OctetsUsed
  FROM msi_adsl AS u
  JOIN (SELECT UserID, SUM(AcctInputOctets + AcctOutputOctets) AS OctetsUsed
          FROM radact
         WHERE AcctStartTime BETWEEN '2010-11-01' AND '2010-11-31'
         GROUP BY UserID
       )    AS a ON a.UserID = u.UserID
  JOIN (SELECT UserID, SUM(Value) AS AdHoc
          FROM adsl_adhoc
         WHERE time BETWEEN '2010-11-01 00:00:00' AND '2010-11-31 00:00:00'
         GROUP BY UserId
       )    AS h ON h.UserID = u.UserID
 WHERE u.canceled = '0000-00-00'
 LIMIT 10

Each sub-query computes the value of the aggregate for each user over the specified period, generating the UserID and the aggregate value as output columns; the main query then simply pulls the correct user data from the main user table and joins with the aggregate sub-queries.

Upvotes: 3

dabal
dabal

Reputation: 420

I think that the problem is here

FROM  `msi_adsl`
INNER JOIN
        (`radacct`, `adsl_adhoc`)
ON
        (CONCAT(`msi_adsl`.`username`,'@',`msi_adsl`.`realm`)
           = `radacct`.`UserName` AND `msi_adsl`.`id`=`adsl_adhoc`.`id`)

You are mixing joins with Cartesian product, and this is not good idea, because it's a lot harder to debug. Try this:

FROM  `msi_adsl`
INNER JOIN
        `radacct`
ON
      CONCAT(`msi_adsl`.`username`,'@',`msi_adsl`.`realm`) = `radacct`.`UserName`
JOIN  `adsl_adhoc` ON  `msi_adsl`.`id`=`adsl_adhoc`.`id`

Upvotes: 0

Related Questions