1
\$\begingroup\$

I have this really long query, which in my opinion looks ugly next to all the tidy code in my project. I was wondering if there was a way to optimize it or make it shorter at least:

$query = <<<SQL
    SELECT * FROM users 
    WHERE NOT EXISTS (
        SELECT id
        FROM profile_likes 
        WHERE profile_id = users.id
        AND viewer_id = $my_user->id
    )
    AND NOT EXISTS (
        SELECT id
        FROM profile_dislikes
        WHERE profile_id = users.id
        AND viewer_id = $my_user->id
    )
    AND id != $my_user->id
SQL

This query is a part of an user filter which has the purpose of filtering the users to only those that the currently logged in user ($my_user) hasn't liked/disliked yet.

\$\endgroup\$
  • \$\begingroup\$ Be sure that those interpolated variables are not user input. If they could be, you need to use parameterization or you have no security. \$\endgroup\$ – Aluan Haddad Feb 11 '18 at 4:51
  • 1
    \$\begingroup\$ None of the variables are user input. \$\endgroup\$ – Ivan Grigorov Feb 11 '18 at 5:04
  • 1
    \$\begingroup\$ Roger, I was just checking, it's amazing how often people forget. Regarding your question, it seems MySQL is still crap and doesn't support many basic set operations. I think the best option is to use UNION and then do the sub-query, but someone may have a much better suggestion. \$\endgroup\$ – Aluan Haddad Feb 11 '18 at 5:23
  • 1
    \$\begingroup\$ I appreciate the concern. It happens to me quite often to forget to parametrize quries. I'll follow your suggestion and look into using UNION. Thanks. \$\endgroup\$ – Ivan Grigorov Feb 11 '18 at 5:59
2
\$\begingroup\$

Do you really need to keep likes and dislikes in different tables?

Seems to me that these would be pretty much the same thing with a different value in one single field. So maybe a profile_reactions table instead. Doing this would simplify your data model down to two tables and probably be more scalable in the future, if/when you want to ad more reaction types.

SELECT * should typically be avoided. It makes it harder for readers of the code to understand what fields they have to work with in the result set (they would have to go look at database schema to understand this). It can be more bandwidth intensive than necessary. It you only want a few fields from a table with a large number of fields, why pull all the extra data? It can potentially make the code more fragile. Can a database schema change potentially introduce bugs in code?

Consider whether relational storage (i.e. MySQL) is right for this in the long term. Since you are going to be dealing with most of the records in these tables, this is a query that really could be problematic over time, getting slower and slower as your user base grows. It simply doesn't scale well. The primary question to ask is whether the likes/dislikes (reactions) have any meaning outside the context of an owning user. My guess is that they don't. And if they don't, why does this data need to be normalized into it's own table?

This data model is one that actually works quite well in NoSQL storage, where the data model might look like this:

// User
{
  "id": 123,
  "reactions_to_users": {
    234: "like",
    456: "dislike",
    789: "like",
    ...
  }
  "reactions_from_users": {
    234: "like",
    456: "dislike",
    111: "like",
  },
  ...
}

So assuming that you would already have this user object loaded up earlier in code (like you do know). When you go to make your query against the data store to get the the un-liked/disliked users, you would simply need to get the user ids under reactions_to_users and make a query against the data store for all users outside of this set.

I think your actual SQL code in PHP looks good. I prefer using heredoc or nowdoc syntax like you are doing. I also prefer SQL the read more vertically than horizontally, breaking up at each SQL phrase like you have done. I wouldn't change your style here.

\$\endgroup\$
3
\$\begingroup\$

Personally, I would just stick with what you have already got.

You could try something like this, although too me it doesn't look any tidier. You would need to profile it against your database, to see if it performs any better.

SELECT * 
FROM users
left outer join profile_likes on profile_likes.profile_id = users.id AND profile_likes.viewer_id = $my_user->id
left outer join profile_dislikes on profile_dislikes.profile_id = users.id AND profile_dislikes.viewer_id = $my_user->id
where users.id <> $my_user->id
having profile_likes.profile_id is null and profile_dislikes.profile_id is null
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.