Did you like my product?

Question

I have the following MySQL query which selects product details and whether the viewing user likes them. I feel like it could be more efficient and that there is a more practical approach than using a sub-query here.

Any suggestions?

SELECT p.*, 
         pm.filename AS thumbnail,
         (SELECT COUNT(id) FROM likes WHERE productId = p.id AND 
                userId = '$viewingUserId' AND isActive = '1' LIMIT 1) 
            as liked
            FROM products p, productmedia pm
        WHERE p.userId = '$userId'
        AND p.id = pm.productId
        AND pm.sortOrder = '0'
        ORDER BY p.timestamp DESC

Answer 1

More important than the performance, your code is very likely vulnerable to SQL Injection. (I can't say for sure without looking at your PHP code)

If $viewingUserId or $userId is chosen by the user in any way, then you have a problem. And even if it is not, I would highly recommend using prepared queries.

I don't know if you're using the mysql_* methods or mysqli_* methods or the PDO extention, but you should be aware that the mysql_* methods are deprecated (That means: Don't use them) and that you should either use mysqli (note the i) or PDO.

Answer 2

Naming

You should name your aliases in such a way that looking at them tells you something about their meaning. p and pm tell nothing to the next programmer who will have to maintain your code. It's better to type a few extra letters and add clarity, like prod and prMedia.

Using SELECT *

SELECT * is not very efficient as SQL has to go back and look up every column's DDL in your information schema. Only SELECT the columns you actually need for faster query.

Old-style JOIN

You have this:

FROM products p, productmedia pm
        WHERE p.userId = '$userId'
        AND p.id = pm.productId
        AND pm.sortOrder = '0'

This is bad practice and will likely become deprecated eventually. Instead, use an explicit type of join. I will use INNER JOIN in this case.

Numbers stored as text?

I saw isActive = '1' and prMedia.sortOrder = '0'. It's better to use INT for integers and BOOLEAN for true/false fields.

The modified script

I eliminated your subquery by working it into the WHERE clause.

SELECT 
    prod.*, 
    prMedia.filename AS thumbnail,
    COUNT(prod.id) AS liked
FROM products AS prod
  INNER JOIN productmedia AS prMedia
    ON prod.userId = '$userId'
    AND prod.id = prMedia.productId
WHERE prod.userId = '$viewingUserId' 
    AND prMedia.sortOrder = '0'
    AND prod.isActive = '1' 
ORDER BY prod.timestamp DESC
LIMIT 1;

Answer 3

You're going to have to forgive me here if I get the syntax not quite right. I primarily work with PostgreSQL, not MySQL.

Unless the user is allowed to like an item more than once, there's no reason to perform a count: the user has either liked the item or they haven't. If there are no restrictions on how often the user is allowed to favorite in the database, there should be (judging from your query, you'd be needing a partial unique index in a PostgreSQL database to ensure there's only 1 active like per user/product).

To get the simplest query, you'll want to use a LEFT JOIN rather than a subquery. This query assumes that you want to select items the user hasn't favorited (which your current query doesn't appear to do):

SELECT
    products.*, 
    productmedia.filename AS thumbnail,
    CASE WHEN likes.productId IS NOT NULL THEN true ELSE false END AS liked
FROM
    products
    JOIN productmedia ON productmedia.productId = products.id
    LEFT JOIN likes ON likes.productId = products.id AND userId = ? AND isActive = 1
WHERE
    productmedia.sortOrder = '0'
ORDER BY
    products.timestamp DESC;