WordPress.org

Make WordPress Core

Opened 13 months ago

Last modified 2 days ago

#23849 new defect (bug)

Unexpected results when running WP_User_Query with role and meta_query

Reported by: layotte Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Users Keywords: dev-feedback has-patch
Focuses: Cc:

Description

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

Attachments (1)

23849.diff (3.8 KB) - added by jdgrimes 2 days ago.

Download all attachments as: .zip

Change History (11)

comment:1 layotte13 months ago

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

comment:2 follow-up: wonderboymusic13 months ago

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

comment:3 in reply to: ↑ 2 nacin13 months ago

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

comment:4 sillybean12 months ago

  • Cc steph@… added

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.

comment:5 SergeyBiryukov12 months ago

  • Summary changed from Unexepected results when running WP_User_Query with role and meta_query to Unexpected results when running WP_User_Query with role and meta_query

comment:6 sillybean12 months ago

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.

comment:7 SergeyBiryukov7 months ago

#25421 was marked as a duplicate.

comment:8 SergeyBiryukov7 months ago

  • Component changed from Query to Users

comment:9 takien7 months ago

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;
}
Last edited 7 months ago by SergeyBiryukov (previous) (diff)

jdgrimes2 days ago

comment:10 jdgrimes2 days ago

  • Keywords has-patch added

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.

Note: See TracTickets for help on using tickets.