WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#14035 closed defect (bug) (fixed)

get_boundary_post() sorts by ID rather than creation date

Reported by: jk0 Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.0.1
Component: Performance Keywords: has-patch commit
Focuses: Cc:

Description

When get_boundary_post calls get_posts, posts are sorted by their ID rather than creation date. In other words, if old posts were imported or post IDs are skewed (for whatever reason), the 'start' <link /> generated in the header will not be accurate.

I propose sorting by post creation date. Patch attached.

Attachments (3)

14035.diff (536 bytes) - added by jk0 4 years ago.
Changed get_posts to use orderby's default (post_date).
14035.2.diff (788 bytes) - added by scribu 4 years ago.
omit no_found_rows; correct spelling
14035.3.diff (834 bytes) - added by scribu 4 years ago.
bring back accidentally removed params

Download all attachments as: .zip

Change History (14)

comment:1 jk04 years ago

  • Cc jk0 added

comment:2 scribu4 years ago

  • Milestone changed from Unassigned to 3.1

comment:3 scribu4 years ago

Actually, I think the orderby parameter should be left out, so that the default is used.

jk04 years ago

Changed get_posts to use orderby's default (post_date).

comment:4 jk04 years ago

  • Version changed from 3.0 to 3.0.1

comment:5 tigertech4 years ago

This bug is a more serious problem than it initially appears. It can cause performance problems on WordPress sites with thousands of posts.

The reason is that the final SQL it creates when sorting by ID is:

SELECT SQL_CALC_FOUND_ROWS  wp_posts.* FROM wp_posts  WHERE 1=1  AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish')  ORDER BY wp_posts.ID ASC LIMIT 0, 1;

However, there is no valid MySQL index on this combination. So MySQL loads all the data from the entire wp_posts table where wp_posts.post_type = 'post' and wp_posts.post_status = 'publish' (which is most of them), then does a filesort, then throws away all but one of the rows.

Here's the MySQL "EXPLAIN" output for one of these queries:

mysql> explain SELECT   wp_posts.* FROM wp_posts  WHERE 1=1  AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish')  ORDER BY wp_posts.ID ASC LIMIT 0, 1;
+----+-------------+----------+------+------------------+------------------+---------+-------------+------+-----------------------------+
| id | select_type | table    | type | possible_keys    | key              | key_len | ref         | rows | Extra                       |
+----+-------------+----------+------+------------------+------------------+---------+-------------+------+-----------------------------+
|  1 | SIMPLE      | wp_posts | ref  | type_status_date | type_status_date | 124     | const,const | 3792 | Using where; Using filesort |
+----+-------------+----------+------+------------------+------------------+---------+-------------+------+-----------------------------+
1 row in set (0.00 sec)

Compare this to the "EXPLAIN" output if the bug is fixed and it correctly sorts by post_date:

mysql> explain SELECT   wp_posts.* FROM wp_posts  WHERE 1=1  AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish')  ORDER BY wp_posts.post_date ASC LIMIT 0, 1;
+----+-------------+----------+------+------------------+------------------+---------+-------------+------+-------------+
| id | select_type | table    | type | possible_keys    | key              | key_len | ref         | rows | Extra       |
+----+-------------+----------+------+------------------+------------------+---------+-------------+------+-------------+
|  1 | SIMPLE      | wp_posts | ref  | type_status_date | type_status_date | 124     | const,const | 3793 | Using where |
+----+-------------+----------+------+------------------+------------------+---------+-------------+------+-------------+
1 row in set (0.00 sec)

There's no "filesort" in the correct version (and "filesort" is bad).

When the wp_posts table isn't in the MySQL or disk cache, I've seen the first version take more than 5 seconds occasionally (it shows up in the MySQL slow query log).

So fixing this bug will not only correct the behavior of the link in the post header; it will avoid a significant performance issue.

(The attachment 14035.diff patch appears to be correct, by the way.)

comment:6 tigertech4 years ago

  • Cc tigertech added

comment:7 scribu4 years ago

  • Component changed from General to Performance
  • Keywords commit added
  • Milestone changed from Awaiting Triage to 3.1

PS: The 'no_found_rows' => true bit can be ommited due to [15548].

scribu4 years ago

omit no_found_rows; correct spelling

comment:8 scribu4 years ago

Besides performance, this is actually a logical error too, given the function's description. See 14035.2.diff

comment:9 markjaquith4 years ago

  • Keywords commit removed

Was the removal of the category parameter accidental?

scribu4 years ago

bring back accidentally removed params

comment:10 scribu4 years ago

  • Keywords commit added

Yeah, the 'order' parameter was missing too. See 14035.3.diff

comment:11 nacin4 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [15660]) Have get_boundary_post sort by post_date, not ID. props scribu, jk0. fixes #14035.

Note: See TracTickets for help on using tickets.