WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#12704 closed defect (bug) (fixed)

Multiple post_types can no longer be specified in query

Reported by: mitchoyoshitaka Owned by: dd32
Milestone: 3.0 Priority: high
Severity: normal Version: 3.0
Component: Query Keywords:
Focuses: Cc:

Description

With query rev 13774, multiple post types specified in the query (#10791) don't work. If a simplex array is given, it will work but produce a strict warning:

Warning: Illegal offset type in isset or empty in /Applications/MAMP/htdocs/wordpress/wp-includes/query.php on line 1715

I wasn't able to create a fix which also incorporates dd32's additions as I'm not entirely sure what the point of the $q[ $q['post_type'] ] business is.

Attachments (1)

12704.diff (3.9 KB) - added by dd32 4 years ago.
largely untested

Download all attachments as: .zip

Change History (22)

comment:1 follow-up: dd324 years ago

  • Status changed from new to accepted

I wasn't able to create a fix which also incorporates dd32's additions as I'm not entirely sure what the point of the $q[ $qpost_type? ] business is.

This is for handling queries such as /wiki/this-is/a/page/ which resolves to ?wiki=this-is/a/page which will result in $q[ $qpost_type? ] = this-is/a/page

I'll have a look into the options available for fixing this over the weekend.

comment:2 aaroncampbell4 years ago

  • Cc aaron@… added

comment:3 in reply to: ↑ 1 mitchoyoshitaka4 years ago

Replying to dd32:

I wasn't able to create a fix which also incorporates dd32's additions as I'm not entirely sure what the point of the $q[ $qpost_type? ] business is.

This is for handling queries such as /wiki/this-is/a/page/ which resolves to ?wiki=this-is/a/page which will result in $q[ $qpost_type? ] = this-is/a/page

Hmm, I see... so is it correct to say that that $q[ $q['post_type'] ] mapping is only necessary for queries based on URL like your example?

Thanks dd32.

comment:4 dd324 years ago

Commit coming up which i believe will fix this..

I thought about just adding an !is_array() check in there, But i feared that might cause implementation headaches down the line.

Instead, I'm looping over the post_types, and adding the requests posts to the listing.

An example SQL:

 SELECT   wp_posts.* FROM wp_posts  WHERE 1=1  AND (1=0 OR wp_posts.post_name IN('test') OR wp_posts.ID IN(173)) AND wp_posts.post_type IN ('note', 'wiki') AND (wp_posts.post_status = 'publish')  ORDER BY wp_posts.post_date DESC

comment:5 dd324 years ago

(In [13838]) Support multiple post_type query params/post_type's to be specified in a query. See #12704

comment:6 dd324 years ago

(In [13839]) Remove Debug cruft.. See #12704

comment:7 follow-up: ryan4 years ago

The following get_posts() call breaks with this change:

get_posts(array('post_type' => 'any', 'name' => 'apple', 'numberposts' => 1, 'suppress_filters' => false));

The query produced before this change looks like this:

 SELECT wp_trunk_posts.* FROM wp_trunk_posts WHERE 1=1 AND wp_trunk_posts.post_name = 'apple' AND wp_trunk_posts.post_type != 'revision' AND wp_trunk_posts.post_type != 'nav_menu_item' AND (wp_trunk_posts.post_status = 'publish') ORDER BY wp_trunk_posts.post_date DESC

After the change it looks like this:

SELECT wp_trunk_posts.* FROM wp_trunk_posts WHERE 1=1 AND wp_trunk_posts.post_type != 'revision' AND wp_trunk_posts.post_type != 'nav_menu_item' AND (wp_trunk_posts.post_status = 'publish') ORDER BY wp_trunk_posts.post_date DESC 

Notice that post_name is not in the query. Since this query has no limit, all published posts and pages are retrieved. This can result in memory exhaustion and bad performance.

comment:8 O.S.4 years ago

I'm having the same problem. The 'name' parameter isn't being passed when you try to use a 'post_type' parameter now.

comment:9 dd324 years ago

O.S.: On the latest nightly?

This commit missed the ticket ~12hours ago:

[wp-svn] [14027] trunk/wp-includes/query.php: Fix a case where $post_type = 'any' causes a large query to run without any specifications (post_name/limits/ id/etc). See #12704

comment:10 O.S.4 years ago

That revision fixed the problem when using 'any', but when i use a another custom post type name('books', 'movies', etc) it's still sending back every post in that type.

comment:11 in reply to: ↑ 7 O.S.4 years ago

I think I figured it out. I changed line 1758 from

} elseif ( '' != $q['name'] ) {

to

}
if ( '' != $q['name'] ) {

comment:12 dd324 years ago

  • Ignoring the post_type_object->query_var
  • Not accounting for cases where the query may be array( post_type => fruits, name => apple) (instead of query_var => apple)

Both of those combined seem to be leading to that, Instead of $_post_type it should be checking the object query var. etc. If someone doesn't get to it before i get home from work, I'll take another look (~9hours).

changing that if branch will/may have other implications for custom post querying.

I'm tempted to change the current loop from what it currently is, to a separate block, and filling name/pagename respectivly and letting those branches take care of the selection, should reduce duplication and fix this issue. - But I'll need to look at the code closer.

dd324 years ago

largely untested

comment:13 dd324 years ago

attachment 12704.diff added

Largely untested, but passes every situation i threw at it late last night. I probably missed one however.

I noticed a weird query:

SELECT   wp_posts.* FROM wp_posts  WHERE 1=1  AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish')  ORDER BY wp_posts.ID ASC LIMIT 0, 1

coming from

require, require_once, include, get_header, locate_template, load_template, require_once, wp_head, do_action, call_user_func_array, start_post_rel_link, get_boundary_post_rel_link, get_boundary_post, get_posts, WP_Query->query, WP_Query->get_posts

after this patch, i'm not sure if it was there before or not.

comment:14 dd324 years ago

(In [14072]) Alternative implementation of custom post_type query_var handling. fills name & pagename for custom post_types to reduce amount of code duplication. See #12704

comment:15 follow-ups: prettyboymp4 years ago

This solution creates a problem where a taxonomy can't have the same name of a post_type. So having a post_type of 'download' and a related taxonomy name 'download' you get a query that looks like this when trying to view a post named 'american-red-cross' (original queryvars = download=american-red-cross, post_type=download):

SELECT SQL_CALC_FOUND_ROWS wp_posts.* FROM wp_posts WHERE 1=1 AND (1=0 OR wp_posts.post_name IN('american-red-cross')) AND 0 AND wp_posts.post_type = 'download' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'private') ORDER BY wp_posts.post_date DESC LIMIT 0, 20

