"Catrope" changed the status of MediaWiki.r110520 to "fixme" and commented it.
URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/110520#c31535
Old Status: new
New Status: fixme
Commit summary for MediaWiki.r110520:
AFT5: Fix query-continue support for rating/helpfulness sorting. Refs r110412,
stil has a lingering issue with ratings sort.
Catrope's comment:
<pre>
+CREATE INDEX /*_*/af_net_helpfulness_af_id ON /*_*/aft_article_feedback
(af_id, af_net_helpfulness);
</pre>
This index is the wrong way around, because you're doing:
<pre>
+ $order = "af_net_helpfulness $direction, af_id $direction";
</pre>
<pre>
- $limit = 25, $continue = null ) {
+ $limit = 25, $continue = null, $continueId ) {
</pre>
Adding a non-optional parameter after an optional parameter is bad form, please
make this one optional as well (default to null is probably fine).
<pre>
+ $order = "rating $direction, af_id $direction";
+ $continueSql = "(rating.aa_response_boolean $continueDirection ".intVal(
$continue )
</pre>
It took me a while to figure out that you have a table called 'rating' and a
field alias called 'rating', and that the latter is an alias for
rating.aa_response_boolean . That's really confusing, please don't do that.
The query that orders by rating looks like it's not going to sort efficiently.
I couldn't tell you offhand how it could be written more efficiently, because
of the excessive normalization going on. The page ID and the rating value are
in different tables, so I don't think you're gonna be able to pull that off
without cross-table indexes (which MySQL doesn't have). Also, why are you
selecting and sorting on aa_response_'''boolean''' here? Isn't the rating an
integer type rather than a boolean type?
OK otherwise.
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
