Cleaner way to write this MySql query

Question

I generate below query through php

$sql .= "Select * from test";

$sub_sql = '';

foreach($value as $n_k => $n_v)
{
    switch($n_v)
    {
        case '1':
            $sub_sql .= " OR (id = 1 )"; 
        break;
        case '2':
            $sub_sql .= " OR (category = 4) ";
        break;
        case '3':
            $sub_sql .= " OR (location = 'U.S.') ";
        break;  
    }
}

if($sub_sql != '')
{
    $sql .= " where ( 0 ".$sub_sql." ) ";
}

$sql .= "GROUP BY id  ORDER BY stored_date DESC LIMIT 0, 5 ";

But as you'll can see where $subsql is concatenated to $sql, that part looks really messy. Evn though it works fine but it looks really poor. Can anyone pls help me with this?

For example I think it would help to remove double whitespaces, and spell mysql keywords capitalized. Furthermore this seems to be off-topic because you don't really have a question about coding, but there's the SE Codereview http://codereview.stackexchange.com site, try it there. — idmean, Mar 12 '14 at 17:18
@wumm First of all thanks for you comment. secondly I really don't mind having the whitespaces and the capitalization as it is not really going to affect the result. Also I very well understand that I can use SE Codereview for this but already saw some of the similar question so thought I also could give it a try as I am desperately in need to get the answer as I can't really think of any other solution + my project leader wont allow such queries also. Thanx again. — Grish, Mar 12 '14 at 17:31
If the goal is to get rid of the dynamic sql, you will need to parameterize the query, see this question to guide you http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php — Will P., Mar 12 '14 at 17:42
So what is REALLY the problem here? Your project leader doesn't like your code formatting? Well why don't you agree on a coding style guide? Sometimes you have the need to build queries dynamically like this. I don't really see the problem here other than I would agree with comment by @wumm that your are stylistically inconsistent. Maybe something different could be done with your loop/switch construct, but it's hard to say without understanding what is in `$value`. — Mike Brant, Mar 12 '14 at 18:14
@MikeBrant problem here is that as you can see where I concatenate both the queries it is something like `WHERE (0 OR $n_v = 2 AND category = 4)` I think which is not correct as I add `0` to the query. — Grish, Mar 15 '14 at 02:23

fancyPants · Answer 1 · 2014-03-15T07:23:09.747

Is this a simplified example? Because I see no reason not to simplify it like this:

$sql .= "Select * from test where a=1";

foreach($value as $n_k => $n_v)
{
    switch($n_v)
    {
        case '1':
            $sql .= " AND (id = 1 )"; 
        break;
        case '2':
            $sql .= " AND (category = 4)";
        break;
        case '3':
            $sql .= " AND (location = 'U.S.')";
        break;  
    }
}

$sql .= " ORDER BY stored_date DESC LIMIT 0, 5 ";

Note, that I removed the GROUP BY from your statement. You shouldn't use it, if you don't use aggregate functions. Group by collapses group of rows and displays random rows from each group. If you have a need to remove duplicate rows, consider to use the DISTINCT keyword in the SELECT clause.

UPDATE:

Your new version can be shortened like this:

$sql .= "Select * from test";

foreach($value as $n_k => $n_v)
{
    switch($n_v)
    {
        case '1':
            $sql .= " where (id = 1 ) "; 
        break;
        case '2':
            $sql .= " where (category = 4) ";
        break;
        case '3':
            $sql .= " where (location = 'U.S.') ";
        break;  
    }
}

$sql .= " GROUP BY id  ORDER BY stored_date DESC LIMIT 0, 5 ";

Basically it comes down to this. If you have to add multiple AND conditions, you always add a WHERE 1=1 (if you have no other fix (non-dynamic) conditions) to the query. It doesn't change anything, the optimizer will throw it away again. But you can easily append as many conditions as you want. That's what I did in my original answer before your edit. Note though, that you can not just append OR whatever = true when you are mixing AND and OR conditions. You will have to use parantheses to not get a false resultset. This is where your (0 OR ...) thing comes from. Since you only have 1 condition, there's no need at all for all I said right now. Just add it as WHERE whatever = true and that's it.

@fancyPants I'll surely keep in mind next time about the group by's. thanks. I have updated the question, Sorry my bad. — Grish, Mar 15 '14 at 02:19
@fancyPants but I dont think it will work because if more than one condition is satisfied then it will keep on appending `where` as it goes in a loop and I will end up getting sql errors — Grish, Mar 20 '14 at 19:09

score 0 · Answer 2 · answered Mar 12 '14 at 21:10

0

How about adding the conditions to the sql?

Something like:

Select * from test where a=1
AND 
(
  ($n_v = 1 AND id = 1)
  OR
  ($n_v = 2 AND category = 4)
  OR
  ($n_v = 3 AND location = 'U.S.')
)

answered Mar 12 '14 at 21:10

FJT

1,923
1
15
14

Cleaner way to write this MySql query

2 Answers2