HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlMinor

Naruto, where are you?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
narutowhereyouare

Problem

During Winter Bash 2014, the Naruto hat was awarded to users who gave an answer that got accepted but hasn't received votes for 12 hours.
It was a weird thing, really:
why would someone ever accept a question but not upvote it?
I haven't yet experienced a situation where this would be reasonable.

Accepted answers without upvotes seem to be an anomaly:
it happens when the OP doesn't have enough reputation to upvote,
and the post gets overlooked by other users in the community.
The community might actually want to clean these up.

To find eligible posts I put together this simple SEDE query (latest version, NOT for review):

DECLARE @username as NVarchar(60) = RTRIM(LTRIM(##DisplayName:string? ##));
DECLARE @userId as int = ##UserId:int?-1##;

SELECT TOP 100
  u.id as [User Link],
  p.Id as [Post Link],
  p.CreationDate
FROM Posts p
  JOIN Users u ON p.OwnerUserId = u.Id
  JOIN Posts q ON p.ParentId = q.Id
WHERE 
  (@username = '' OR u.DisplayName = @username)
  AND (@userId = -1 OR u.Id = @userId)
  AND p.Score = 0
  AND p.Id = q.AcceptedAnswerId
ORDER BY p.CreationDate DESC


The query accepts two parameters:

  • Optional display name, to filter by username



  • Optional user id, to filter by user id



  • With default values it will show all eligible posts of the given site



  • It won't make sense to set both display name and user id, but such misuse would be the user's fault



Something I don't like about this is the username parameter.
I would have wanted NULL or empty string as default value but couldn't figure out how to do that.
Using space does work, but it's not exactly pretty,
I'm wondering if there's a better way I'm missing.

I'm interested in any other suggestions to improve this query.

Solution

There is not a whole lot to say, but I found a few small things that could be improved.

Consistency

DECLARE @username as NVarchar(60) = RTRIM(LTRIM(##DisplayName:string? ##));
DECLARE @userId as int = ##UserId:int?-1##;
DECLARE @limit as int = ##Limit:int?100##;


Would be better as:

DECLARE @userName AS NVARCHAR(60) = RTRIM(LTRIM(##DisplayName:string? ##));
DECLARE @userId AS INT = ##UserId:int?-1##;
DECLARE @limit AS INT = ##Limit:int?100##;


Aliases

I think your aliases mostly obfuscate the query. It's also recommended to explicitly state the type of join, e.g.:

FROM Posts AS posts
  INNER JOIN Users AS users ON p.OwnerUserId = u.Id
  INNER JOIN Posts AS questions ON p.ParentId = q.Id


Trim

Your left and right trim operations don't really achieve anything. Do you expect a user name to have a bunch of white space before or after it? I'm not sure SE would even allow that. I removed them and got identical results.

Everything combined:

DECLARE @username AS NVARCHAR(60) = ##DisplayName:string? ##;
DECLARE @userId AS INT = ##UserId:int?-1##;
DECLARE @limit AS INT = ##Limit:int?100##;

SELECT TOP 100
  users.Id AS [User Link],
  posts.Id AS [Post Link],
  posts.CreationDate
FROM Posts AS posts
  INNER JOIN Users AS users ON posts.OwnerUserId = users.Id
  INNER JOIN Posts AS questions ON posts.ParentId = questions.Id
WHERE 
  (@username = '' OR users.DisplayName = @username)
  AND (@userId = -1 OR users.Id = @userId)
  AND posts.Score = 0
  AND posts.Id = questions.AcceptedAnswerId
ORDER BY posts.CreationDate DESC


Eliminating the empty string check

There is a way to eliminate the @username = '' check, however it does have a performance impact. I ran it and it runs in ~400 ms, which is not bad but a bit slower. It makes the index scan on the [Users].[UIX_Users_Id] table take longer due to partial string match.

WHERE
  u.DisplayName LIKE CONCAT('%', @username, '%')
  AND (@userId = -1 OR u.Id = @userId)
  AND p.Score = 0
  AND p.Id = q.AcceptedAnswerId

Code Snippets

DECLARE @username as NVarchar(60) = RTRIM(LTRIM(##DisplayName:string? ##));
DECLARE @userId as int = ##UserId:int?-1##;
DECLARE @limit as int = ##Limit:int?100##;
DECLARE @userName AS NVARCHAR(60) = RTRIM(LTRIM(##DisplayName:string? ##));
DECLARE @userId AS INT = ##UserId:int?-1##;
DECLARE @limit AS INT = ##Limit:int?100##;
FROM Posts AS posts
  INNER JOIN Users AS users ON p.OwnerUserId = u.Id
  INNER JOIN Posts AS questions ON p.ParentId = q.Id
DECLARE @username AS NVARCHAR(60) = ##DisplayName:string? ##;
DECLARE @userId AS INT = ##UserId:int?-1##;
DECLARE @limit AS INT = ##Limit:int?100##;

SELECT TOP 100
  users.Id AS [User Link],
  posts.Id AS [Post Link],
  posts.CreationDate
FROM Posts AS posts
  INNER JOIN Users AS users ON posts.OwnerUserId = users.Id
  INNER JOIN Posts AS questions ON posts.ParentId = questions.Id
WHERE 
  (@username = '' OR users.DisplayName = @username)
  AND (@userId = -1 OR users.Id = @userId)
  AND posts.Score = 0
  AND posts.Id = questions.AcceptedAnswerId
ORDER BY posts.CreationDate DESC
WHERE
  u.DisplayName LIKE CONCAT('%', @username, '%')
  AND (@userId = -1 OR u.Id = @userId)
  AND p.Score = 0
  AND p.Id = q.AcceptedAnswerId

Context

StackExchange Code Review Q#77437, answer score: 9

Revisions (0)

No revisions yet.