WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#16152 closed defect (bug) (fixed)

category__in now shows posts from children categories too

Reported by: linuxologos Owned by:
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: Query Keywords: has-patch commit
Focuses: Cc:

Description

Until 3.0.4 category_in parameter of query_posts() does not show posts from any children of the categories given to it, as currently stated in the codex too. In 3.1-RC2 it does show posts from child-categories.

Let's say we have a category with some subcategories. Posts are assigned either to parent category or to children of the category (not both to parent and child). If we use

query_posts( array( 'category__in' => array($parent_category_id) ) )

in 3.0.4 only posts assigned to parent category are shown.
But in 3.1 posts from parent as well as subcategories are shown with the same code.

I'm not sure if it is a bug or just the way the parameter will work since 3.1, but people using this parameter that way will possibly see their sites working unexpectedly after upgrading to 3.1.

Attachments (4)

16152.patch (571 bytes) - added by SergeyBiryukov 3 years ago.
16152.2.patch (984 bytes) - added by SergeyBiryukov 3 years ago.
16152.3.patch (1.3 KB) - added by SergeyBiryukov 3 years ago.
16152.diff (2.2 KB) - added by ryan 3 years ago.
Parse cat once.

Download all attachments as: .zip

Change History (30)

comment:1 linuxologos3 years ago

There may be a connection with #16088.

comment:2 nacin3 years ago

  • Milestone changed from Awaiting Review to 3.1

comment:3 SergeyBiryukov3 years ago

parse_tax_query() calls WP_Tax_Query(), where include_children is true by default. Is it safe to use 'include_children' => false for category__in and category__not_in?

SergeyBiryukov3 years ago

comment:4 SergeyBiryukov3 years ago

  • Keywords has-patch added

comment:5 scribu3 years ago

  • Keywords commit added

Patch looks good.

comment:6 linuxologos3 years ago

Patch works.

comment:7 ryan3 years ago

Wont' this break 'cat' queries, which should include children? 'cat' gets mapped to 'category__in' which the patch changes to not include children.

Last edited 3 years ago by ryan (previous) (diff)

comment:8 follow-up: ryan3 years ago

Perhaps the 'cat' block could set a flag to force inclusion of children for 'category__in'. This would mean that passing both cat and category__in would result in child inclusion for all categories. A small back compat break. Alternatively, go back to calling get_term_chidlren() inside the 'cat' block.

Version 0, edited 3 years ago by ryan (next)

comment:9 ryan3 years ago

Do we need the same treatment for tag__in and tag__not_in?

comment:10 nacin3 years ago

They're not hierarchical, so children should never be searched.

comment:11 in reply to: ↑ 8 ; follow-up: SergeyBiryukov3 years ago

Replying to ryan:

Perhaps the 'cat' block could set a flag to force inclusion of children for 'category__in'. This would mean that passing both cat and category__in would result in child inclusion for all categories. A small back compat break. Alternatively, go back to calling get_term_children() inside the 'cat' block.

I'm inclined to the second option. Is there a reason why it was removed? Can't find the changeset.

SergeyBiryukov3 years ago

comment:12 in reply to: ↑ 11 SergeyBiryukov3 years ago

Replying to SergeyBiryukov:

Can't find the changeset.

Found: [15613] for #12891.

Last edited 3 years ago by SergeyBiryukov (previous) (diff)

SergeyBiryukov3 years ago

comment:13 SergeyBiryukov3 years ago

Also included array type cast as per #14892.

comment:14 ryan3 years ago

Note that parse_tax_query() is run twice. It'd be nice to avoid finding all children twice, especially for the second call which will already include the children.

comment:15 ryan3 years ago

Actually, since the children are not put back into the 'cat' arg we won't have to look up children for the children on the second pass. We're still unnecessarily finding the children for the terms in 'cat' again, however. parse_tax_query() may need an extra arg indicating that it is being called a second time. The second call to parse_tax_query(), right before get_sql(), could pass this flag and cause parse_tax_query() to skip the 'cat' block.

comment:16 SergeyBiryukov3 years ago

Can we use a static variable?

comment:17 scribu3 years ago

No, because that would only allow a single tax query (via WP_Query) per request.

comment:18 SergeyBiryukov3 years ago

Thanks! Working on the new patch.

ryan3 years ago

Parse cat once.

comment:19 ryan3 years ago

(In [17246]) Don't parse cat query var twice. Don't include children for category* queries. Props SergeyBiryukov. see #16152

comment:20 Denis-de-Bernardy3 years ago

Can't this last changeset break plugins that rely on wp_query to parse arbitrary queries?

comment:21 follow-up: ryan3 years ago

The change prevents only 'cat' from being processed again. I don't see a need to double process that. The flag is cleared for subsequent query() calls made using the same object.

Multiple tax queries are still possible. I tested with requests such as:

$query = new WP_Query( array('tag_slug__and' => array('foo', 'bar', 'test',), 'tag__in' => array( 3 ) ) );

That creates two entries in the tax_query list. One of which is operator AND and the other IN. These two have a 'relationship' of AND. The resulting query is like:

SELECT SQL_CALC_FOUND_ROWS wp_trunk_posts.* FROM wp_trunk_posts INNER JOIN wp_trunk_term_relationships ON (wp_trunk_posts.ID = wp_trunk_term_relationships.object_id) WHERE 1=1 AND ( wp_trunk_term_relationships.term_taxonomy_id IN (12) AND wp_trunk_posts.ID IN (
SELECT object_id
FROM wp_trunk_term_relationships
WHERE term_taxonomy_id IN (12,13,14)
GROUP BY object_id HAVING COUNT(object_id) = 3
) ) AND wp_trunk_posts.post_type = 'post' AND (wp_trunk_posts.post_status = 'publish' OR wp_trunk_posts.post_status = 'private') GROUP BY wp_trunk_posts.ID ORDER BY wp_trunk_posts.post_date DESC LIMIT 0, 5

comment:22 in reply to: ↑ 21 Denis-de-Bernardy3 years ago

Replying to ryan:

The change prevents only 'cat' from being processed again. I don't see a need to double process that. The flag is cleared for subsequent query() calls made using the same object.

I'll give it a quick whirl this week (and change my forked plugin if needed). But my point was, I was mainly worried of old time plugins (which are still in use) along the lines of Custom Query String, which worked fine to date in spite of being massively obsolete, and which calls $wp_query->parse_query() using arbitrary arguments to do its stuff.

comment:23 ryan3 years ago

I *think* that will be okay since the flag is reset inside parse_query(). parse_tax_query() should be safe to run multiple times per query. The flag just avoids duplicating work.

comment:24 ryan3 years ago

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

comment:25 markjaquith3 years ago

Caused a bug: #16622

comment:26 Anton Torvald2 years ago

Nothing.

Last edited 2 years ago by SergeyBiryukov (previous) (diff)
Note: See TracTickets for help on using tickets.