# What are the top voted comments on closed questions, and who made the comments?

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
)
WHERE Score > 0
), ClosedPosts AS (
SELECT Id, ClosedDate
FROM Posts
WHERE ClosedDate IS NOT NULL
)

SELECT Score, Text, Commenter

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.

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
--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.

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

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.
• 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. – Amani Kilumanga Jan 30 '16 at 10:55