Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

#Nitpicking#

Nitpicking

#SELECT DISTINCT#

SELECT DISTINCT

#Nitpicking#

#SELECT DISTINCT#

Nitpicking

SELECT DISTINCT

Source Link
Phrancis
  • 20.5k
  • 6
  • 69
  • 155

#Nitpicking#

Aliases/notation

Single-letter alias names are not very helpful. Aliases should at least give Mr. Maintainer a clue what it stands for. What if you had a table called family along with friends... would you call it F2?

FROM friends F
LEFT JOIN message_share S ON S.ouid_fk <> F.friend_two
LEFT JOIN messages M ON M.msg_id = S.msg_id_fk AND M.uid_fk = S.ouid_fk
LEFT JOIN myusertable U ON U.uid = M.uid_fk AND U.status1='1'

Also, I find it good practice to use the optional AS keyword, just looks better in my opinion. How about:

FROM friends AS Frnd
LEFT JOIN message_share AS Share ON Share.ouid_fk <> Frnd.friend_two
LEFT JOIN messages AS Msg ON Msg.msg_id = Share.msg_id_fk AND Msg.uid_fk = Share.ouid_fk
LEFT JOIN myusertable AS User ON User.uid = Msg.uid_fk AND Usr.status1 = '1'

Column names

Some of your column names are pretty cryptic. I realize you may not have any say into it, but if this is your database you may consider changing names like ouid_fk and such to something more meaningful, if a bit more verbose.

Indentation/spacing

Use line breaks and indentation to facilitate reading of your queries. Also consistent spacing is an improvement.

(SELECT DISTINCT M.msg_id, M.uid_fk, M.message, S.created, M.like_count,M.comment_count,M.share_count, U.username,M.uploads, S.uid_fk AS share_uid,S.ouid_fk AS share_ouid

Why not instead:

(SELECT DISTINCT 
 Msg.msg_id, 
 Msg.uid_fk, 
 Msg.message, 
 Share.created, 
 Msg.like_count,
 Msg.comment_count,
 Msg.share_count, 
 Usr.username,
 Msg.uploads, 
 Share.uid_fk AS share_uid,
 Share.ouid_fk AS share_ouid

INT in a VARCHAR column?

I find this odd:

AND Usr.status1 = '1'

To me in a well-designed database, this status1 column should be int (4 bytes), smallint (2 bytes) or tinyint (1 byte). Not to mention, status1 is not a very good column name.

#SELECT DISTINCT#

I don't feel that this is needed at all, for a couple of reasons:

  1. Do you expect multiple messages to have the same msg_id? Wouldn't that completely defeat the purpose of an ID?

  2. Your GROUP BY msg_id already does this, but it does the work on the result set rather than the source data set, which will likely be much faster.

Here is what I would refactor to. However there is no way for me to test performance since you did not provide DDL or sample data.

(SELECT 
 Msg.msg_id, 
 Msg.uid_fk, 
 Msg.message, 
 Share.created, 
 Msg.like_count,
 Msg.comment_count,
 Msg.share_count, 
 Usr.username,
 Msg.uploads, 
 Share.uid_fk AS share_uid,
 Share.ouid_fk AS share_ouid
FROM friends AS Frnd
LEFT JOIN message_share AS Share ON Share.ouid_fk <> Frnd.friend_two
LEFT JOIN messages AS Msg ON Msg.msg_id = Share.msg_id_fk AND Msg.uid_fk = Share.ouid_fk
LEFT JOIN myusertable AS User ON User.uid = Msg.uid_fk AND Usr.status1 = '1'
WHERE Frnd.friend_one='199095' AND Frnd.role='fri'
GROUP BY Msg.msg_id
ORDER BY Share.created DESC LIMIT 10)
UNION
(SELECT 
 Msg.msg_id, 
 Msg.uid_fk, 
 Msg.message, 
 Share.created, 
 Msg.like_count,
 Msg.comment_count,
 Msg.share_count, 
 Usr.username,
 Msg.uploads, 
 Share.uid_fk AS share_uid,
 Share.ouid_fk AS share_ouid
FROM friends AS Frnd
LEFT JOIN message_share AS Share ON Share.ouid_fk <> Frnd.friend_two
LEFT JOIN messages AS Msg ON Msg.msg_id = Share.msg_id_fk AND Msg.uid_fk = Share.ouid_fk
LEFT JOIN myusertable AS User ON User.uid = Msg.uid_fk AND Usr.status1 = '1'
WHERE Frnd.friend_one='199095'
GROUP BY Msg.msg_id
ORDER BY Share.created DESC LIMIT 10)
lang-sql

AltStyle によって変換されたページ (->オリジナル) /