2
\$\begingroup\$

In "How Not To Sort By Average Rating", Evan Miller explained in simple terms and demonstrated with compelling examples that the usual naïve methods of calculating the average rating are flawed, and that the maths for this problem has been worked out almost a century ago—(lower bound of) the Wilson score confidence interval:

$$ \left(\hat{p} + \frac{z_{α/2}^2}{2n}\pm z_{α/2}\sqrt{[\hat{p}(1-\hat{p}) + z_{α/2}^2/4n]/n}\right)/(1 + z_{α/2}^2/n) $$ (Use minus where it says plus/minus to calculate the lower bound.) Here \$\hat{p}\$ is the observed fraction of positive ratings, \$z_{α/2}\$ is the \$(1-α/2)\$ quantile of the standard normal distribution, and \$n\$ is the total number of ratings.

Take \$α\$ to be 0.05, i.e. assume 95% confidence interval; from the Standard normal table, the z value corresponding to \$1 - 0.05/2 = 0.97500\$ is 1.96. Hence the hard-coded magic numbers 1.96, 1.9208, 0.9604 and 3.8416, which represent \$z_{α/2}\$, \$z_{α/2}^2/2\$, \$z_{α/2}^2/4\$ and \$z_{α/2}^2\$, respectively, at \$α = 0.05\$.

I decided to put the concepts described therein to use on rating tags on the main site of a Stack Exchange subnetwork. I considered rating questions and answers (including the community wikis) separately, and defined a positive rating as an upvote and a negative rating as a downvote. I filtered tags with 0 votes to avoid division by 0.

This SEDE query is the result:

DECLARE @SQLString nvarchar(MAX);
DECLARE @SQLSubStringQ nvarchar(2000);
DECLARE @SQLSubStringA nvarchar(2000);
DECLARE @positive nvarchar(100) = 'CAST(SUM(CASE WHEN Votes.VoteTypeId = 2 THEN 1 ELSE 0 END) AS numeric(19, 9))';
DECLARE @negative nvarchar(100) = 'CAST(SUM(CASE WHEN Votes.VoteTypeId = 3 THEN 1 ELSE 0 END) AS numeric(19, 9))';
SET     @SQLSubStringQ = 'SELECT ((' + @positive + ' + ' + '1.9208) / (' + @positive + ' + ' + @negative + ') -
                                  1.96 * SQRT((' + @positive + ' * ' + @negative + ') / (' + @positive + ' + ' + @negative + ') + 0.9604) / 
                                         (' + @positive + ' + ' + @negative + ')) / (1 + 3.8416 / (' + @positive + ' + ' + @negative + '))
                          FROM Posts
                          INNER JOIN Votes
                          ON Posts.Id = Votes.PostId
                          WHERE Posts.Id = ANY (SELECT a.PostId FROM PostTags a WHERE a.TagId = PostTags.TagId)
                          HAVING ' + @positive + ' + ' + @negative + ' > 0';
SET     @SQLSubStringA = 'SELECT ((' + @positive + ' + ' + '1.9208) / (' + @positive + ' + ' + @negative + ') -
                                  1.96 * SQRT((' + @positive + ' * ' + @negative + ') / (' + @positive + ' + ' + @negative + ') + 0.9604) / 
                                         (' + @positive + ' + ' + @negative + ')) / (1 + 3.8416 / (' + @positive + ' + ' + @negative + '))
                          FROM Posts
                          INNER JOIN Votes
                          ON Posts.Id = Votes.PostId
                          WHERE Posts.ParentId = ANY (SELECT a.PostId FROM PostTags a WHERE a.TagId = PostTags.TagId)
                          HAVING ' + @positive + ' + ' + @negative + ' > 0';
SET     @SQLString = CAST('' as nvarchar(MAX)) + -- Workaround: https://stackoverflow.com/questions/4833549/nvarcharmax-still-being-truncated/17785175#17785175
                      'SELECT Tags.TagName,
                       Tags.Count AS QuestionCount,
                       Sum(Posts.Score) AS NetQuestionScore,
                       (' + @SQLSubStringQ + ') AS QuestionRatings,
                       Sum(Posts.AnswerCount) AS AnswerCount,
                       (
                         SELECT COALESCE(SUM(a.Score), 0) FROM Posts a WHERE a.ParentId = ANY 
                         (SELECT b.PostId FROM PostTags b WHERE b.TagId = PostTags.TagId)
                       ) AS NetAnswerScore,
                       (' + @SQLSubStringA + ') AS AnswerRatings
                       FROM Tags
                       INNER JOIN PostTags
                       ON Tags.Id = PostTags.TagId
                       INNER JOIN Posts
                       ON PostTags.PostId = Posts.Id
                       AND Posts.PostTypeId IN (1, 2, 3)
                       WHERE (' + @SQLSubStringQ + ') IS NOT NULL OR (' + @SQLSubStringA + ') IS NOT NULL
                       GROUP BY PostTags.TagId, Tags.TagName, Tags.Count
                       ORDER BY Tags.TagName ASC';
EXECUTE(@SQLString);

