How can I speed up this query?

Question

I have code to generate four cars from a DB with their photos:

$l=$database->query("SELECT car,brand,exactprice FROM $table ORDER BY rand() LIMIT 4");
$buffer="";
foreach($l as $l){
    $buffer.="<li><h3>{$l['car']}</h3><p>Price {$l['exactprice']}</p>";
    $k=$database->query("SELECT logo FROM logo WHERE brand='{$l['brand']}'");
    $buffer.="<img src='{$base_addr}/{$k[0]['logo']}'><br>";
}

That's my current code. Can I rewrite my MySQL command so I would get logo from the logo with the first select I did?

Currently it takes 4.32 seconds to load this page.

-1, SQL-injectable code. You should know about this issue, you have been told before: http://stackoverflow.com/questions/6207763/mysql-insert-if-this-ip-dont-have-any-records — Johan, Jun 12 '11 at 22:06
its not injectable, $database->query does filter and make safe input and output, thanks for head up anywayz — Zalaboza, Jun 12 '11 at 22:13
no it doesn't **only** for values, not for table/column/database names, see my answer and the link in it for details. — Johan, Jun 12 '11 at 22:24
That doesn't make sense to me: `$database->query` receives a string, without any hints what it's supposed to mean. If `$l['brand']` == `';DROP TABLE logo; --`, how exactly will $database->query know that `SELECT logo FROM logo WHERE brand=''; DROP TABLE logo; --'` is *not* what you had in mind? — Piskvor left the building, Jun 12 '11 at 22:28

Jason McCreary · Accepted Answer · 2011-06-12T22:09:25.913

5

You didn't supply what $table was, but either way you could use a JOIN on the brand column (assuming that this column is the appropriate foreign key).

SELECT $table.car, $table.brand, $table.exactprice, logo.logo
FROM $table JOIN logo ON $table.brand = logo.brand
ORDER BY rand() LIMIT 4

As far as the performance, you could look into creating indexes on the columns you use frequently and benchmarking by adding EXPLAIN in front of the query above.

Some references to check out:

edited Jun 12 '11 at 22:09

answered Jun 12 '11 at 22:04

Jason McCreary

71,546
23
135
174

1

Good point about benchmarking; but even without specific data, I'd venture to say that calling `RAND()` for every row might be a significant CPU hog. – Piskvor left the building Jun 12 '11 at 22:15
@Piskvor , any other ideas about generating randomly 4 rows ? – Zalaboza Jun 12 '11 at 22:21
@Momen M El Zalabany: [Yes.](http://stackoverflow.com/questions/6324983/how-can-i-speed-up-this-query/6325044#6325044) – Piskvor left the building Jun 12 '11 at 22:23

score 1 · Answer 2 · answered Jun 12 '11 at 22:04

1

SELECT car, $table.brand, exactprice, logo FROM $table JOIN logo ON $table.brand=logo.brand ORDER BY rand() LIMIT 4 should do it, by joining the brand table into the query.

answered Jun 12 '11 at 22:04

James Allardice

164,175
21
332
312

Column 'brand' in field list is ambiguous – Zalaboza Jun 12 '11 at 22:16
Did you miss the `$table.` part before `brand` in the field list? It's ambiguous if you don't specify the table because the column `brand` appears in both tables. – James Allardice Jun 12 '11 at 22:18

score 1 · Answer 3 · answered Jun 12 '11 at 22:04

Change your script to this:

$l=$database->query("SELECT $table.car, logo.logo, $table.exactprice FROM $table join logo on logo.brand= $table.brand ORDER BY rand() LIMIT 4");
        $buffer="";
        foreach($l as $l){
            $buffer.="<li><h3>{$l['car']}</h3><p>Price {$l['exactprice']}</p>";
            $buffer.="<img src='{$base_addr}/{$l['logo']}'><br>

score 1 · Answer 4 · edited May 23 '17 at 10:00

1

There are 2 problems currently. First, this query should be done using a JOIN. The query to do so has already been suplied in other answers, so I won't repeat it again.

Also, in case you're wondering why it's slow, it's also related with the ORDER BY rand(). For that, you could check This Stackoverflow question

edited May 23 '17 at 10:00

Community

1
1

answered Jun 12 '11 at 22:08

Lumbendil

2,906
1
19
24

score 1 · Answer 5 · edited May 23 '17 at 10:00

Please fix the SQL-injection hole by doing:

$allowed_tables = array('table1', 'table2');
$table = $_POST['table']; //or wherever table come from.
if (in_array($table, $allowed_tables)) {
    $query = "SELECT car,brand,exactprice FROM `$table` ORDER BY rand() LIMIT 4";
    $l=$database->query($query);
    ......
}

Using parameters or mysql_real_escape_string() will not help you because you are using dynamic table names.
See also this question: How to prevent SQL injection with dynamic tablenames?

score 1 · Answer 6 · answered Jun 12 '11 at 22:13

Depending on your table sizes, indexes and structure, you may have other problems (check with EXPLAIN SELECT), but there's a potential issue in ORDER BY RAND() - that sorts the whole table, calling RAND() for every row, and then throws most of that away and gives you the first four rows; hugely inefficient. You could, however, do a neat trick with subqueries, for example something like this one from @Jan Kneschke's article:

SELECT name
  FROM random JOIN
       (SELECT CEIL(RAND() *
                    (SELECT MAX(id)
                       FROM random)) AS id
        ) AS r2
       USING (id);

Note that this will only give you randomly distributed results as long as there aren't any deleted rows (or they are evenly distributed); this may or may not be a problem, depending on the purpose of your query. See that article for an in-depth discussion: http://jan.kneschke.de/projects/mysql/order-by-rand/

Full disclosure: Not affiliated with that site, it simply has a great explanation. — Piskvor left the building, Jun 12 '11 at 22:18

score 0 · Answer 7 · answered Jun 12 '11 at 22:06

0

SELECT car,brand,exactprice, logo FROM $table left join logo on ($table.brand=logo.brand) ORDER BY rand() LIMIT 4

answered Jun 12 '11 at 22:06

i get this error :( Column 'brand' in field list is ambiguous – Zalaboza Jun 12 '11 at 22:16
that means its in both tables, so you specify which you want in the select – Jun 12 '11 at 22:43

How can I speed up this query?

7 Answers7