3
\$\begingroup\$

Following this linked question and the feedback it generated, I put together a new query which utilizes a JOIN.

WITH UpvotedComments AS (
  SELECT
    PostId,
    Score,
    Text,
    --Comments.UserDisplayName doesn't return users' current display names
    Commenter = (
      SELECT DisplayName
      FROM Users
      WHERE Id = UserId
    )
  FROM Comments
  WHERE Score > 0
), ClosedPosts AS (
  SELECT Id, ClosedDate
  FROM Posts
  WHERE ClosedDate IS NOT NULL
)

SELECT Score, Text, Commenter

FROM UpvotedComments

INNER JOIN ClosedPosts ON UpvotedComments.PostId = ClosedPosts.Id

ORDER BY Score DESC, ClosedDate DESC

It can get a bit slow on sites like Stack Overflow, where the returns are greater than 50 000.

\$\endgroup\$
1
\$\begingroup\$

Overall, your code is good, but there are a couple of performance improvements that were pointed out above. Here is a more explicit answer on how to improve the performance of the code provided.

Improvement 1: Eliminate subqueries in SELECT

Subqueries in the select statement referencing an external table will take a major hit in performance (row by row look vs set based join). In this case, we can avoid that by joining the Users table. We're assuming Id is the primary key in Users, so no duplicates to worry about. We are doing an inclusive join, which assumes all UserIds have a corresponding record in the Users table (typical foreign key constraint).

Improvement 2: Eliminate redundant external references

We reference the Posts table twice in the original query, once to limit to closed posts and another time to sort by the ClosedDate column. Instead, we can do an inclusive join to Posts, set our criteria either in the WHERE clause or in the JOIN, and do the sort, so that it is all done in one pass. We again assume that Id in Posts is a primary key, so no duplicates to worry about.

SELECT
  c.PostId,
  c.Score,
  c.Text,
  u.DisplayName AS Commenter,
  p.ClosedDate
FROM 
  Comments c
--Improvement 1, inclusive join to Users
JOIN
  Users u ON u.Id = c.UserId
--Improvement 2, inclusive join to Posts
JOIN
  Posts p ON p.Id = c.PostId 
WHERE 
  c.Score > 0
  AND p.ClosedDate IS NOT NULL --Improvement 2, Closed posts only
ORDER BY 
  c.Score DESC, 
  p.ClosedDate DESC --Improvement 2, sort by ClosedDate

Also, if possible, you can typically gain a performance improvement by moving the sort into the application layer.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks, this really helpful. Are single-letter variable names standard in T-SQL? \$\endgroup\$ – Amani Kilumanga Feb 14 '16 at 6:47
  • 1
    \$\begingroup\$ Np! Abbreviations are common, but more descriptive aliasing is useful particularly in more complex queries. \$\endgroup\$ – vanlee1987 Feb 14 '16 at 17:39
2
\$\begingroup\$

SEDE has two great features that you should incorporate into your code:

  • Input parameters
  • Magic markup columns

Input parameters

SEDE lets you declare fields to be input towards the query, like so:

##Parameter:type?defaultvalue##

And so, I suggest you use three of them for the following things:

  • Threshold (The threshold of comment scores to select)
  • Amount per page (The amount per page of comments)
  • Page number (To go with the above one)

By using pages, you can simplify your returning dataset and help to reduce the massive call times of the query.


Magic markup columns

SEDE has a great feature where certain columns can be transformed into special markup. The specific columns are:

  • Post Link (generated from Post Id)
  • User Link (generated from User Id)
  • Comment Link (generated from Comment Id)
  • Suggested Edit Link (generated from Suggested Edit Id)
  • Tags

As it stands...

your code is good, but you are doing a few things you can improve on:

  • Your generation of Commenter can be easily replaced with UserId as [User Link] to save extraneous calling of another table.
  • SELECT Id, ClosedDate your SELECTion is redundant. To use the field, you don't actually have to select it, simply reference it. Seeing as the resulting table is only used for the Id anyway, it's pointless to have it.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks! I wasn't aware of the magic markup columns (I was looking for something like user link in the schema). However, I prefer using the display name as seen in Users.DisplayName. As for the redundant selection, I believe the ClosedDate is used for ordering, but I will have to look into referencing. \$\endgroup\$ – Amani Kilumanga Jan 30 '16 at 10:55

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.