Skip to:

Opened 11 months ago

Last modified 5 months ago

#7442 new enhancement

Use JOIN rather than Subquery on user search

Reported by: brandonliles Owned by:
Milestone: 3.0 Priority: normal
Severity: normal Version:
Component: Core Keywords: has-patch dev-feedback early


The user search relies on two subqueries. This is a known weakness of MySQL performance. Using joins will improve performance.

Attachments (3)

bp-search-use-joins.patch (2.2 KB) - added by brandonliles 11 months ago.
bp-search-use-joins2.patch (2.0 KB) - added by brandonliles 11 months ago.
bp-search-optimize.patch (2.3 KB) - added by brandonliles 9 months ago.
Subquery Materialization Approach

Download all attachments as: .zip

Change History (13)

#1 @r-a-y
11 months ago

Thanks for creating this ticket and for the patch, @brandonliles!

However, the patch is breaking multiple unit tests.

If you are able to, can you set up WordPress for PHP unit testing?

Once you have done so, navigate to the buddypress directory and run phpunit in a command prompt. Then, you will be able to see what testcases are breaking.

If you need further help in setting up PHPUnit, let us know.

#2 @brandonliles
11 months ago

Thanks for the feedback @r-a-y just uploaded a second version of the patch that doesn't break stuff :)

#3 @hnla
9 months ago

  • Keywords has-patch dev-feedback added
  • Milestone changed from Awaiting Review to 2.9

9 months ago

Subquery Materialization Approach

#4 @brandonliles
9 months ago

After seeing the query still perform poorly on a site I took another look, although joins can improve performance, MySQL still tends to look at too many rows in the bp_activity table. Determining the matching user_ids first and plugging those values into the query is a major performance improvement.

Version 0, edited 9 months ago by brandonliles (next)

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.

9 months ago

#6 @r-a-y
9 months ago

Thanks for the new patch, @brandonliles.

bp-search-optimize.patch passes existing unit tests. The rationale behind the "query for IDs first" approach makes sense to me.

Looks good, but needs sign off from a lead dev. We might need some new unit tests to cover this change though.

Last edited 9 months ago by r-a-y (previous) (diff)

This ticket was mentioned in Slack in #buddypress by hnla. View the logs.

8 months ago

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.

8 months ago

#9 @hnla
7 months ago

  • Keywords early added
  • Milestone changed from 2.9 to 3.0

#10 @boonebgorges
5 months ago

@brandonliles Thanks for the ticket and for your patches!

For some background on your original suggestion: We intentionally converted these (and other) user-related queries to subqueries a number of years ago. BuddyPress must remain compatible with configurations where the wp_users table does not live in the same database - or even the same server - as wp_bp_xprofile_data. See eg

Separating the subquery into a separate $wpdb->get_col() seems like an OK move to me. It'd be even better if we could use this opportunity to wrap a layer of caching around the search. Do you happen to have any benchmarks related to the performance improvements you've mentioned?

I think we have sufficient test coverage for this kind of change. I can't think of a situation where the suggested change would break something that wouldn't be detected by the tests we've already got.

Note: See TracTickets for help on using tickets.