From c04185a1f2d81ef5d70b6210ce7f07d950506e34 Mon Sep 17 00:00:00 2001 From: Boone Gorges Date: Tue, 9 Jun 2015 17:41:35 +0000 Subject: [PATCH] Avoid returning duplicate matches when using a meta query in `WP_User_Query`. A meta_query containing an `OR` relation can result in the same record matching multiple clauses, leading to duplicate results. The previous prevention against duplicates [18178] #17582 became unreliable in 4.1 when `WP_Meta_Query` introduced support for nested clauses. The current changeset adds a new method `WP_Meta_Query::has_or_relation()` for checking whether an `OR` relation appears anywhere in the query, and uses the new method in `WP_User_Query` to enforce distinct results as necessary. Props maxxsnake. Fixes #32592. git-svn-id: https://develop.svn.wordpress.org/trunk@32713 602fd350-edb4-49c9-b593-d223f7449a82 --- src/wp-includes/meta.php | 25 ++++++++++ src/wp-includes/user.php | 2 +- tests/phpunit/tests/meta/query.php | 78 ++++++++++++++++++++++++++++++ tests/phpunit/tests/user/query.php | 68 ++++++++++++++++++++++++++ 4 files changed, 172 insertions(+), 1 deletion(-) diff --git a/src/wp-includes/meta.php b/src/wp-includes/meta.php index ead200455f..d1b74d904a 100644 --- a/src/wp-includes/meta.php +++ b/src/wp-includes/meta.php @@ -953,6 +953,15 @@ class WP_Meta_Query { */ protected $clauses = array(); + /** + * Whether the query contains any OR relations. + * + * @since 4.3.0 + * @access protected + * @var bool + */ + protected $has_or_relation = false; + /** * Constructor. * @@ -1046,6 +1055,7 @@ class WP_Meta_Query { // Sanitize the 'relation' key provided in the query. if ( isset( $relation ) && 'OR' === strtoupper( $relation ) ) { $clean_queries['relation'] = 'OR'; + $this->has_or_relation = true; /* * If there is only a single clause, call the relation 'OR'. @@ -1578,6 +1588,21 @@ class WP_Meta_Query { */ return apply_filters( 'meta_query_find_compatible_table_alias', $alias, $clause, $parent_query, $this ) ; } + + /** + * Check whether the current query has any OR relations. + * + * In some cases, the presence of an OR relation somewhere in the query will require the use of a DISTINCT or + * GROUP BY keyword in the SELECT clause. The current method can be used in these cases to determine whether + * such a clause is necessary. + * + * @since 4.3.0 + * + * @return bool True if the query contains any OR relations, otherwise false. + */ + public function has_or_relation() { + return $this->has_or_relation; + } } /** diff --git a/src/wp-includes/user.php b/src/wp-includes/user.php index 2599ce01a9..cb9695cf7b 100644 --- a/src/wp-includes/user.php +++ b/src/wp-includes/user.php @@ -709,7 +709,7 @@ class WP_User_Query { $this->query_from .= $clauses['join']; $this->query_where .= $clauses['where']; - if ( 'OR' == $this->meta_query->relation ) { + if ( $this->meta_query->has_or_relation() ) { $this->query_fields = 'DISTINCT ' . $this->query_fields; } } diff --git a/tests/phpunit/tests/meta/query.php b/tests/phpunit/tests/meta/query.php index 47fe7a6d28..d3b315ac13 100644 --- a/tests/phpunit/tests/meta/query.php +++ b/tests/phpunit/tests/meta/query.php @@ -806,4 +806,82 @@ class Tests_Meta_Query extends WP_UnitTestCase { $this->assertRegExp( "/{$wpdb->postmeta}\.meta_key = \'exclude\'\s+OR/", $sql['where'] ); $this->assertNotContains( "{$wpdb->postmeta}.post_id IS NULL", $sql['where'] ); } + + /** + * @group 32592 + */ + public function test_has_or_relation_should_return_false() { + $q = new WP_Meta_Query( array( + 'relation' => 'AND', + array( + 'key' => 'foo', + 'value' => 'bar', + ), + array( + 'relation' => 'AND', + array( + 'key' => 'foo1', + 'value' => 'bar', + ), + array( + 'key' => 'foo2', + 'value' => 'bar', + ), + ), + ) ); + + $this->assertFalse( $q->has_or_relation() ); + } + + /** + * @group 32592 + */ + public function test_has_or_relation_should_return_true_for_top_level_or() { + $q = new WP_Meta_Query( array( + 'relation' => 'OR', + array( + 'key' => 'foo', + 'value' => 'bar', + ), + array( + 'relation' => 'AND', + array( + 'key' => 'foo1', + 'value' => 'bar', + ), + array( + 'key' => 'foo2', + 'value' => 'bar', + ), + ), + ) ); + + $this->assertTrue( $q->has_or_relation() ); + } + + /** + * @group 32592 + */ + public function test_has_or_relation_should_return_true_for_nested_or() { + $q = new WP_Meta_Query( array( + 'relation' => 'AND', + array( + 'key' => 'foo', + 'value' => 'bar', + ), + array( + 'relation' => 'OR', + array( + 'key' => 'foo1', + 'value' => 'bar', + ), + array( + 'key' => 'foo2', + 'value' => 'bar', + ), + ), + ) ); + + $this->assertTrue( $q->has_or_relation() ); + } } diff --git a/tests/phpunit/tests/user/query.php b/tests/phpunit/tests/user/query.php index 2b22f1ec06..79a6fb1a47 100644 --- a/tests/phpunit/tests/user/query.php +++ b/tests/phpunit/tests/user/query.php @@ -789,4 +789,72 @@ class Tests_User_Query extends WP_UnitTestCase { $this->assertEqualSets( $expected, $found ); } + + /** + * @ticket 32592 + */ + public function test_top_level_or_meta_query_should_eliminate_duplicate_matches() { + $users = $this->factory->user->create_many( 3 ); + + add_user_meta( $users[0], 'foo', 'bar' ); + add_user_meta( $users[1], 'foo', 'bar' ); + add_user_meta( $users[0], 'foo2', 'bar2' ); + + $q = new WP_User_Query( array( + 'meta_query' => array( + 'relation' => 'OR', + array( + 'key' => 'foo', + 'value' => 'bar', + ), + array( + 'key' => 'foo2', + 'value' => 'bar2', + ), + ), + ) ); + + $found = wp_list_pluck( $q->get_results(), 'ID' ); + $expected = array( $users[0], $users[1] ); + + $this->assertEqualSets( $expected, $found ); + } + + /** + * @ticket 32592 + */ + public function test_nested_or_meta_query_should_eliminate_duplicate_matches() { + $users = $this->factory->user->create_many( 3 ); + + add_user_meta( $users[0], 'foo', 'bar' ); + add_user_meta( $users[1], 'foo', 'bar' ); + add_user_meta( $users[0], 'foo2', 'bar2' ); + add_user_meta( $users[1], 'foo3', 'bar3' ); + + $q = new WP_User_Query( array( + 'meta_query' => array( + 'relation' => 'AND', + array( + 'key' => 'foo', + 'value' => 'bar', + ), + array( + 'relation' => 'OR', + array( + 'key' => 'foo', + 'value' => 'bar', + ), + array( + 'key' => 'foo2', + 'value' => 'bar2', + ), + ), + ), + ) ); + + $found = wp_list_pluck( $q->get_results(), 'ID' ); + $expected = array( $users[0], $users[1] ); + + $this->assertEqualSets( $expected, $found ); + } }