Error in SQL query, unsure how to fix

Question

I am getting an error in a query:

Error: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'group by `messages`.`to_id` UNION SELECT `users`.`username` as 'subj' at line 13

I didn't write the query, and not sure what union does so not sure how to fix it.

select `thread`.`subject`,
       `thread`.`id`, 
       max(`thread`.`mostrecent`) as 'mostrecent',
       sum(`thread`.`from` + `thread`.`to`) as 'messages' 
from (
    SELECT `users`.`username` as 'subject', 
           `messages`.`to_id` as 'id' , 
           max(`messages`.`created`) as 'mostrecent',
           count(*) as 'from',
           0 as 'to' from `messages` 
           join `users` on `messages`.`to_id` = `users`.`id` 
           where `messages`.`from_id` = $id 
           group by `messages`.`to_id` 
    UNION 
    SELECT `users`.`username` as 'subject',
           `messages`.`from_id` as 'id', 
           max(`messages`.`created`) as 'mostrecent', 
           0 as 'from', 
           count(*) as 'to' from `messages` 
           join `users` on `messages`.`from_id` = `users`.`id` 
           where `messages`.`to_id` = $id
           group by `messages`.`from_id`
) as thread group by `thread`.`subject` order by max(`thread`.`mostrecent`) desc

You should learn to indent queries properly - it will save you a lot of pain in the future. — Raj More, Oct 15 '14 at 12:52
My apologies. I blamed you there outright. I do stand by the fact that un-indented code is harder to deal with, so if you have the chance to correct it, please consider it. — Raj More, Oct 15 '14 at 13:03
@BenShepherd the reason why its throwing an error at the group by is because $id is not escaped with single quotes... however thats a really bad way of putting parameters into a query [**READ MY POST HERE**](http://stackoverflow.com/a/26367414/2733506) — John Ruddell, Oct 15 '14 at 13:03

score 2 · Answer 1 · edited May 23 '17 at 11:49

you cannot order by an aggregate function and you dont have quotes around your $id which php requires... see my note at the end about sql injection

SELECT `thread`.`subject`,
       `thread`.`id`, 
       max(`thread`.`mostrecent`) as 'mostrecent',
       sum(`thread`.`from` + `thread`.`to`) as 'messages' 
from (
    SELECT `users`.`username` as 'subject', 
           `messages`.`to_id` as 'id' , 
           max(`messages`.`created`) as 'mostrecent',
           count(*) as 'from',
           0 as 'to' from `messages` 
           join `users` on `messages`.`to_id` = `users`.`id` 
           where `messages`.`from_id` = $id 
-- ------------------------------------^---^ = needs quotes
           group by `messages`.`to_id` 
    UNION 
    SELECT `users`.`username` as 'subject',
           `messages`.`from_id` as 'id', 
           max(`messages`.`created`) as 'mostrecent', 
           0 as 'from', 
           count(*) as 'to' from `messages` 
           join `users` on `messages`.`from_id` = `users`.`id` 
           where `messages`.`to_id` = $id
-- ----------------------------------^---^ = needs quotes
           group by `messages`.`from_id`
) as thread group by `thread`.`subject` order by max(`thread`.`mostrecent`) desc
-- -----------------------------------------------^------------------------^ = bad

instead

try changing it to just mostrecent because you already pulled out the max and gave it the alias mostrecent.. you can reference an alias in anything after the WHERE so GROUP BY and beyond including the ORDER BY

ORDER BY mostrecent DESC

so the final query should look like this...

SELECT 
    `thread`.`subject`,
    `thread`.`id`, 
    MAX(`thread`.`mostrecent`) AS 'mostrecent',
    SUM(`thread`.`from` + `thread`.`to`) AS 'messages' 
FROM 
(   SELECT 
        `users`.`username` AS 'subject', 
        `messages`.`to_id` AS 'id' , 
        MAX(`messages`.`created`) AS 'mostrecent',
        COUNT(*) AS 'from',
        0 AS 'to' 
    FROM `messages` 
    JOIN `users` ON `messages`.`to_id` = `users`.`id` 
    WHERE `messages`.`from_id` = '$id' 
    GROUP BY `messages`.`to_id` 

    UNION 

    SELECT 
        `users`.`username` AS 'subject',
        `messages`.`from_id` AS 'id', 
        MAX(`messages`.`created`) AS 'mostrecent', 
        0 AS 'from', 
        COUNT(*) AS 'to' 
    FROM `messages` 
    JOIN `users` ON `messages`.`from_id` = `users`.`id` 
    WHERE `messages`.`to_id` = '$id'
    GROUP BY `messages`.`from_id`
) AS thread 
GROUP BY `thread`.`subject` 
ORDER BY mostrecent DESC

NOTE

this query is vunerable to sql injection and I would recommend you parameterize your query and bind the $id to the query afterwards

you should read my post about writing a safer query...

I'm aware of the vulnerability, the id is grabbed from the database and cannot be changed by the user anywhere. — Ben, Oct 15 '14 at 14:20
Ben as @tadman said on [**my post**](http://stackoverflow.com/a/26367414/2733506) *Being disciplined about using parameterized queries means the chance of an escaping bug slipping through is really low. Manually escaping some things is always risky and should be done only as an absolute last resort even if you're "sure" the data is fine. Where that data originates could change in the future and you might be caught wide open.* it's just a good idea to practice it for the future — John Ruddell, Oct 15 '14 at 14:35

score 0 · Answer 2 · answered Oct 15 '14 at 12:51

Both your inner queries, and your outer query should be reworked to include both columns on the GROUP BY instead of just one:

select 
    `thread`.`subject`,
    `thread`.`id`, 
    max(`thread`.`mostrecent`) as 'mostrecent',
    sum(`thread`.`from` + `thread`.`to`) as 'messages' 
from 
(
    SELECT 
        `users`.`username` as 'subject', 
        `messages`.`to_id` as 'id' , 
        max(`messages`.`created`) as 'mostrecent',
        count(*) as 'from',
        0 as 'to' 
    FROM `messages` 
    JOIN `users` on `messages`.`to_id` = `users`.`id` 
    WHERE `messages`.`from_id` = $id 
    GROUP BY `users`.`username`, `messages`.`to_id` 
UNION 

    SELECT 
        `users`.`username` as 'subject',
        `messages`.`from_id` as 'id', 
        max(`messages`.`created`) as 'mostrecent', 
        0 as 'from', 
        count(*) as 'to' 
    FROM `messages` 
    JOIN `users` on `messages`.`from_id` = `users`.`id` 
    WHERE `messages`.`to_id` = $id
    GROUP BY `users`.`username`, `messages`.`from_id`
) as thread 
GROUP BY `thread`.`subject`, `thread`.`id`
ORDER BY max(`thread`.`mostrecent`) desc

Error in SQL query, unsure how to fix

2 Answers2

instead

NOTE