Make WordPress Core

Opened 16 years ago

Closed 15 years ago

#8731 closed defect (bug) (fixed)

Taxonomies set $post_status_join = true seems unnecessarily

Reported by: bkrausz's profile bkrausz Owned by: ryan's profile 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 15 years ago.
Relevant wp_posts rows and generated SQL for a simple query
8731.diff (449 bytes) - added by ryan 15 years ago.
8731.2.diff (1.4 KB) - added by ryan 15 years ago.
test-taxonomies.php (966 bytes) - added by azaozz 15 years ago.
Simple plugin to test with

Download all attachments as: .zip

Change History (22)

#1 @ryan
15 years ago

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

#2 @Denis-de-Bernardy
15 years ago

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

punting

@gortsleigh
15 years ago

Relevant wp_posts rows and generated SQL for a simple query

#3 @gortsleigh
15 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 ";
 				}

#4 @ryan
15 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.

#5 @ryan
15 years ago

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

@ryan
15 years ago

#6 follow-up: @ryan
15 years ago

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

#7 in reply to: ↑ 6 @gortsleigh
15 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!

#8 follow-up: @ryan
15 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.

#9 in reply to: ↑ 8 @gortsleigh
15 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.

@ryan
15 years ago

#10 @ryan
15 years ago

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

#11 @gortsleigh
15 years ago

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

#12 @ryan
15 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

#13 @ryan
15 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This breaks plugins that add custom taxonomies for attachments.

#14 @ryan
15 years ago

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

@azaozz
15 years ago

Simple plugin to test with

#15 @gortsleigh
15 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

#16 @ryan
15 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?

#17 @gortsleigh
15 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'.

#18 @Denis-de-Bernardy
15 years ago

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