Skip to:
Content

BuddyPress.org

Changeset 11071


Ignore:
Timestamp:
09/09/2016 03:00:13 PM (9 years ago)
Author:
boonebgorges
Message:

Groups: Overhaul BP_Groups_Group::get() SQL query.

The new query follows the WP and BP convention of "split" queries: one
for the IDs matching the query parameters, and a second one for the
objects corresponding to the matched IDs. This configuration allows for
improved caching and better performance, especially on installations
with large numbers of groups.

The rewrite serves as a convenient excuse to address a number of
longtime annoyances with the way group queries work:

  • Previously, comma syntax was used for table joins, rather than the JOIN keyword. This required some string manipulation when using external tools for generating SQL fragments, such as WP_Tax_Query and WP_Meta_Query. See #5099, #5874. We now use the more standard JOIN syntax.
  • The logic for assembling the "total" query is overwhelmingly simpler.
  • Previously, group queries required three joined tables: the groups table, one groupmeta table (for last_activity), and a second groupmeta table (for total_member_count). After this changeset, these tables will only be joined when needed for sorting (orderby or type). The last_activity and total_member_count properties, when needed for display in a group loop, are lazyloaded from groupmeta - where they're precached by default (see update_meta_cache) - rather than pulled as part of the primary query, and are made available via __get() for backward compatibility.

See #5451, #5874. Fixes #5099.

Location:
trunk
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/bp-groups/classes/class-bp-groups-group.php

    r11065 r11071  
    106106     * @var int
    107107     */
    108     public $total_member_count;
     108    protected $total_member_count;
    109109
    110110    /**
    … …  
    138138     * @var string
    139139     */
    140     public $last_activity;
     140    protected $last_activity;
    141141
    142142    /**
    … …  
    242242                }
    243243            }
    244 
    245             // Set up some specific group vars from meta. Excluded
    246             // from the bp_groups cache because it's cached independently.
    247             $this->last_activity      = groups_get_groupmeta( $this->id, 'last_activity' );
    248             $this->total_member_count = (int) groups_get_groupmeta( $this->id, 'total_member_count' );
    249244
    250245            // Set user-specific data.
    … …  
    430425    }
    431426
     427    /**
     428     * Magic getter.
     429     *
     430     * @since 2.7.0
     431     *
     432     * @param string $key Property name.
     433     * @return mixed
     434     */
     435    public function __get( $key ) {
     436        switch ( $key ) {
     437            case 'last_activity' :
     438                return groups_get_groupmeta( $this->id, 'last_activity' );
     439
     440            case 'total_member_count' :
     441                return (int) groups_get_groupmeta( $this->id, 'total_member_count' );
     442
     443            default :
     444            break;
     445        }
     446    }
     447
     448    /**
     449     * Magic issetter.
     450     *
     451     * Used to maintain backward compatibility for properties that are now
     452     * accessible only via magic method.
     453     *
     454     * @since 2.7.0
     455     *
     456     * @param string $key Property name.
     457     * @return bool
     458     */
     459    public function __isset( $key ) {
     460        switch ( $key ) {
     461            case 'last_activity' :
     462            case 'total_member_count' :
     463                return true;
     464
     465            default :
     466                return false;
     467        }
     468    }
     469
    432470    /** Static Methods ****************************************************/
    433471
    … …  
    778816        $bp = buddypress();
    779817
    780         $sql       = array();
    781         $total_sql = array();
    782 
    783         $sql['select'] = "SELECT DISTINCT g.id, g.*, gm1.meta_value AS total_member_count, gm2.meta_value AS last_activity";
    784         $sql['from']   = " FROM {$bp->groups->table_name_groupmeta} gm1, {$bp->groups->table_name_groupmeta} gm2,";
     818        $sql = array(
     819            'select'     => "SELECT DISTINCT g.id",
     820            'from'       => "{$bp->groups->table_name} g",
     821            'where'      => '',
     822            'orderby'    => '',
     823            'pagination' => '',
     824        );
     825
    785826
    786827        if ( ! empty( $r['user_id'] ) ) {
    787             $sql['members_from'] = " {$bp->groups->table_name_members} m,";
    788         }
    789 
    790         $sql['group_from'] = " {$bp->groups->table_name} g WHERE";
    791 
    792         if ( ! empty( $r['user_id'] ) ) {
    793             $sql['user_where'] = " g.id = m.group_id AND";
    794         }
    795 
    796         $sql['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'";
     828            $sql['from'] .= " JOIN {$bp->groups->table_name_members} m ON ( g.id = m.group_id )";
     829        }
     830
     831        $where_conditions = array();
    797832
    798833        if ( empty( $r['show_hidden'] ) ) {
    799             $sql['hidden'] = " AND g.status != 'hidden'";
     834            $where_conditions['hidden'] = "g.status != 'hidden'";
    800835        }
    801836
    802837        if ( ! empty( $r['search_terms'] ) ) {
    803838            $search_terms_like = '%' . bp_esc_like( $r['search_terms'] ) . '%';
    804             $sql['search'] = $wpdb->prepare( " AND ( g.name LIKE %s OR g.description LIKE %s )", $search_terms_like, $search_terms_like );
     839            $where_conditions['search'] = $wpdb->prepare( "( g.name LIKE %s OR g.description LIKE %s )", $search_terms_like, $search_terms_like );
    805840        }
    806841
    … …  
    812847
    813848        if ( ! empty( $meta_query_sql['where'] ) ) {
    814             $sql['meta'] = $meta_query_sql['where'];
     849            $where_conditions['meta'] = $meta_query_sql['where'];
    815850        }
    816851
    … …  
    830865
    831866        if ( ! empty( $group_type_clause ) ) {
    832             $sql['group_type'] = $group_type_clause;
     867            $where_conditions['group_type'] = $group_type_clause;
    833868        }
    834869
    835870        if ( ! empty( $r['user_id'] ) ) {
    836             $sql['user'] = $wpdb->prepare( " AND m.user_id = %d AND m.is_confirmed = 1 AND m.is_banned = 0", $r['user_id'] );
     871            $where_conditions['user'] = $wpdb->prepare( "m.user_id = %d AND m.is_confirmed = 1 AND m.is_banned = 0", $r['user_id'] );
    837872        }
    838873
    839874        if ( ! empty( $r['include'] ) ) {
    840875            $include        = implode( ',', wp_parse_id_list( $r['include'] ) );
    841             $sql['include'] = " AND g.id IN ({$include})";
     876            $where_conditions['include'] = "g.id IN ({$include})";
    842877        }
    843878
    844879        if ( ! empty( $r['exclude'] ) ) {
    845880            $exclude        = implode( ',', wp_parse_id_list( $r['exclude'] ) );
    846             $sql['exclude'] = " AND g.id NOT IN ({$exclude})";
     881            $where_conditions['exclude'] = "g.id NOT IN ({$exclude})";
    847882        }
    848883
    … …  
    876911                $orderby = $order_orderby['orderby'];
    877912            }
     913        }
     914
     915        // 'total_member_count' and 'last_activity' sorts require additional table joins.
     916        if ( 'total_member_count' === $orderby ) {
     917            $sql['from'] .= " JOIN {$bp->groups->table_name_groupmeta} gm_total_member_count ON ( g.id = gm_total_member_count.group_id )";
     918            $where_conditions['total_member_count'] = "gm_total_member_count.meta_key = 'total_member_count'";
     919        } elseif ( 'last_activity' === $orderby ) {
     920
     921            $sql['from'] .= " JOIN {$bp->groups->table_name_groupmeta} gm_last_activity on ( g.id = gm_last_activity.group_id )";
     922            $where_conditions['last_activity'] = "gm_last_activity.meta_key = 'last_activity'";
    878923        }
    879924
    … …  
    894939        // Random order is a special case.
    895940        if ( 'rand()' === $orderby ) {
    896             $sql[] = "ORDER BY rand()";
     941            $sql['orderby'] = "ORDER BY rand()";
    897942        } else {
    898             $sql[] = "ORDER BY {$orderby} {$order}";
     943            $sql['orderby'] = "ORDER BY {$orderby} {$order}";
    899944        }
    900945
    … …  
    902947            $sql['pagination'] = $wpdb->prepare( "LIMIT %d, %d", intval( ( $r['page'] - 1 ) * $r['per_page']), intval( $r['per_page'] ) );
    903948        }
     949
     950        $where = '';
     951        if ( ! empty( $where_conditions ) ) {
     952            $sql['where'] = implode( ' AND ', $where_conditions );
     953            $where = "WHERE {$sql['where']}";
     954        }
     955
     956        $paged_groups_sql = "{$sql['select']} FROM {$sql['from']} {$where} {$sql['orderby']} {$sql['pagination']}";
    904957
    905958        /**
    … …  
    912965         * @param array  $r     Array of parsed arguments for the get method.
    913966         */
    914         $paged_groups_sql = apply_filters( 'bp_groups_get_paged_groups_sql', join( ' ', (array) $sql ), $sql, $r );
    915         $paged_groups     = $wpdb->get_results( $paged_groups_sql );
    916 
    917         $total_sql['select'] = "SELECT COUNT(DISTINCT g.id) FROM {$bp->groups->table_name} g, {$bp->groups->table_name_groupmeta} gm";
    918 
    919         if ( ! empty( $r['user_id'] ) ) {
    920             $total_sql['select'] .= ", {$bp->groups->table_name_members} m";
    921         }
    922 
    923         if ( ! empty( $sql['hidden'] ) ) {
    924             $total_sql['where'][] = "g.status != 'hidden'";
    925         }
    926 
    927         if ( ! empty( $sql['search'] ) ) {
    928             $total_sql['where'][] = $wpdb->prepare( "( g.name LIKE %s OR g.description LIKE %s )", $search_terms_like, $search_terms_like );
    929         }
    930 
    931         if ( ! empty( $r['user_id'] ) ) {
    932             $total_sql['where'][] = $wpdb->prepare( "m.group_id = g.id AND m.user_id = %d AND m.is_confirmed = 1 AND m.is_banned = 0", $r['user_id'] );
    933         }
    934 
    935         // Temporary implementation of meta_query for total count
    936         // See #5099.
    937         if ( ! empty( $meta_query_sql['where'] ) ) {
    938             // Join the groupmeta table.
    939             $total_sql['select'] .= ", ". substr( $meta_query_sql['join'], 0, -2 );
    940 
    941             // Modify the meta_query clause from paged_sql for our syntax.
    942             $meta_query_clause = preg_replace( '/^\s*AND/', '', $meta_query_sql['where'] );
    943             $total_sql['where'][] = $meta_query_clause;
    944         }
    945 
    946         // Trim leading 'AND' to match `$total_sql` query style.
    947         if ( ! empty( $group_type_clause ) ) {
    948             $total_sql['where'][] = preg_replace( '/^\s*AND\s*/', '', $group_type_clause );
    949         }
    950 
    951         // Already escaped in the paginated results block.
    952         if ( ! empty( $include ) ) {
    953             $total_sql['where'][] = "g.id IN ({$include})";
    954         }
    955 
    956         // Already escaped in the paginated results block.
    957         if ( ! empty( $exclude ) ) {
    958             $total_sql['where'][] = "g.id NOT IN ({$exclude})";
    959         }
    960 
    961         $total_sql['where'][] = "g.id = gm.group_id";
    962         $total_sql['where'][] = "gm.meta_key = 'last_activity'";
    963 
    964         $t_sql = $total_sql['select'];
    965 
    966         if ( ! empty( $total_sql['where'] ) ) {
    967             $t_sql .= " WHERE " . join( ' AND ', (array) $total_sql['where'] );
    968         }
     967        $paged_groups_sql = apply_filters( 'bp_groups_get_paged_groups_sql', $paged_groups_sql, $sql, $r );
     968        $paged_group_ids  = $wpdb->get_col( $paged_groups_sql );
     969
     970        $uncached_group_ids = bp_get_non_cached_ids( $paged_group_ids, 'bp_groups' );
     971        if ( $uncached_group_ids ) {
     972            $group_ids_sql = implode( ',', array_map( 'intval', $uncached_group_ids ) );
     973            $group_data_objects = $wpdb->get_results( "SELECT g.* FROM {$bp->groups->table_name} g WHERE g.id IN ({$group_ids_sql})" );
     974            foreach ( $group_data_objects as $group_data_object ) {
     975                wp_cache_set( $group_data_object->id, $group_data_object, 'bp_groups' );
     976            }
     977        }
     978
     979        $paged_groups = array();
     980        foreach ( $paged_group_ids as $paged_group_id ) {
     981            $paged_groups[] = new BP_Groups_Group( $paged_group_id );
     982        }
     983
     984        $total_groups_sql = "SELECT COUNT(DISTINCT g.id) FROM {$sql['from']} $where";
    969985
    970986        /**
    … …  
    977993         * @param array  $r         Array of parsed arguments for the get method.
    978994         */
    979         $total_groups_sql = apply_filters( 'bp_groups_get_total_groups_sql', $t_sql, $total_sql, $r );
     995        $total_groups_sql = apply_filters( 'bp_groups_get_total_groups_sql', $total_groups_sql, $sql, $r );
    980996        $total_groups     = (int) $wpdb->get_var( $total_groups_sql );
    981997
    … …  
    9971013        // Set up integer properties needing casting.
    9981014        $int_props = array(
    999             'id', 'creator_id', 'enable_forum', 'total_member_count',
     1015            'id', 'creator_id', 'enable_forum'
    10001016        );
    10011017
    … …  
    10161032     *
    10171033     * We use WP_Meta_Query to do the heavy lifting of parsing the
    1018      * meta_query array and creating the necessary SQL clauses. However,
    1019      * since BP_Activity_Activity::get() builds its SQL differently than
    1020      * WP_Query, we have to alter the return value (stripping the leading
    1021      * AND keyword from the 'where' clause).
     1034     * meta_query array and creating the necessary SQL clauses.
    10221035     *
    10231036     * @since 1.8.0
    … …  
    10431056
    10441057            $meta_sql = $groups_meta_query->get_sql( 'group', 'g', 'id' );
    1045 
    1046             // BP_Groups_Group::get uses the comma syntax for table
    1047             // joins, which means that we have to do some regex to
    1048             // convert the INNER JOIN and move the ON clause to a
    1049             // WHERE condition
    1050             //
    1051             // @todo It may be better in the long run to refactor
    1052             // the more general query syntax to accord better with
    1053             // BP/WP convention.
    1054             preg_match_all( '/JOIN (.+?) ON/', $meta_sql['join'], $matches_a );
    1055             preg_match_all( '/ON \((.+?)\)/', $meta_sql['join'], $matches_b );
    1056 
    1057             if ( ! empty( $matches_a[1] ) && ! empty( $matches_b[1] ) ) {
    1058                 $sql_array['join']  = implode( ',', $matches_a[1] ) . ', ';
    1059                 $sql_array['where'] = $meta_sql['where'] . ' AND ' . implode( ' AND ', $matches_b[1] );
    1060             }
     1058            $sql_array['join']  = $meta_sql['join'];
     1059            $sql_array['where'] = self::strip_leading_and( $meta_sql['where'] );
    10611060        }
    10621061
    … …  
    11981197
    11991198            case 'last_activity' :
    1200                 $order_by_term = 'last_activity';
     1199                $order_by_term = 'gm_last_activity.meta_value';
    12011200                break;
    12021201
    12031202            case 'total_member_count' :
    1204                 $order_by_term = 'CONVERT(gm1.meta_value, SIGNED)';
     1203                $order_by_term = 'CONVERT(gm_total_member_count.meta_value, SIGNED)';
    12051204                break;
    12061205
    … …  
    16671666        // The no_results clauses are the same between IN and NOT IN.
    16681667        if ( false !== strpos( $sql_clauses['where'], '0 = 1' ) ) {
    1669             $clause = $sql_clauses['where'];
     1668            $clause = self::strip_leading_and( $sql_clauses['where'] );
    16701669
    16711670        // The tax_query clause generated for NOT IN can be used almost as-is.
    16721671        } elseif ( 'NOT IN' === $operator ) {
    1673             $clause = $sql_clauses['where'];
     1672            $clause = self::strip_leading_and( $sql_clauses['where'] );
    16741673
    16751674        // IN clauses must be converted to a subquery.
    16761675        } elseif ( preg_match( '/' . $wpdb->term_relationships . '\.term_taxonomy_id IN \([0-9, ]+\)/', $sql_clauses['where'], $matches ) ) {
    1677             $clause = " AND g.id IN ( SELECT object_id FROM $wpdb->term_relationships WHERE {$matches[0]} )";
     1676            $clause = " g.id IN ( SELECT object_id FROM $wpdb->term_relationships WHERE {$matches[0]} )";
    16781677        }
    16791678
    … …  
    16841683        return $clause;
    16851684    }
     1685
     1686    /**
     1687     * Strips the leading AND and any surrounding whitespace from a string.
     1688     *
     1689     * Used here to normalize SQL fragments generated by `WP_Meta_Query` and
     1690     * other utility classes.
     1691     *
     1692     * @since 2.7.0
     1693     *
     1694     * @param string $s String.
     1695     * @return string
     1696     */
     1697    protected static function strip_leading_and( $s ) {
     1698        return preg_replace( '/^\s*AND\s*/', '', $s );
     1699    }
    16861700}

  • trunk/tests/phpunit/testcases/groups/class-bp-groups-group.php

    r10937 r11071  
    397397    /**
    398398     * @group get
     399     */
     400    public function test_get_with_type_alphabetical() {
     401        $time = time();
     402        $g1 = $this->factory->group->create( array(
     403            'name' => 'A Group',
     404            'date_created' => bp_core_current_time(),
     405        ) );
     406        $g2 = $this->factory->group->create( array(
     407            'name' => 'D Group',
     408            'date_created' => gmdate( 'Y-m-d H:i:s', $time - 100 ),
     409        ) );
     410        $g3 = $this->factory->group->create( array(
     411            'name' => 'B Group',
     412            'date_created' => gmdate( 'Y-m-d H:i:s', $time - 100000 ),
     413        ) );
     414        $g4 = $this->factory->group->create( array(
     415            'name' => 'C Group',
     416            'date_created' => gmdate( 'Y-m-d H:i:s', $time - 1000 ),
     417        ) );
     418
     419        $groups = BP_Groups_Group::get( array( 'type' => 'alphabetical' ) );
     420        $found = wp_parse_id_list( wp_list_pluck( $groups['groups'], 'id' ) );
     421        $this->assertEquals( array( $g1, $g3, $g4, $g2 ), $found );
     422    }
     423
     424    /**
     425     * @group get
    399426     * @group group_meta_query
    400427     * @ticket BP5099
    … …  
    564591    public function test_convert_orderby_to_order_by_term_last_activity() {
    565592        $c = new _BP_Groups_Group();
    566         $this->assertEquals( 'last_activity', _BP_Groups_Group::_convert_orderby_to_order_by_term( 'last_activity' ) );
     593        $this->assertEquals( 'gm_last_activity.meta_value', _BP_Groups_Group::_convert_orderby_to_order_by_term( 'last_activity' ) );
    567594    }
    568595
    … …  
    572599    public function test_convert_orderby_to_order_by_term_total_member_count() {
    573600        $c = new _BP_Groups_Group();
    574         $this->assertEquals( 'CONVERT(gm1.meta_value, SIGNED)', _BP_Groups_Group::_convert_orderby_to_order_by_term( 'total_member_count' ) );
     601        $this->assertEquals( 'CONVERT(gm_total_member_count.meta_value, SIGNED)', _BP_Groups_Group::_convert_orderby_to_order_by_term( 'total_member_count' ) );
    575602    }
    576603
Note: See TracChangeset for help on using the changeset viewer.