Unexpected results when running WP_User_Query with role and meta_query

[layotte]

There appears to be a couple of bugs in WP_User_Query when doing a role query and adding additional meta_query options (especially when relation is set to "OR").

This code:

$args = array(
	'role'		=> 'Author',
	'number'	=> 100,
	'offset'	=> 0,
	'meta_query' => array(
		'relation' => 'OR',
		array(
			'key'		=> '_my_key',
			'compare'	=> 'NOT EXISTS',
		),
		array(
			'key'		=> '_my_key',
			'value'		=> 'off',
			'compare'	=> 'NOT LIKE',
		),
		),
);
$users = new WP_User_Query( $args );

Expected results:
Any author where _my_key either does not exist or if it does exists is set to "off"

Returned results:
All authors and/or all users with _my_key set.

The above generates this SQL:

SELECT DISTINCT SQL_CALC_FOUND_ROWS wp_users.* 
FROM wp_users 
INNER JOIN wp_usermeta ON wp_users.ID = wp_usermeta.user_id
INNER JOIN wp_usermeta AS mt1 ON (wp_users.ID = mt1.user_id)
INNER JOIN wp_usermeta AS mt2 ON (wp_users.ID = mt2.user_id) 
WHERE 1=1 
AND (wp_usermeta.meta_key = '_pigeonpack_subscription'
	OR  (mt1.meta_key = '_pigeonpack_subscription' AND CAST(mt1.meta_value AS CHAR) NOT LIKE '%off%')
	OR  (mt2.meta_key = 'wp_capabilities' AND CAST(mt2.meta_value AS CHAR) LIKE '%\"Author\"%') ) 
ORDER BY user_login ASC LIMIT 100

This seems like a bug to me, the "Author" meta should not be modified by the relation => 'OR' argument and thus should not be included in the other meta query statement. Further, the NOT EXISTS isn't a true NOT EXISTS statement. In fact, I'm not even sure why the NOT EXISTS statement looks the way it does. I haven't had too much time to look into this any more in depth.

I tested this in WP3.5 and Trunk

[layotte]

Oops, that SQL should be this:

SELECT DISTINCT SQL_CALC_FOUND_ROWS wp_users.* 
FROM wp_users 
INNER JOIN wp_usermeta ON wp_users.ID = wp_usermeta.user_id
INNER JOIN wp_usermeta AS mt1 ON (wp_users.ID = mt1.user_id)
INNER JOIN wp_usermeta AS mt2 ON (wp_users.ID = mt2.user_id) 
WHERE 1=1 
AND (wp_usermeta.meta_key = '_my_key'
	OR  (mt1.meta_key = '_my_key' AND CAST(mt1.meta_value AS CHAR) NOT LIKE '%off%')
	OR  (mt2.meta_key = 'wp_capabilities' AND CAST(mt2.meta_value AS CHAR) LIKE '%\"Author\"%') ) 
ORDER BY user_login ASC LIMIT 100

[wonderboymusic]

I just ran this query against my local database with 1.2 million users in it - I cancelled the query after 2 minutes of it with no end in sight.
We really need to not store role and caps in serialized arrays. We only get away with it because most people only have a few users. At scale it all falls apart.

SELECT DISTINCT SQL_CALC_FOUND_ROWS wp_users.* 
FROM wp_users 
INNER JOIN wp_usermeta ON wp_users.ID = wp_usermeta.user_id
INNER JOIN wp_usermeta AS mt1 ON (wp_users.ID = mt1.user_id)
INNER JOIN wp_usermeta AS mt2 ON (wp_users.ID = mt2.user_id) 
WHERE 1=1 
AND (wp_usermeta.meta_key = 'first_name'
	OR  (mt1.meta_key = 'first_name' AND CAST(mt1.meta_value AS CHAR) NOT LIKE '%off%')
	OR  (mt2.meta_key = 'wp_capabilities' AND CAST(mt2.meta_value AS CHAR) LIKE '%\"Author\"%') ) 
ORDER BY user_login ASC LIMIT 100

[nacin]

Replying to wonderboymusic:

We really need to not store role and caps in serialized arrays. We only get away with it because most people only have a few users. At scale it all falls apart.

#10201

[sillybean]

I'm seeing something similar in beta 1, I think. If I've interpreted the docs correctly, this ought to show me users who haven't entered either a first or last name (limited to 10 for sanity):

$users = new WP_User_Query( array(
		'number' => 10,
		'meta_query' => array(
			'relation' => 'OR',
			array(
				'key' => 'first_name',
				'compare' => 'NOT EXISTS',
			),
			array(
				'key' => 'last_name',
				'compare' => 'NOT EXISTS',
			),
		)
	 ) );

Instead I get all users who have either a first or last name set.

[sillybean]

My example is bad; first_name and last_name rows always exist, even if they're empty. However, the problem is the same with two custom meta fields (phone and twitter): all users are returned, even those who have filled out one or both fields.

[SergeyBiryukov]

#25421 was marked as a duplicate.

[takien]

Thanks SergeyBiryukov mentioned my ticket was duplicated to this.

Here is my quick and dirty fix, at lest until it patched.

add_action('pre_user_query','pre_user_query_fix');

function pre_user_query_fix_callback ( $matches ) {
	return ') AND '.preg_replace('/(OR  |AND  )/i','',rtrim($matches[0],')'));
}
function pre_user_query_fix( $vars ) {
	$vars->query_where = preg_replace_callback('/(.*?)  (.*?)(\'wp_capabilities\')(.*?)%\'\) \)/i','pre_user_query_fix_callback',$vars->query_where);
	return $vars;
}

[jdgrimes]

I just came upon this problem as well. The issue is always present on multisite even when no role query is explicitly set, because on multisite it adds "AND mt1.meta_key = 'wptests_capabilities'" to all of the queries (I assume so that only confirmed users are queried).

23849.diff​ fixes both issues, and includes unit tests demonstrating the problems. All the patch does is check if the meta query is using 'relation' => 'OR', and if so it uses a second WP_Meta_Query object to get the SQL for the caps query, instead of just appending the conditions to the main meta query. As a result the caps query is always an AND, instead of sometimes being unintentionally changed to an OR.

[kovshenin]

This looks like a fairly easy fix now that we have nested meta queries. Instead of squeezing the caps query inside an existing meta query, we can create a new caps meta query and convert the original query into a nested one. See 23849.2.diff​.

[kovshenin]

In 23849.3.diff​: When looking whether a cap query is already present in the meta query, look into the parsed meta query, not just the meta_query query var. I said "query" six times.

[boonebgorges]

I said "query" six times.

+6 on this patch.

[kovshenin]

In 23849.4.diff​: Change the order of the meta queries which should also fix #27026.

[kovshenin]

In 30094:

Use a nested meta query when querying by role in WP_User_Query.

If a user query includes a meta query together with a role argument,
nest the original meta query and append the role meta query with an
AND relationship.

fixes #23849, #27026.

[boonebgorges]

In 37359:

Tests: Correct 'meta_query' syntax in test related to WP_User_Query 'who' param.

The test, introduced in [32207], used the incorrect syntax for 'meta_query' -
one fewer level of array-nesting than what WP_Meta_Query requires. This
slip uncovered a bug introduced into WP_User_Query in [30094], whereby
an incorrectly formatted 'meta_query' parameter would be properly parsed by
WP_User_Query when passed alongside who=authors.

We need to fix the inconsistent syntax in order to resolve #36724.

See #36724, #32019, #23849, #27026.