But this not only looks abominable, it's SLOW.

  1. How can the readability be improved?
  2. Any way to boost the performance so that it can be run on larger SE sites?
  3. Any suggestion for a version 2?
\$\endgroup\$
  • 1
    \$\begingroup\$ Any time you EXECUTE SQL code from a string it will be slow, that's the nature of the beast. I recommend against it unless it's absolutely necessary, \$\endgroup\$ – Phrancis Sep 28 '17 at 0:35
  • \$\begingroup\$ Regarding your question 3, what are you wanting to parameterize? Also, where do all these numbers come from? For example, 1.9208, 1.96 and 3.8416... \$\endgroup\$ – Phrancis Sep 28 '17 at 1:08
  • 1
    \$\begingroup\$ You can parameterize in pure SQL using stored procedures and functions just fine, I just didn't know what you were looking for. I've been inspecting your code, if you can clarify that aspect of parameterization I can likely offer a good review about it. \$\endgroup\$ – Phrancis Sep 28 '17 at 2:01
  • \$\begingroup\$ No, SEDE does not allow stored procedures/functions to be created (as it gives no write permissions to us) \$\endgroup\$ – Phrancis Sep 28 '17 at 2:12
  • 1
    \$\begingroup\$ This question is lacking something important: a description of what it's actually doing. Since the code is fairly hard to read, a description is a must. \$\endgroup\$ – Mast Sep 28 '17 at 15:24
1
\$\begingroup\$

So it seems that dynamic SQL in general and EXECUTE() in particular, as pointed out by Phrancis, is slow. As I also don't want to be dealing with the composition of the query string, I set out to rewrite the query in static SQL, now with CTE as is available on SEDE.

DECLARE @Question tinyint = 1
DECLARE @Answer tinyint = 2
DECLARE @UpMod tinyint = 2
DECLARE @DownMod tinyint = 3
DECLARE @z numeric(19, 9) = 1.96
DECLARE @zz numeric(19, 9) = SQUARE(@z)
;
WITH TotalUpDownVotesByTags_VIEW AS (
    SELECT
          Tags.TagName
        , PostTypes.Name AS PostType
        , COUNT(DISTINCT Posts.Id) AS PostCount
        , CAST(SUM(CASE
                       WHEN Votes.VoteTypeId = @UpMod THEN 1
                       ELSE 0
                   END)
               AS numeric(19, 9))
          AS PostUpvotes
        , CAST(SUM(CASE
                       WHEN Votes.VoteTypeId = @DownMod THEN 1
                       ELSE 0
                   END)
               AS numeric(19, 9))
          AS PostDownvotes
    FROM Posts
        INNER JOIN PostTypes
            ON Posts.PostTypeId = PostTypes.Id
        INNER JOIN PostTags
            ON (CASE
                    WHEN Posts.PostTypeId = @Question THEN Posts.Id
                    WHEN Posts.PostTypeId = @Answer THEN Posts.ParentId
                END) = PostTags.PostId
        INNER JOIN Tags
            ON PostTags.TagId = Tags.Id
        LEFT JOIN Votes
            ON Posts.Id = Votes.PostId
    GROUP BY Tags.TagName, PostTypes.Name
)

SELECT
      TagName
    , PostType
    , PostCount
    , PostUpvotes
    , PostDownvotes
    , COALESCE((PostUpvotes + @zz / 2 - @z * SQRT(PostUpvotes * PostDownvotes
                  / NULLIF(PostUpvotes + PostDownvotes, 0) + @zz / 4))
                  / (PostUpvotes + PostDownvotes + @zz), 0)
      AS PostRatings
FROM TotalUpDownVotesByTags_VIEW
ORDER BY PostRatings DESC

This version is substantially shorter, cleaner, and faster: SEDE reports 2501 rows returned in 117 ms vs 1303 rows returned in 11174 ms in the old one, which is a whooping 100× improvement in performance; I have done away with magic numbers and string concatenation, so readability is improved; by using CTE, not using sub-queries and just having a better query structure really cuts down the SLOC count (the string would fit nicely in a nvarchar(2000) field, less than half the space required by the old version).

The other changes I've done include simplifying the math a bit (canceling out the \$n\$'s in the numerator and denominator), and also showing tags with zero voting and no answers for completeness' sake. The query can be tweaked to not be grouped by PostTypes.Name if it's commented out in the three places it appears, but I don't know how to parameterize that without resorting to EXECUTE(), which would be going back the old way.

\$\endgroup\$
  • \$\begingroup\$ Much nicer already. As far as your grouping question, a simpler (though more verbose) way to do this is by using temp tables and IF/ELSE branching. I put together a simple example using SEDE that you can adapt for your needs. See the comments I included in the query. Hope this helps! \$\endgroup\$ – Phrancis Oct 24 '17 at 12:17
  • \$\begingroup\$ @Phrancis This is what I feared: it's basically writing the whole thing again. It seems I have to rewrite my CTE as a temp table to have IF/ELSE branching and not get an error from SEDE. It keeps complaining about the two group by columns, that I think it's just simpler to have the two versions in separate queries. Thanks for replying though. \$\endgroup\$ – Gao Oct 30 '17 at 8:27

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.