[wp-trac] [WordPress Trac] #30478: Comment query results returned in an inconsistent order on some versions of MySQL

[WordPress Trac]

#30478: Comment query results returned in an inconsistent order on some versions of
MySQL
--------------------------+------------------
 Reporter:  boonebgorges  |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  4.2
Component:  Comments      |     Version:
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:
--------------------------+------------------
Changes (by boonebgorges):

 * keywords:  needs-patch needs-unit-tests => has-patch
 * milestone:  Future Release => 4.2


Comment:

 [attachment:30478.diff] is a first pass at the improved orderby
 functionality in comment queries. The new syntax is exactly the same as in
 the case of `WP_Query`: `'orderby' => array( 'comment_date_gmt' => 'ASC',
 'user_id' => 'DESC' )`, etc. A couple of comments about the
 implementation:

 - `comment_ID` is the only guaranteed-unique column, so it must always be
 used as the tiebreaker in order to resolve the issue described in this
 ticket. So in 30478.diff, if it's detected that 'comment_ID' is not one of
 the columns explicitly declared in 'orderby', it's tacked on at the end.
 The ASC|DESC value for the 'comment_ID' clause is determined as follows:
 if ordering by 'comment_date' or 'comment_date_gmt', inherit that
 ASC|DESC, otherwise grab the first ASC or DESC available in the collected
 orderby clauses and use it. The patch contains unit tests describing these
 various cases.
 - Previously, the orderby clause did not use a table alias, except when
 joining against commentmeta (so, `comment_date ASC` rather than
 `wp_comments.comment_date ASC`). Reproducing this logic ended up being
 overly complicated, so I've gone ahead and added the `$wpdb->comments`
 alias for all values of orderby. This required changing a few existing
 unit tests for the new SQL syntax. In certain edge cases, this might
 affect plugins that are doing sloppy manipulation of the SQL string at the
 'comments_clauses' filter. I assume that we aren't concerned with this
 kind of breakage.
 - For demonstration purposes, I've added `sleep()` statements into
 `test_orderby_meta()` so that I could regularize the failure conditions
 that very occasionally pop up (they depend on comment_date_gmt being
 different, which is why sleeping for a second or two does the trick). See
 https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/44397988. The
 patch fixes these cases. These `sleep()` commands should not be committed
 - they're just for testing/demonstration.

 I think this is a nice enhancement to the API, bringing it closer in line
 with `WP_Query`. It also has the benefit of fixing the odd orderby bugs we
 see in some Travis builds. Feedback welcome.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/30478#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform