Opened 19 months ago
Last modified 5 weeks ago
#47642 reviewing defect (bug)
Order by comment count - posts list tables
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Posts, Post Types | Keywords: | has-screenshots has-patch needs-unit-tests early |
Focuses: | administration | Cc: |
Description
Results from posts list tables with enabled ordering by comment count are mixed (posts are missing and aren't unique). Pagination required.
Links:
wp-admin/edit.php?orderby=comment_count&order=asc
wp-admin/edit.php?orderby=comment_count&order=asc&paged=2
No matter which post type.
You need to set 'Number of items per page' in screen options in order to get a pagination.
Set sorting by comment count in comments column.
In lists of posts some of them are missing. Page number 1 and number 2 don't have unique posts.
I was testing it for some cases:
30+ posts and 'posts_per_page' = 20
30+ posts and 'posts_per_page' = 10
250+ posts and 'posts_per_page' = 20
24 pages and 'posts_per_page' = 20
Results in all cases were wrong.
The official queries are from wp-includes/class-wp-query.php (line: 2984) https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-query.php#L2984, variable: $this->request. Below applied filter are generated ids for current posts.
SQL queries from debugging (official queries):
Page number 1 (paged=1):
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'future' OR wp_posts.post_status = 'draft' OR wp_posts.post_status = 'pending' OR wp_posts.post_status = 'private') ORDER BY wp_posts.comment_count ASC LIMIT 0, 20
Page number 1 (paged=2):
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'future' OR wp_posts.post_status = 'draft' OR wp_posts.post_status = 'pending' OR wp_posts.post_status = 'private') ORDER BY wp_posts.comment_count ASC LIMIT 20, 20
What's funny if I added 'wp_posts.post_title' to query the problem is solved (unique posts):
I think that problem is actually in the database queries/ settings what's caused the bad output in admin's panel.
For other sorting like title or date this problem doesn't occur.
Attachments (4)
Change History (28)
#2
@
16 months ago
OK, reproduced this .. it has to do with the fact that te results are ordered by a column that has the very same information. Thus is *might* be possible that de database engine spits the results to PHP in a (lets call it) *random* order. Thanks @herregroen for pointing that out
#3
@
16 months ago
so .. here is the fix. We need an extra unique (!) colum to sort by AFTER the comment count has been sorted. I used post_modified in this case.
#5
@
16 months ago
for the record: tested with pagination of 20 using 23 posts ( all had no comments )
#6
@
16 months ago
Thanks for the patch @ramon-fincken. The post_modified field is also non-unique. It's possible that two posts could share the same comment count and modified date.
The secondary sort order should probably be the post ID instead.
#9
@
10 months ago
It works for me... also, to maintain consistency, should we check the same thing for the "modified" (default) option?
So, if we order and modified two posts at the same exact time... we may have the same problem, so it should be and option to order by the modified and ID field.
#10
@
9 months ago
Chances are that you collide with 2 very same (second) updates are less than having posts with zero comments if you ask me.
#12
@
7 months ago
- Milestone changed from Awaiting Review to 5.6
- Owner set to johnbillion
- Status changed from new to reviewing
- Version 5.2.2 deleted
#13
@
4 months ago
- Keywords needs-unit-tests added; needs-testing removed
Thanks for the patch @ramon-fincken . I think this adjustment needs to be made at a lower level, inside the WP_Query
class. That way, any code that's ordering posts by comment count will have the order corrected, not just queries for that screen in the admin area.
47642.diff makes that change.
I'm working on some tests for this now.
In addition, this problem affects more than just the comment_count
ordering, the main one being ordering by date. Let's tackle this first, then look at dates in a separate ticket.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
3 months ago
#16
@
3 months ago
- Keywords early added
- Milestone changed from 5.6 to 5.7
Punting from 5.6 and marking for 5.7-early, this is one of those things that is incredibly annoying and weird for users but we have to be quite careful with changes to this area.
Related: #44349
In particular:
Would there be a case when the orderby should not have ID?
I don't believe so, no. This change does have potential to break custom filters that are doing string replacements on the resulting SQL though.
This ticket was mentioned in Slack in #core by helen. View the logs.
3 months ago
#20
@
3 months ago
here we go .. this patch tests for numeric values and datetime values.
order can happen on string OR array , so we need to test for both of them.
Also .. if it is an array, we do NOT need to add the ID if it is already present
see patch: patch.47642.20201030.1
This ticket was mentioned in Slack in #core by ramonfincken. View the logs.
3 months ago
#22
@
3 months ago
Thanks for the updated patch Ramon, I'll take a look at this once work starts on 5.7.
alltough I see that your ID resultset in mysql has duplicates I cannot reproduce this using:
posts with 0, 1, 2, 3, 3, 4 comments
paginated 2
I do not have overlapping or missing posts in the list.
Still this might me an error because of the post_status. Could you run the queries and do them only for the publish post status?