[wp-trac] [WordPress Trac] #30478: Comment query results returned in an inconsistent order on some versions of MySQL
WordPress Trac
noreply at wordpress.org
Tue Dec 23 18:28:14 UTC 2014
#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
More information about the wp-trac
mailing list