Winterbash 2014 "Red Shirt" Estimation

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
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
(
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?

• 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

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.

• I did notice I left in some variables I had previously intended to use, from an older versiom which did not turn out very well. Nice catch and nice review! Dec 20 '14 at 15:57

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

• those two where statements are not the same. between is a little weird, it acts like a WHERE x < Date AND x > otherDate and doesn't include the start and end date.
– Malachi
Dec 19 '14 at 20:22
• there is a couple of posts on StackOverflow. give me a second. looks like this answer is saying something a little different than what I am saying, what do you think? stackoverflow.com/a/16347680/1214743
– Malachi
Dec 19 '14 at 20:25
• Aaron Bertrand is pretty well known on SO his blog talks about Dates and SQL Server pretty thoroughly. sqlblog.com/blogs/aaron_bertrand/archive/2009/10/16/…
– Malachi
Dec 19 '14 at 20:31
-- 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.

• I've never heard of ABS() that's a good idea! Dec 19 '14 at 20:30

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

• The magic-column thing is sede specific though... Anyone using sede probably has stumbled upon them a few times.. Dec 19 '14 at 20:52
• Maybe @Vogel612. Maybe not. What's that do? Dec 19 '14 at 20:54

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)

• Fair enough.. but CTE is technically not a database object.. Dec 19 '14 at 20:13
• how technical are we talking here? it's a Temporary Table that is destroyed after the transaction. @Mat'sMug
– Malachi
Dec 19 '14 at 20:16

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.

• Good idea, perhaps adding tags as options would be good! Dec 19 '14 at 20:28
• I've hacked this this query to order by the first tag. Dec 19 '14 at 20:42