Winterbash 2014 "Red Shirt" Estimation

Question

Some fellow code reviewers (hi @Janos!) have been inquiring about a SEDE query to allow to check progress of the Red Shirt "hat" progression.

Try it here!

---

Background

Red Shirt

cast 5 downvotes on posts that are later deleted or closed

---

Limitations

There are certain Stack Exchange limitations which make querying this information a bit tricky. Namely:

• The data is only refreshed once a week, on Sundays. This makes it impossible to have "real time" results.

• User voting activity is anonymous, in that a user can only see their own voting activity in their own profile. This disallows joining voting and user data on SEDE.

Assumptions

I have made certain assumptions, based on the trends I generally see on closed questions. They are:

• A user who votes to close/delete a bad question will usually also downvote the question.

• A user who downvotes a question will usually do so before voting to close/delete.

Usage

To get usefulness out of this (and as indicated in the SQL comments at the top of the query):

The way that this report can be used is by comparing the results set side-by-side with your "votes" under your activity reports. Filter by down-votes and look to see if questions you down-voted are in the result set below.

For example:

Query

/* 
Winter Bash 2014
"Red Shirt" hat estimation
"Cast 5 downvotes on posts that are later deleted or closed"

The way that this report can be used is by comparing the results set 
side-by-side with your "votes" under your activity reports.
Filter by down-votes and look to see if questions you down-voted are
in the result set below.
*/

-- NumberWeeks: Number of weeks to go back

-- DATETIME VARIABLES

DECLARE @today DATETIME;
SET @today = CURRENT_TIMESTAMP;

DECLARE @weeks_ago INT;
SET @weeks_ago = ##NumberWeeks:int?4##;
-- Number of weeks must not go into the future, hence the following:
SET @weeks_ago = (CASE WHEN @weeks_ago >0 THEN -@weeks_ago ELSE @weeks_ago END);

DECLARE @target_week DATETIME;
SET @target_week = DATEADD(WEEK, @weeks_ago, CURRENT_TIMESTAMP);

-- POST-RELATED VARIABLES

DECLARE @downvote TINYINT;
SET @downvote = (SELECT Id FROM VoteTypes WHERE Name LIKE 'Down%'); --3

DECLARE @question_post TINYINT;
SET @question_post = (SELECT Id FROM PostTypes WHERE Name = 'Question'); --1

DECLARE @closed_post TINYINT;
SET @closed_post = (SELECT Id FROM PostHistoryTypes WHERE Name = 'Post Closed'); --10

DECLARE @deleted_post TINYINT;
SET @closed_post = (SELECT Id FROM PostHistoryTypes WHERE Name = 'Post Deleted'); --12

-- QUERY BEGINS

WITH cte_downvoted_posts AS
(
  SELECT Votes.PostId AS dvote
  FROM Votes
  INNER JOIN Posts ON Votes.PostId = Posts.Id
  WHERE VoteTypeId = @downvote
)

SELECT 
    Posts.Id AS [Post Link]               -- magic column
  , Posts.OwnerUserId AS [User Link]      -- magic column
  , Posts.CreationDate AS [Creation Date]
  , Posts.ClosedDate AS [Closed Date]

FROM Posts
INNER JOIN cte_downvoted_posts
    ON Posts.Id = cte_downvoted_posts.dvote
INNER JOIN PostHistory
    ON Posts.Id = PostHistory.PostId

WHERE Posts.CreationDate <= @today
  AND Posts.CreationDate > @target_week
  AND Posts.PostTypeId = @question_post
  AND Posts.ClosedDate IS NOT NULL

GROUP BY
    Posts.Id
  , Posts.OwnerUserId
  , Posts.CreationDate
  , Posts.ClosedDate

ORDER BY
    Posts.CreationDate DESC

Concerns

• Nitpicks are fine! Anything from naming to indentation to inconsistencies, please don't be shy to point out anything at all!

• I noticed a lot of nested loops in the execution plan, when it's doing joins. Is there a better way to do this to avoid them?

• Is there a way to make this query result set more useful, or user-friendly?

• Are my comments appropriate/useful? Should I have fewer, or more of them?

