Make WordPress Core

Opened 8 years ago

Closed 8 years ago

#35137 closed defect (bug) (fixed)

get_terms() with a meta_query filter returns duplicated terms

Reported by: jadpm's profile jadpm Owned by: boonebgorges's profile boonebgorges
Milestone: 4.4.1 Priority: normal
Severity: normal Version: 4.4
Component: Taxonomy Keywords: fixed-major
Focuses: Cc:

Description

get_terms() got support for meta_query in 4.4. However, it seems that under some meta_query scenarios it returns duplicated results - for example, when the same term has more than one value for a termmeta key.

Consider the following termmeta table, asuming the item with term_id = 1 is the default category:

meta_id term_id meta_key meta_value
1 1 foo bar
2 1 foo bor

Now, consider this arguments:

Array
(
    [hide_empty] => 1
    [hierarchical] => 1
    [pad_counts] => 1
    [orderby] => name
    [order] => DESC
    [meta_query] => Array
        (
            [0] => Array
                (
                    [key] => foo
                    [value] => bur
                    [type] => CHAR
                    [compare] => !=
                )

            [relation] => AND
        )

)

The performed query in get_terms() is as follows:

SELECT t.*, tt.* 
FROM wp_dev_terms AS t  
INNER JOIN wp_dev_termmeta ON ( t.term_id = wp_dev_termmeta.term_id ) 
INNER JOIN wp_dev_term_taxonomy AS tt ON t.term_id = tt.term_id 
WHERE tt.taxonomy IN ('category') 
AND ( 
  ( wp_dev_termmeta.meta_key = 'foo' AND CAST(wp_dev_termmeta.meta_value AS CHAR) != 'bur' )
) 
ORDER BY t.name DESC 

Note that as there is no DISTINCT or GROUP BY t.term_id statement, we get the same row twice, as both rows meet the query requirements.

When querying posts, as far as I've seen, we do group by post ID. We might want to do the same here.

Attachments (2)

35137.patch (1.8 KB) - added by jadpm 8 years ago.
Patch for the bug
35137.unittest.patch (968 bytes) - added by jadpm 8 years ago.
Unit test for the fix

Download all attachments as: .zip

Change History (10)

#1 @swissspidy
8 years ago

  • Component changed from General to Taxonomy
  • Version changed from trunk to 4.4

#2 @boonebgorges
8 years ago

  • Keywords needs-patch needs-unit-tests added
  • Milestone changed from Awaiting Review to 4.5

@jadpm Thanks for the ticket, and good catch.

@pento or anyone else - are there any general performance issues to keep in mind regarding GROUP BY vs DISTINCT? My research is turning up inconclusive.

#3 @pento
8 years ago

I suspect there'll be no practical difference.

For simple and un-indexed queries, DISTINCT is usually faster. It doesn't have an implicit sort, so it can return as soon as its read enough rows.

As this query is already sorting, and it will be using the primary key on wp_terms, the GROUP BY will run at the same speed.

Also, given that this functionality was introduced in 4.4, I'm inclined to fix this in 4.4.1.

#4 @jadpm
8 years ago

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

I'm attaching a patch that adds a DISTINCT clause when the term query contains a meta_query (as this is the scenario where the bug was reproduced) and a unit test for it.

@jadpm
8 years ago

Patch for the bug

@jadpm
8 years ago

Unit test for the fix

#5 @boonebgorges
8 years ago

  • Milestone changed from 4.5 to 4.4.1
  • Owner set to boonebgorges
  • Status changed from new to reviewing

Thanks, @jadpm !

#6 @boonebgorges
8 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 36003:

Ensure get_terms() results are unique when using 'meta_query'.

The introduction of 'meta_query' to get_terms() in 4.4 made it possible for
get_terms() to erroneously return duplicate results. To address the issue,
we add the DISTINCT keyword to the SQL query when a 'meta_query' parameter
has been provided.

Props @jadpm.
Fixes #35137.

#7 @boonebgorges
8 years ago

  • Keywords fixed-major added; has-patch has-unit-tests removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 4.4.1.

#8 @boonebgorges
8 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 36004:

Ensure get_terms() results are unique when using 'meta_query'.

The introduction of 'meta_query' to get_terms() in 4.4 made it possible for
get_terms() to erroneously return duplicate results. To address the issue,
we add the DISTINCT keyword to the SQL query when a 'meta_query' parameter
has been provided.

Merges [36003] to the 4.4 branch.

Props @jadpm.
Fixes #35137.

Note: See TracTickets for help on using tickets.