WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#8731 closed defect (bug) (fixed)

Taxonomies set $post_status_join = true seems unnecessarily

Reported by: bkrausz Owned by: ryan
Milestone: 2.8 Priority: normal
Severity: major Version: 2.7
Component: Taxonomy Keywords: has-patch
Focuses: Cc:

Description

Pretty self explanatory. On line 1964 in wp-includes/query.php $post_status_join is set to true if a taxonomy search is done. This makes it impossibly to use a query to limit a taxonomy search to published posts.

For example, /wp-admin/edit.php?taxonomy=thing&term=thing-1 will return revisions regardless of any other parameters, making impossible to duplicated the functionality of, say /wp-admin/edit.php?category_name=uncategorized, which will only show published posts.

Attachments (4)

trac_8731_sample.txt (2.1 KB) - added by gortsleigh 5 years ago.
Relevant wp_posts rows and generated SQL for a simple query
8731.diff (449 bytes) - added by ryan 5 years ago.
8731.2.diff (1.4 KB) - added by ryan 5 years ago.
test-taxonomies.php (966 bytes) - added by azaozz 5 years ago.
Simple plugin to test with

Download all attachments as: .zip

Change History (22)

comment:1 ryan5 years ago

  • Component changed from General to Taxonomy
  • Owner changed from anonymous to ryan

comment:2 Denis-de-Bernardy5 years ago

  • Keywords needs-patch added
  • Milestone changed from 2.8 to Future Release

punting

gortsleigh5 years ago

Relevant wp_posts rows and generated SQL for a simple query

comment:3 gortsleigh5 years ago

  • Cc gortsleigh added
  • Keywords has-patch added; needs-patch removed
  • Milestone changed from Future Release to 2.8
  • Severity changed from normal to major

I have an issue with this join that is not just an annoyance; it is returning incorrect results.

My WP application has registered several custom taxonomies (for example, 'artist'). There are a number of posts generated via an import filter; each post has an artist associated using a call like:

wp_set_object_terms($newid, $artist_slug, 'artist', false);

When I make a query for the posts associated with a specific artist, WordPress returns no results (I guess because there are not multiple revisions of the matching posts?). If I force the value of $post_status_join to false, the correct posts are returned. This seems to me a serious bug.

It also seems unnecessary as there does not seem to be any special need to return revision data for a taxonomy query as opposed to a query with any other criteria. I would think that most programmers would expect taxonomy queries to return published posts, just the same as a category or tag search would do.

I have attached a file with relevant wp_posts rows from my WP DB, along with the SQL that WP generated from a query of the form "/index.php?artist=son-volt"

I hope you will reconsider this for 2.8

I propose the patch be as follows:

ndex: wp-includes/query.php
===================================================================
--- wp-includes/query.php	(revision 11373)
+++ wp-includes/query.php	(working copy)
@@ -1958,7 +1958,6 @@
 					$whichcat .= " AND $wpdb->posts.ID IN (" . implode(', ', $post_ids) . ") ";
 					$post_type = 'any';
 					$q['post_status'] = 'publish';
-					$post_status_join = true;
 				} else {
 					$whichcat = " AND 0 ";
 				}

comment:4 ryan5 years ago

Related changesets [7491] and [7520].

As far as I can tell, $post_status_join exists solely for this case. If we don't use it here it can be removed.

comment:5 ryan5 years ago

I think we have to retain this for attachments to work. Instead, we need to exclude revision post types from the query.

ryan5 years ago

comment:6 follow-up: ryan5 years ago

Patch excludes revisions when post_type is set to 'any'. This will keep revisions from showing up where not expected.

comment:7 in reply to: ↑ 6 gortsleigh5 years ago

Replying to ryan:

Patch excludes revisions when post_type is set to 'any'. This will keep revisions from showing up where not expected.

Ryan, this patch does not change my results, as the JOIN returns no results. I updated the posts from wp-admin/edit.php just in case I was missing something in using wp_insert_post() -- still no results.

The question is why the JOIN returns zero results. query.php at about line 1956 sets $post_ids to the result of get_objects_in_term( - the posts returned by that function all have a post_parent of 0, which is why the join returns no results.

The join looks like

SELECT SQL_CALC_FOUND_ROWS wp_posts.* FROM wp_posts JOIN wp_posts AS p2 ON (wp_posts.post_parent = p2.ID) WHERE 1=1 AND wp_posts.ID IN (20, 37, 71) AND wp_posts.post_type != 'revision' AND ((wp_posts.post_status = 'publish') OR (wp_posts.post_status = 'inherit' AND (p2.post_status = 'publish'))) ORDER BY wp_posts.post_date DESC LIMIT 0, 25

The posts here with IDs 20,37,71 have post_parent = 0 because they are primary posts, ie not revisions. It seems the join is wrong ... that the left-hand side should remain the wp_posts.ID, and the join should be based on p2.post_parent = wp_posts.ID

Unfortunately, I'm not clear on what the result set should like like if there are revisions being returned, so I'm not sure that simply reversing the criteria is what you need. It *does* return the correct results for me, however!

comment:8 follow-up: ryan5 years ago

Ah, okay. I think the idea is to get any attachments belonging to those posts, but I'm not really too clear on it myself. I thought the plugins that add taxonomies to images used this, but perhaps not since it seems rather broken.

Could you attach a patch containing the reversed criteria that worked for you? I'll find one of the image taxonomy plugins and test it against the patch. If all else fails, we'll go with your origin suggestion. I just want to make sure we're not breaking something else in the process of fixing this.

comment:9 in reply to: ↑ 8 gortsleigh5 years ago

I can't come up with a good explanation for this join. If I "reverse" the join as I had mentioned above, I will get multiple rows of duplicate posts for posts that have attachments, due to the absence of a GROUP BY clause. Not useful, with or without GROUP BY. With the join unchanged, I get zero results even when there are posts with attachments. This still looks like a bug to me, and I vote for going with my original patch.

ryan5 years ago

comment:10 ryan5 years ago

How about that? Remove all traces of post_status_join and exclude revisions from post_type = any queries.

comment:11 gortsleigh5 years ago

That looks good. The patch works well for me, and filtering out revisions is a good addition!

comment:12 ryan5 years ago

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

(In [11392]) Lose broken post_status_join. Exclude revisions from post_type = any queries. Props gortsleigh. fixes #8731

comment:13 ryan5 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This breaks plugins that add custom taxonomies for attachments.

comment:14 ryan5 years ago

(In [11420]) Revert [11392]. It breaks custom taxonomies for attachments. see #8731

azaozz5 years ago

Simple plugin to test with

comment:15 gortsleigh5 years ago

I have had time again to test this using the 'test-taxonomies.php' attachment above, and it is still broken.

I have verified a simple reproducible set of steps to show the error. These steps are not showing the generated wp_rules or the query, simply the results you see in the browser.

To reproduce:

  1. Install WP trunk from SVN (2.8-bleeding)
  2. Copy test-taxonomies.php to plugins; activate the plugin
  3. Set permalinks to 'Day and name'
  4. Add a post: Title 'My first thing'; Assign it a 'Things' term of 'first'; publish
  5. Add a post: Title 'My second thing'; Assign it a 'Things' term of 'second'; publish
  6. Visit the site URL '/thing/second/' .. result: 404
  7. In wp-includes/query.php ca. line 1961, change to read: '$post_status_join = false'
  8. re-visit URL '/thing/second/' .. result: the post 'My second thing' is displayed.

I get the same results whether the post has attachments or not. The fault, as before, lies in the join

JOIN ON (wp_posts.post_parent = p2.ID)

Is it possible that the join is only correct when post_type == 'attachment' ?? If so, $post_status_join should only be set to true when $where is set to select that type, eg near line 324 of query.php

comment:16 ryan5 years ago

See [11452] which changed the JOIN back to a LEFT JOIN, which is the way it was in 2.7. Do you see this bug in 2.7.1 as well, or just 2.8?

comment:17 gortsleigh5 years ago

Ryan, this is more difficult to test in 2.7.1 because the custom taxonomies are not presented in the admin interface.

However, it DOES work in 2.8-bleeding! And I must say in retrospect that the join even makes sense to me now :) -- it will return published posts (left-hand side) whether or not there are attachments. I can see that this will also return posts that have attachments with associated taxonomy terms. Pretty slick. I would say r11452 did the trick, and this issue can be closed as 'fixed'.

comment:18 Denis-de-Bernardy5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed
Note: See TracTickets for help on using tickets.