Answer 1

• You don't need to join to Posts at all in your CTE since you don't use any of its columns and you get a "Post must exist" requirement from your main query.
• Along the same lines, you don't use PostHistory at all in your query but join to it all the same.
• You declare @deleted_post but you never use it; you just set @closed_post twice (presumably incorrectly the second time to the ID of the "Deleted Post" row.)
• But then again, you don't use @closed_post or @deleted_post in your query, so why are you getting those exactly?
• Some None-1-2 testing would reveal if SE ever adds a new votetype beginning with "Down" your query will fail as you assume your @downvote variable will only be assigned a single scalar value. So either add TOP 1 to your query or change your WHERE clause to an = operator to prevent this. (Also, since you're only searching for the "DownMod" votetype, why the LIKE operator in the first place?)

I removed the CTE entirely and changed your Main Query to:

SELECT 
    Posts.Id AS [Post Link]               -- magic column
  , Posts.OwnerUserId AS [User Link]      -- magic column
  , Posts.CreationDate AS [Creation Date]
  , Posts.ClosedDate AS [Closed Date]

FROM Posts
INNER JOIN Votes
    ON Posts.Id = Votes.PostId

WHERE Posts.CreationDate <= @today
  AND Posts.CreationDate > @target_week
  AND Posts.PostTypeId = @question_post
  AND Posts.ClosedDate IS NOT NULL
  AND VoteTypeId = @downvote

GROUP BY
    Posts.Id
  , Posts.OwnerUserId
  , Posts.CreationDate
  , Posts.ClosedDate

ORDER BY
    Posts.CreationDate DESC

Personally, I would just call your Top 1 Subquery to get the DownMod VotetypeID and Question PostTypeID directly in the query, but that's just religion.

Answer 2

I find this pretty nicely done and really hard to pick on. But here's a little something. Instead of writing this condition this way:

WHERE Posts.CreationDate <= @today
  AND Posts.CreationDate > @target_week

This is shorter and simpler:

WHERE Posts.CreationDate BETWEEN @target_week AND @today

Keep in mind though that's not exactly the same, because if I remember correctly, BETWEEN uses <= and >= conditions. But even if so, it might not make a difference anyway in the case of this query. (I confirmed with 1 week to go back, the results are identical.)

Btw, @Malachi pointed out an excellent article about range queries and their dangers.

A very minor nitpick, but in this column alias I would spell out the "dvote" nicely as "downvote":

  SELECT Votes.PostId AS dvote

Answer 3

-- Number of weeks must not go into the future, hence the following:
SET @weeks_ago = (CASE WHEN @weeks_ago >0 THEN -@weeks_ago ELSE @weeks_ago END);

This boils down to taking the absolute value of the given parameter, so it could be simplified to:

SET @weeks_ago = abs(@weeks_ago)

I would expect the query to return 0 rows for invalid parameter values though.

Answer 4

Comments

-- NumberWeeks: Number of weeks to go back

-- DATETIME VARIABLES

DECLARE @today DATETIME;
SET @today = CURRENT_TIMESTAMP;

DECLARE @weeks_ago INT;
SET @weeks_ago = ##NumberWeeks:int?4##;

1. The comment about the number of weeks should be much closer to the declaration.
2. A better variable name would eliminate the need for this variable entirely.

SELECT 
    Posts.Id AS [Post Link]               -- magic column
  , Posts.OwnerUserId AS [User Link]      -- magic column

Okay, so it's a magic column? What does the magic column do? (For those that don't know, this makes special columns display as a hyperlink.)

Answer 5

Common Table Expressions are for all, intents and purposes, a Table and should be named accordingly,

WITH cte_downvoted_posts AS

Should be

WITH DownvotedPosts AS

Drop the Hungarian Naming style and go with Pascal for big name items like Tables and Columns.

All objects in the Database should be Pascal (In my opinion)

Answer 6

This is more of a feature request than a proper code review, but it might be nice to allow the user to pick particular topics to either filter or sort.

Also, a possibly quicker way to check against one's own history would be to simply click on the question link. If you downvoted it, the downvote button on that question will be highlighted.