comment:16 in reply to: ↑ 15 prettyboymp4 years ago

  • Priority changed from normal to high

Replying to prettyboymp:

This solution creates a problem where a taxonomy can't have the same name of a post_type. So having a post_type of 'download' and a related taxonomy name 'download' you get a query that looks like this when trying to view a post named 'american-red-cross' (original queryvars = download=american-red-cross, post_type=download):

This is a backwards compatibility issue.

comment:17 nacin4 years ago

Related to that: #11531

comment:18 in reply to: ↑ 15 dd324 years ago

Replying to prettyboymp:

This solution creates a problem where a taxonomy can't have the same name of a post_type. So having a post_type of 'download' and a related taxonomy name 'download' you get a query that looks like this when trying to view a post named 'american-red-cross' (original queryvars = download=american-red-cross, post_type=download):

That, from what i can tell, is unrelated to the issue at hand here. Allowing for multiple post_types and/or taxonomies to register the same query var should probably be raised in its own ticket.. unless this is actually caused here?

comment:19 prettyboymp4 years ago

The issue was caused from this ticket. Changing the handling to use
$qv[$_post_type] instead of $q['name'] for post names of custom post_types is interfering with the handling that was setup previously for taxonomies where $qv[$t->query_var] is used when checking against taxonomies (currently line 1421) if a post_type has the same name as a taxonomy. If this were all completely new functionality, that would be fine, we can say that post_types can't share names with taxonomies, but since the ability to have both has been around since 2.9, this is going to break some sites that have done this.

comment:20 dd324 years ago

Changing the handling to use $qv[$_post_type] instead of $qname? for post names of custom post_types is interfering with the handling that was setup previously for taxonomies where $qv[$t->query_var] is used when checking against taxonomies (currently line 1421) if a post_type has the same name as a taxonomy.

Ah, That was a oversight of the original incarnation. [14072] fixed that.

comment:21 dd324 years ago

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

Marking as fixed, re-open if any issues are stil existing. (Or open new tickets for unrelated issues).

Note: See TracTickets for help on using tickets.