Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

This SQL of yours has three issues I want to address before looking at the performance...

###Formatting:

Formatting:

SELECT c.id,
 c.fromUserId,
 c.toUserId,
 m.userId,
 m.message,
 m.timestamp,
 pm.filename AS thumbnail,
 u.username
FROM conversations c,
 messages m,
 productmedia pm,
 users u
WHERE (c.fromUserId = '9' OR c.toUserId = '9')
 AND c.id = m.conversationId
 AND m.timestamp = (
 SELECT MAX(m.timestamp)
 FROM messages m
 WHERE m.conversationId = c.id)
 AND c.productId = pm.productId
 AND pm.sortOrder = '0'
 AND m.userId = u.id
GROUP BY c.id
ORDER BY m.timestamp DESC

I am not advocating the above as the right way to indent SQL, but, it is consistent, and it allows you to more easily see what is what in the query. What's nice is that you have consistently capitalized the keywords, etc. so that's good.

I am familiar with the above indenting/layout, so, I can now more easily see the next two issues....

###Aliases:

Aliases:

You have the table messages twice in your query, once in the sub-select, and again in the main query. Both of these instances of the table have the alias m.... <sarcasm>this is not at all confusing... </sarcasm>

###Group By:

Group By:

Your group by clause is one of those things that will never be resolved to my satisfaction. MySQL has what I consider to be a bug.....(they call it a feature) ..... a massive, oh-my-god, how-could-they-do-that, what-were-they-thinking bug.

What does this mean practically? It means that your query makes no sense to anyone other than a MySQL expert. Your query makes no sense to me.... I can't help. I don't know what the expected behaviour is in your code. Your query does not even contain any aggregated values? Huh!

###Next steps:

Next steps:

  • Figure out what you are grouping by (or even if the group-by is needed).
  • pull an explain-plan from the database when it runs the query.

This SQL of yours has three issues I want to address before looking at the performance...

###Formatting:

SELECT c.id,
 c.fromUserId,
 c.toUserId,
 m.userId,
 m.message,
 m.timestamp,
 pm.filename AS thumbnail,
 u.username
FROM conversations c,
 messages m,
 productmedia pm,
 users u
WHERE (c.fromUserId = '9' OR c.toUserId = '9')
 AND c.id = m.conversationId
 AND m.timestamp = (
 SELECT MAX(m.timestamp)
 FROM messages m
 WHERE m.conversationId = c.id)
 AND c.productId = pm.productId
 AND pm.sortOrder = '0'
 AND m.userId = u.id
GROUP BY c.id
ORDER BY m.timestamp DESC

I am not advocating the above as the right way to indent SQL, but, it is consistent, and it allows you to more easily see what is what in the query. What's nice is that you have consistently capitalized the keywords, etc. so that's good.

I am familiar with the above indenting/layout, so, I can now more easily see the next two issues....

###Aliases:

You have the table messages twice in your query, once in the sub-select, and again in the main query. Both of these instances of the table have the alias m.... <sarcasm>this is not at all confusing... </sarcasm>

###Group By:

Your group by clause is one of those things that will never be resolved to my satisfaction. MySQL has what I consider to be a bug.....(they call it a feature) ..... a massive, oh-my-god, how-could-they-do-that, what-were-they-thinking bug.

What does this mean practically? It means that your query makes no sense to anyone other than a MySQL expert. Your query makes no sense to me.... I can't help. I don't know what the expected behaviour is in your code. Your query does not even contain any aggregated values? Huh!

###Next steps:

  • Figure out what you are grouping by (or even if the group-by is needed).
  • pull an explain-plan from the database when it runs the query.

This SQL of yours has three issues I want to address before looking at the performance...

Formatting:

SELECT c.id,
 c.fromUserId,
 c.toUserId,
 m.userId,
 m.message,
 m.timestamp,
 pm.filename AS thumbnail,
 u.username
FROM conversations c,
 messages m,
 productmedia pm,
 users u
WHERE (c.fromUserId = '9' OR c.toUserId = '9')
 AND c.id = m.conversationId
 AND m.timestamp = (
 SELECT MAX(m.timestamp)
 FROM messages m
 WHERE m.conversationId = c.id)
 AND c.productId = pm.productId
 AND pm.sortOrder = '0'
 AND m.userId = u.id
GROUP BY c.id
ORDER BY m.timestamp DESC

I am not advocating the above as the right way to indent SQL, but, it is consistent, and it allows you to more easily see what is what in the query. What's nice is that you have consistently capitalized the keywords, etc. so that's good.

I am familiar with the above indenting/layout, so, I can now more easily see the next two issues....

Aliases:

You have the table messages twice in your query, once in the sub-select, and again in the main query. Both of these instances of the table have the alias m.... <sarcasm>this is not at all confusing... </sarcasm>

Group By:

Your group by clause is one of those things that will never be resolved to my satisfaction. MySQL has what I consider to be a bug.....(they call it a feature) ..... a massive, oh-my-god, how-could-they-do-that, what-were-they-thinking bug.

What does this mean practically? It means that your query makes no sense to anyone other than a MySQL expert. Your query makes no sense to me.... I can't help. I don't know what the expected behaviour is in your code. Your query does not even contain any aggregated values? Huh!

Next steps:

  • Figure out what you are grouping by (or even if the group-by is needed).
  • pull an explain-plan from the database when it runs the query.
Source Link
rolfl
  • 98.1k
  • 17
  • 219
  • 419

This SQL of yours has three issues I want to address before looking at the performance...

###Formatting:

SELECT c.id,
 c.fromUserId,
 c.toUserId,
 m.userId,
 m.message,
 m.timestamp,
 pm.filename AS thumbnail,
 u.username
FROM conversations c,
 messages m,
 productmedia pm,
 users u
WHERE (c.fromUserId = '9' OR c.toUserId = '9')
 AND c.id = m.conversationId
 AND m.timestamp = (
 SELECT MAX(m.timestamp)
 FROM messages m
 WHERE m.conversationId = c.id)
 AND c.productId = pm.productId
 AND pm.sortOrder = '0'
 AND m.userId = u.id
GROUP BY c.id
ORDER BY m.timestamp DESC

I am not advocating the above as the right way to indent SQL, but, it is consistent, and it allows you to more easily see what is what in the query. What's nice is that you have consistently capitalized the keywords, etc. so that's good.

I am familiar with the above indenting/layout, so, I can now more easily see the next two issues....

###Aliases:

You have the table messages twice in your query, once in the sub-select, and again in the main query. Both of these instances of the table have the alias m.... <sarcasm>this is not at all confusing... </sarcasm>

###Group By:

Your group by clause is one of those things that will never be resolved to my satisfaction. MySQL has what I consider to be a bug.....(they call it a feature) ..... a massive, oh-my-god, how-could-they-do-that, what-were-they-thinking bug.

What does this mean practically? It means that your query makes no sense to anyone other than a MySQL expert. Your query makes no sense to me.... I can't help. I don't know what the expected behaviour is in your code. Your query does not even contain any aggregated values? Huh!

###Next steps:

  • Figure out what you are grouping by (or even if the group-by is needed).
  • pull an explain-plan from the database when it runs the query.
lang-sql

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