Make WordPress Core

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#27193 closed defect (bug) (invalid)

tax_query returns only partial results

Reported by: p_enrique's profile p_enrique Owned by:
Milestone: Priority: low
Severity: normal Version: 1.5
Component: Query Keywords: needs-patch needs-unit-tests needs-docs
Focuses: Cc:

Description

Querying by taxonomy using WP_Query returns unexpected results. It appears only the first criteria is matched.

Premises

Post 1 is in category 'category1'. $category1 is the term_id.
Post 2 is in category 'category2'. $category2 is the term_id.
Post 3 is in tagged with 'tag1'. $tag1 is the term_id.
Post 4 is in tagged with 'tag2'. $tag2 is the term_id.

$category1 = $this->factory->term->create( array( 'taxonomy' => 'category', 'name' => 'alpha' ) );
$category2 = $this->factory->term->create( array( 'taxonomy' => 'category', 'name' => 'beta' ) );
$tag1 = $this->factory->term->create( array( 'taxonomy' => 'post_tag', 'name' => 'gamma' ) );
$tag2 = $this->factory->term->create( array( 'taxonomy' => 'post_tag', 'name' => 'delta' ) );
$post_id1 = $this->factory->post->create( array( 'post_title' => 'alpha', 'post_category' => array( $category1 ) ) );
$post_id2 = $this->factory->post->create( array( 'post_title' => 'beta', 'post_category' => array( $category2 ) ) );
$post_id3 = $this->factory->post->create( array( 'post_title' => 'gamma', 'post_tag' => array( $tag1 ) ) );
$post_id4 = $this->factory->post->create( array( 'post_title' => 'delta', 'post_tag' => array( $tag2 ) ) );

Testing tax_query for categories 1 and 2

$args = array(
    'tax_query' => array(
        array(
            'taxonomy' => 'category',
            'field' => 'term_id',
            'terms' => array( $category1, $category2 )
        )
    )
);
$query = new WP_Query( $args );
$posts = $query->get_posts();

Expected result: $posts contains post 1 and 2.

Actual result: $posts contains post 1.

Workaround: Add 'relation' => 'OR' to the tax_query.

Testing two taxonomies at once

$query = new WP_Query( array( 
    'tax_query' => array(
        array(
            'taxonomy' => 'category',
            'field' => 'term_id',
            'terms' => array( $category1, $category2 )
	),
        array(
            'taxonomy' => 'post_tag',
            'field' => 'term_id',
            'terms' => array( $tag1, $tag2 )
	),
        'relation' => 'OR'
    )
) );

Expected result: $posts contains posts 1, 2, 3 and 4.

Actual result: $posts contains posts 1 and 2.

Change History (11)

#1 @bcworkz
11 years ago

  • Keywords 2nd-opinion added

I'm unable to replicate these errors. I did use existing data instead of creating with phpunit, but the specifics are identical: posts with only 1 term and terms with only 1 post. Both on 3.8.1 and Rev27302. Query code placed on a template page.

#2 follow-up: @johnbillion
11 years ago

  • Keywords 2nd-opinion removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed

The problem here is being caused by your call to $query->get_posts(). This method is called internally when the WP_Query class is instantiated and should not be called manually. I've tested your code and cannot reproduce the problem once that line is removed.

See the Usage section of WP_Query for the correct usage. Note also that you can use the $query->posts property if you want to see the list of posts that are present in the query.

#3 in reply to: ↑ 2 @p_enrique
11 years ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to johnbillion:

The problem here is being caused by your call to $query->get_posts().
See the Usage section of WP_Query for the correct usage.

Thanks for the reply. I'd like to point out that

  1. the referred Usage section only describes using the WP_Query inside "the Loop",
  2. just below on the same page the get_posts() method is said to "[f]etch and return the requested posts from the database", and
  3. the docblock in /wp-includes/query.php describes the functionality of the get_posts() method as "Retrieve the posts based on query variables" and defines "@return array List of posts" and "@access public".

Based on the above, I'd say that in this case the get_posts() method isn't doing what it's supposed to do according to the available docs. I'm going to dare to reopen this ticket and hope that some diagnoses whether this is actually a documentation problem or a code problem and attaches the appropriate keywords to this ticket.

#4 @johnbillion
11 years ago

  • Focuses docs added
  • Keywords 2nd-opinion added
  • Milestone set to Awaiting Review
  • Priority changed from normal to low
  • Version changed from trunk to 1.5

#5 @wonderboymusic
11 years ago

  • Keywords needs-patch needs-unit-tests added; 2nd-opinion removed
  • Milestone changed from Awaiting Review to 4.0

I will look at this - I started writing some tests, some weird things are happening

#6 @DrewAPicture
11 years ago

  • Focuses docs removed
  • Keywords needs-docs added

#7 @DrewAPicture
10 years ago

Any update here @wonderboymusic?

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.


10 years ago

#9 @DrewAPicture
10 years ago

  • Milestone changed from 4.0 to Future Release

No consensus on whether it's bad documentation or faulty code. Or both. Punting.

Last edited 10 years ago by DrewAPicture (previous) (diff)

#10 @boonebgorges
10 years ago

  • Milestone Future Release deleted
  • Resolution set to invalid
  • Status changed from reopened to closed

[28188] introduced a (failing) test for this issue.

The root cause of this problem is, as suggested by johnbillion https://core.trac.wordpress.org/ticket/27193#comment:2, the incorrect use of WP_Query::get_posts(). The issue is extremely unpleasant to wrap your mind around, so if you are lily-livered I recommend you skip the next two paragraphs.

<awful>
When you create a WP_Query object using a 'tax_query', a couple of query vars are filled in on the WP_Query object for backward compatibility. You can read some of the back story starting here https://core.trac.wordpress.org/ticket/12659#comment:25 and see the original changeset here [16381]. (The current code looks like this https://core.trac.wordpress.org/browser/trunk/src/wp-includes/query.php#L2712 - it's been refactored but it basically does the same thing.) What's happening here is this: back in Ye Olden Days of WordPress, themers who wanted to build titles for their taxonomy archives would do things like get_query_var( 'cat' ) to figure out what the current taxonomy was. Various changes related to the introduction of 'tax_query' made it the case that the 'cat' query var was not really used in most cases anymore, yet when other changes were made that caused 'cat' not to be set in some cases, the result was that these kinds of themes would have some broken page titles. The fix for backward compatibility was to look at the terms that'd been queried by the 'tax_query' argument, *grab the first one*, and set it as 'cat' - that way, at least the value wouldn't be empty.

So here's why this is causing the current problem. When you initially create the WP_Query object, you'll see that the items fetched in the posts property are correct - the items from cat 1 and cat 2. The backward-compatibility code described above sets the cat property to '1' - recall that this is not used to query anything. Then, when you run get_posts(), parse_query() is run; this method sees cat=1, and thinks that (for backward compatibility) it should take precedence over tax_query. So *another* posts query is run, but this one only against category 1. Thus the erroneous results you see above.
</awful>

All the things that p_enrique says here are true https://core.trac.wordpress.org/ticket/27193#comment:3. But the fact remains that WP_Query::get_posts() should only be called internally, and really only when setting up the main loop. The documentation should probably be updated with a note to this effect.

Thanks for bearing with us on this one.

#11 @nacin
10 years ago

The failed unit test was removed in [29979].

Note: See TracTickets for help on using tickets.