WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 12 months ago

#15861 new defect (bug)

Sorting users by post count

Reported by: scribu Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Users Keywords: has-patch dev-feedback
Focuses: administration Cc:

Description

Currently, to enable sorting by post count, there's a JOIN made between the users table and the posts table.

This is bad, because users is a global table, which might be stored in a separate database.

Short-term solution for 3.1 is to disable sorting.

Long-term solution is to avoid the JOIN somehow.

Attachments (1)

15861.patch (4.2 KB) - added by SergeyBiryukov 4 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 @scribu4 years ago

(In [17024]) Disable sorting by post count for now. See #15861

comment:2 @scribu4 years ago

  • Keywords 3.2-early added
  • Milestone changed from 3.1 to Future Release

comment:3 @scribu4 years ago

  • Keywords needs-patch added; 3.2-early removed

comment:4 @sirzooro4 years ago

  • Cc sirzooro added

comment:5 @sirzooro4 years ago

  • Keywords dev-feedback added

Looks that this ticket may stay here for a longer time. What about enabling sorting for single-db config now, and providing fix for single-db and multi-db later?

comment:6 @audiobahn4 years ago

like sirzooro said, we could do something now and fix it later. Change sorting to a post-query operation and sort manually. Later on, a patch could be written to replace the post-query operation with something in the query. Then it would always work multi-db.

comment:7 @nacin4 years ago

We could potentially do select post_author from $wpdb->posts where post_type = 'post' and post_status = 'publish' group by post_author order by count(ID) desc, then use that data to sort in PHP.

@SergeyBiryukov4 years ago

comment:8 in reply to: ↑ description ; follow-up: @SergeyBiryukov4 years ago

Replying to nacin:

We could potentially do select post_author from $wpdb->posts where post_type = 'post' and post_status = 'publish' group by post_author order by count(ID) desc, then use that data to sort in PHP.

15861.patch is an attempt to implement this.

Turned out tricky to use proper LIMITs for paging, so I guess this patch is not scalable. However it should work when querying not only user IDs, but other fields too (in which case $this->results is an array of objects rather than numbers).

Replying to scribu:

users is a global table, which might be stored in a separate database.

How exactly can we move users table to a separate database? Perhaps we should check if it's in the same database and fall back to JOIN, since it's more simple.

comment:9 @edwardw3 years ago

  • Keywords has-patch added; needs-patch removed

comment:10 in reply to: ↑ 8 @scribu3 years ago

Replying to SergeyBiryukov:

How exactly can we move users table to a separate database? Perhaps we should check if it's in the same database and fall back to JOIN, since it's more simple.

But there are several ways you can make queries to a separate DB: create a new WP_DB instance or use SELECT * other_database.wp_users.* etc.

Also, the HyperDB plugin enables this.

Version 0, edited 3 years ago by scribu (next)

comment:11 follow-up: @scribu3 years ago

So, it seems the "don't JOIN global tables with non-global tables" rule can be broken in the following case:

if ( !wp_is_large_network( 'users' ) && !defined( 'CUSTOM_USER_TABLE' ) && !file_exists( WP_CONTENT_DIR . '/db.php' ) )

comment:12 in reply to: ↑ 11 @nacin13 months ago

  • Component changed from Administration to Users
  • Focuses administration added

Replying to scribu:

So, it seems the "don't JOIN global tables with non-global tables" rule can be broken in the following case:

if ( !wp_is_large_network( 'users' ) && !defined( 'CUSTOM_USER_TABLE' ) && !file_exists( WP_CONTENT_DIR . '/db.php' ) )

I'm OK with that.

comment:13 @jdgrimes12 months ago

There were two lines of useless code left in WP_Users_List_Table::get_sortable_columns(). Probably should have been removed in [17024]. It isn't hurting anything, just confused me for a minute.

Note: See TracTickets for help on using tickets.