Changeset 32713

Timestamp:

06/09/2015 05:41:35 PM (10 years ago)

Author:

boonebgorges

Message:

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.

Location:

trunk

Files:

• src/wp-includes/meta.php (modified) (3 diffs)
• src/wp-includes/user.php (modified) (1 diff)
• tests/phpunit/tests/meta/query.php (modified) (1 diff)
• tests/phpunit/tests/user/query.php (modified) (1 diff)

trunk/src/wp-includes/meta.php

@@ -955 +955 @@
 
     /**
+     * Whether the query contains any OR relations.
+     *
+     * @since 4.3.0
+     * @access protected
+     * @var bool
+     */
+    protected $has_or_relation = false;
+
+    /**
      * Constructor.
      *
@@ -1047 +1056 @@
         if ( isset( $relation ) && 'OR' === strtoupper( $relation ) ) {
             $clean_queries['relation'] = 'OR';
+            $this->has_or_relation = true;
 
         /*
@@ -1579 +1589 @@
         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;
+    }
 }
 

trunk/src/wp-includes/user.php

@@ -710 +710 @@
             $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;
             }

trunk/tests/phpunit/tests/meta/query.php

@@ -807 +807 @@
         $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() );
+    }
 }

trunk/tests/phpunit/tests/user/query.php

@@ -790 +790 @@
         $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 );
+    }
 }