Opened 16 years ago
Closed 15 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)
Change History (22)
#3
@
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 "; }
#5
@
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.
#6
follow-up:
↓ 7
@
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
@
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:
↓ 9
@
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
@
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.
#10
@
15 years ago
How about that? Remove all traces of post_status_join and exclude revisions from post_type = any queries.
#11
@
15 years ago
That looks good. The patch works well for me, and filtering out revisions is a good addition!
#13
@
15 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
This breaks plugins that add custom taxonomies for attachments.
#15
@
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:
- Install WP trunk from SVN (2.8-bleeding)
- Copy test-taxonomies.php to plugins; activate the plugin
- Set permalinks to 'Day and name'
- Add a post: Title 'My first thing'; Assign it a 'Things' term of 'first'; publish
- Add a post: Title 'My second thing'; Assign it a 'Things' term of 'second'; publish
- Visit the site URL '/thing/second/' .. result: 404
- In wp-includes/query.php ca. line 1961, change to read: '$post_status_join = false'
- 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
@
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
@
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'.
punting