[buddypress-trac] [BuddyPress Trac] #5451: Groups query overhaul

[buddypress-trac]

#5451: Groups query overhaul
-----------------------------------+------------------
 Reporter:  boonebgorges           |       Owner:
     Type:  enhancement            |      Status:  new
 Priority:  high                   |   Milestone:  2.7
Component:  Groups                 |     Version:
 Severity:  normal                 |  Resolution:
 Keywords:  has-patch 2nd-opinion  |
-----------------------------------+------------------
Changes (by boonebgorges):

 * keywords:   => has-patch 2nd-opinion


Comment:

 [attachment:5451.diff] contains the first steps toward overhauling the
 query. It's the "first steps" because so far I've only refactored what
 we've currently got. No additional caching or explicit performance
 improvements are included here (with one exception - see below). I figured
 it'd be easier to get a review and/or a second opinion on the approach if
 I broke things up. Here's what the patch does:

 - Instead of doing a single query to fetch all groups matching the `get()`
 params, there are now two queries: an ID query and a cache-priming query.
 The old query looked like this:

 {{{
 SELECT DISTINCT g.id, g.*, gm1.meta_value AS total_member_count,
 gm2.meta_value AS last_activity
 FROM wp_bp_groups_groupmeta gm1, wp_bp_groups_groupmeta gm2, wp_bp_groups
 g
 WHERE g.id = gm1.group_id
 AND g.id = gm2.group_id
 AND gm2.meta_key = 'last_activity'
 AND gm1.meta_key = 'total_member_count'
 ORDER BY last_activity DESC
 LIMIT 0, 20
 }}}

 while the new ones look like this:

 {{{
 SELECT DISTINCT g.id
 FROM wp_bp_groups g JOIN wp_bp_groups_groupmeta gm_last_activity on ( g.id
 = gm_last_activity.group_id )
 WHERE gm_last_activity.meta_key = 'last_activity'
 ORDER BY gm_last_activity.meta_value DESC
 LIMIT 0, 20

 SELECT g.*
 FROM wp_bp_groups g
 WHERE g.id IN (63,68,71,70,69,18,66,46,20,61,44,32,45,62,67,24,58,65,27,2)
 }}}

 - As you can see, a costly table join has been removed. This table join
 was previously used to get the `total_member_count`. We only need this
 join when `type=popular` - ie, when we're sorting by `total_member_count`.
 See the section with the comment `// Ordering by 'total_member_count'
 requires another table join.`

 - `BP_Groups_Group::get()` now returns an array of `BP_Groups_Group`
 objects. See #6358. This has lots of benefits. The important one for the
 purposes of this ticket is that it centralizes cache management.

 - `last_activity` and `total_member_count` are no longer loaded into the
 group object by default. Instead, they've been made `protected` and are
 accessible by magic method. This reduces overhead when the values are not
 needed, and points the way to a future where we can do away with
 `populate_extras` altogether.

 - The rebuilding of the query syntax means that we can do away with all
 the funky string manipulation we were doing with `tax_query` and
 `meta_query`. So [attachment:5451.diff] fixes #5099 as well.

 Next steps, once this patch is in:

 - Incrementor-based caching for the ID query. See #6643.
 - Better pre-loop caching for group admins. With object caching disabled,
 `bp_get_group_join_button()` is causing a query for every group in the
 loop. #5711 added cache support for `get_group_administrator_ids()`, so
 all we'd need is single query at the beginning of a group loop that pre-
 fetches these admin lists and puts them into the cache.
 - Move some/all of the `populate_extras` stuff into magic methods, where
 they can be lazy-loaded. This will make our cache treatment more
 standardized, since we won't be maintaining separate versions of the
 object.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5451#comment:11>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac