DISTINCT causing poor performance on user queries

[johnjamesjacoby]

On large installations with thousands/millions of users, a few of our user queries are looking for DISTINCT results on columns that already contain unique data; namely, the id column.

See: BP_User_Query::prepare_user_ids_query()

Related, Andy had noticed this years ago in r3023 and removed one, but it eventually crept back in, either in a later merge, or copy/pasted from a different query.

[johnjamesjacoby]

I propose we remove DISTINCT's where they are not necessary. Patch incoming.

[johnjamesjacoby]

In 7698:

Remove ignored DISTINCT from BP_XProfile_Group::get() queries. See #5303.

[johnjamesjacoby]

In 7699:

Remove unused DISTINCT from BP_Blogs_Blog::is_hidden() query. See #5303.

[johnjamesjacoby]

In 7700:

Remove unnecessary DISTINCT usages from bp-core-classes.php. See #5303.

[johnjamesjacoby]

Related: #5118

[johnjamesjacoby]

In 7701:

Use correct ID column name rather than id. Reverts testing code accidentally committed in r7700. See #5303.

[cmmarslender]

Replying to johnjamesjacoby:

In 7698:

Remove ignored DISTINCT from BP_XProfile_Group::get() queries. See #5303.

This change caused profile fields to repeat when viewing a member profile

[johnjamesjacoby]

In 7708:

Revert r7698. Causing profile fields to repeat themselves in the loop. Needs unit tests before revisiting. See #5303.

[johnjamesjacoby]

Wrong ticket. Sorry y'all.

[lenasterg]

Hi.
I think also in /bp-activity/bp-activity-classes.php at line 295
the

// Select conditions
        $select_sql = "SELECT DISTINCT a.id";

should change to

// Select conditions
        $select_sql = "SELECT a.id";

[boonebgorges]

lenasterg - That DISTINCT is necessary. See #5210. I'm hopeful that the changes in r7765 (#5349) will have introduced enough performance improvements to the activity query that that particular DISTINCT won't matter.

[lenasterg]

Sorry, about the false-positive.

Is it the same case at /bp-groups/bp-groups-classes.php on line 2033 ?

$group_sql = $wpdb->prepare( "SELECT DISTINCT group_id FROM {$bp->groups->table_name_members} WHERE user_id = %d AND is_confirmed = 1 AND is_banned = 0{$pag_sql}", $user_id );

and line 2034

$total_groups = $wpdb->get_var( $wpdb->prepare( "SELECT COUNT(DISTINCT group_id) FROM {$bp->groups->table_name_members} WHERE user_id = %d AND is_confirmed = 1 AND is_banned = 0", $user_id ) );

[boonebgorges]

Yes, we can probably drop DISTINCT in these cases. There should be blocks elsewhere in BP to ensure that each group-user pair can only receive one membership. Though, fwiw, the impact of this particular DISTINCT is probably very, very small, because we're only selecting the heavily indexed group_id column.

[boonebgorges]

In 8289:

Reintroduce DISTINCT keyword to BP_Core_User::get_users()

The lack of DISTINCT was causing duplicate entries when joining against the
xprofile fields for the purposes of alphabetical sorts.

Reverts part of r7700. See #5303

Fixes #5550

[boonebgorges]

In 8290:

Reintroduce DISTINCT keyword to BP_Core_User::get_users()

The lack of DISTINCT was causing duplicate entries when joining against the
xprofile fields for the purposes of alphabetical sorts.

Reverts part of r7700. See #5303

Fixes #5550

[DJPaul]

Is this done now? Are the remaining DISTINCTs required? The issue behind the commit that Boone mentions above indicates we should fix up that SQL query, and then remove the DISTINCT, but I'd prefer that on a separate ticket as this ticket is fairly general.

[boonebgorges]

Let's close this ticket, as (a) I think we've gotten the worse offenders, and (b) it's unlikely that anyone is going to do the kind of systematic review of all instances of DISTINCT that would be required to do more work toward the ticket as described.

Please open separate tickets for individual instances of DISTINCT that are causing problems.