get_adjacent_post excluded_categories isn't excluded

[PatNarciso]

The category I expect to be excluded in $excluded_categories, is not excluded.

With get_adjacent_post,
Assuming $in_same_cat = TRUE
and $excluded_categories = 'ID_OF_CATEGORY'

$cat_array gets built with the ID's of categories it should include. cool.

But, when $excluded_categories contains an ID that $cat_array contains, the ID is removed from the $excluded_categories by array_diff() (link-template.php:1155).

So: the category I expect to be excluded in $excluded_categories, is not excluded.

My resolution: I remove 1154-1157.

if ( ! empty( $cat_array ) ) {
 $excluded_categories = array_diff($excluded_categories, $cat_array);
 $posts_in_ex_cats_sql = '';
}

[jessepollak]

Thanks for the bug report PatNarciso (and a belated-congrats on your first one!), sorry you never got a response!

I just confirmed that this bug is still around in trunk at revision 27076 (though the code has changed since he made the bug report, so things look a little different).

To recreate:

1. Create a three new posts Post 1, Post 2, and Post 3.
2. Give both Post 1 and Post 2 a category of "bad" and give both Post 1 and Post 3 a category of "good"
3. Run

global $post;
$post = POST_1_ID;
$adjacent_post = get_adjacent_post(
    true, // in the same category
    'ID_OF_BAD_CATEGORY', // exclude the test category
    false // should be the next rather than previous post
);

Expected:

This should return Post 3 because, while Post 2 is in the same category as Post 1, it has the excluded_category "bad."

Actual

It returns Post 2 which has the excluded category "bad".

Why

This happens because, as PatNarciso reports, the $excluded_terms array is array_diff'd with the $term_array of acceptable terms. This means that any category that the both the post has and is excluded, will be included.

I think this fix should be as easy as just removing the array_diff line at 1167 — it doesn't seem to serve any purpose but to create this error case. That said, this may not be an error case and this fix will have an affect on the "next" link of a whole lot of blogs, so I'm not sure it's something we want to patch.

I'll defer to core members on whether this is something we want to fix — if so, I'd be happy to write the quick patch for it.

Thanks again PatNarciso!

[jessepollak]

So, I started off by implementing the solution above, but quickly realized that it was insufficient. Removing that array_diff leaves the possibility that we'll have a query that selects posts which are both IN and NOT IN a given term. While discovering the problem with that solution, I also discovered that any time both same_term and excluded_terms are used in get_adjacent_post, we generate a query with both an IN and a NOT IN statement — a little redundant.

In the patch below, I take the following approach:

1. I get the {{${excluded_terms}}} and $same_term_array.
2. I call $same_term_array = array_diff($same_term_array, $excluded_terms), which removes all excluded terms from the $same_term_array
3. If the $same_term_array is not empty, I add an AND IN for the $same_term_array
4. If the $same_term_array is empty and the $excluded_terms is not empty, I add a AND NOT IN for the $excluded_terms

Since both IN and NOT IN are exclusive, either step (4) or step (5) will ensure the adjacent post has the correct terms.

I added a unit test to demonstrate this failure case and verify the patch. I also changed the $term_array to $same_term_array because it's much clearer.

NOTE: this change does affect the get_$adjacent_post_where filter, but I don't think it should be a problem — would appreciate feedback on this.

[jessepollak]

22112.patch

[jessepollak]

22112.2.patch

[jessepollak]

Just added a second patch because I realized I'd mistakenly replaced $term_array in a few places that I didn't mean to. Refer to 22112.2.patch please.

[SergeyBiryukov]

Just got bitten by this.

[jessepollak]

I sort of tabled this ticket because I presumed it would be fixed in #26937, but since that's been reverted and moved to future release, I think this should be added. Is there anything else I should do to help this get into 4.0?

[kovshenin]

Hi! I think the query for exclusion is wrong here. The regular NOT IN (5) will look for at least one row that doesn't match 5, and it'll find plenty if the post has more than one category even if there's a 5. I think we should be reversing this in a subquery, like we do with tax queries. Took a stab in 22112.diff​.

@jessepollak fyi the unit tests in your patch use wp_set_object_terms with append=false, which means every next call on the same object will overwrite the previous set of categories, rather than append.

[jessepollak]

Heh, I guess I should leave the actual patching up to the professionals. That looks much better, thanks :)

[kovshenin]

Replying to jessepollak:

Heh, I guess I should leave the actual patching up to the professionals.

Not really. I actually took your patch to start with which helped a lot, and simply ended up changing a lot of things and reverting some, and when I said "wrong" I was referring to the current code in core, not your patch. You should definitely not feel discouraged. Thank you for your hard work!

[jessepollak]

:thumbsup:

[jessepollak]

For future reference, would you mind pointing me to a place where that subquery strategy is used elsewhere? I feel like it's those kind of small style/pattern things that often elude new contributors :)

[kovshenin]

Sure, here's the tax query code I was referring to.

[jessepollak]

Ah, thanks. Is that done explicitly to avoid situations like this where IN and NOT IN conflict or is there another motive?

[DrewAPicture]

22112.diff​ still applies and unit tests pass.

[wonderboymusic]

In 29248:

In get_adjacent_post(), make $excluded_terms work as expected.

Adds unit tests.
Props jessepollak, kovshenin.
Fixes #22112.

[SergeyBiryukov]

#29050 was marked as a duplicate.

[betzster]

I don't think we need a subquery here. Excluding the $excluded_terms from $term_array should be enough to solve the original issue. We do need to check that $term_array is valid (it exists and is not an error) before manipulating it though.

22112.3.patch fixes this.

Currently in trunk:

mysql> EXPLAIN SELECT p.ID FROM wp_posts AS p INNER JOIN wp_term_relationships AS tr ON p.ID = tr.object_id INNER JOIN wp_term_taxonomy tt ON tr.term_taxonomy_id = tt.term_taxonomy_id WHERE p.post_date < '2014-08-26 14:01:03' AND p.post_type = 'post' AND p.post_status = 'publish' AND tt.taxonomy = 'category' AND tt.term_id IN (7386,193894091,828) AND p.ID NOT IN ( SELECT object_id FROM wp_term_relationships WHERE term_taxonomy_id IN (22006310) ) ORDER BY p.post_date DESC LIMIT 1;
+----+--------------------+-----------------------+-----------------+-----------------------------------+------------------+---------+-------------------------------+------+----------------------------------------------+
| id | select_type        | table                 | type            | possible_keys                     | key              | key_len | ref                           | rows | Extra                                        |
+----+--------------------+-----------------------+-----------------+-----------------------------------+------------------+---------+-------------------------------+------+----------------------------------------------+
|  1 | PRIMARY            | tt                    | range           | PRIMARY,term_id_taxonomy,taxonomy | term_id_taxonomy | 106     | NULL                          |    3 | Using where; Using temporary; Using filesort |
|  1 | PRIMARY            | tr                    | ref             | PRIMARY,term_taxonomy_id          | term_taxonomy_id | 8       | wordpress.tt.term_taxonomy_id |   11 | Using where                                  |
|  1 | PRIMARY            | p                     | eq_ref          | PRIMARY,type_status_date          | PRIMARY          | 8       | wordpress.tr.object_id        |    1 | Using where                                  |
|  2 | DEPENDENT SUBQUERY | wp_term_relationships | unique_subquery | PRIMARY,term_taxonomy_id          | PRIMARY          | 16      | func,const                    |    1 | Using index; Using where                     |
+----+--------------------+-----------------------+-----------------+-----------------------------------+------------------+---------+-------------------------------+------+----------------------------------------------+

With 22112.3.patch:

mysql> EXPLAIN SELECT p.ID FROM wp_posts AS p INNER JOIN wp_term_relationships AS tr ON p.ID = tr.object_id INNER JOIN wp_term_taxonomy tt ON tr.term_taxonomy_id = tt.term_taxonomy_id WHERE p.post_date < '2014-08-26 14:01:03' AND p.post_type = 'post' AND p.post_status = 'publish' AND tt.taxonomy = 'category' AND tt.term_id IN (7386,193894091,828) AND tt.taxonomy = 'category' AND tt.term_id NOT IN (22006310) ORDER BY p.post_date DESC LIMIT 1;
+----+-------------+-------+--------+-----------------------------------+------------------+---------+-------------------------------+------+----------------------------------------------+
| id | select_type | table | type   | possible_keys                     | key              | key_len | ref                           | rows | Extra                                        |
+----+-------------+-------+--------+-----------------------------------+------------------+---------+-------------------------------+------+----------------------------------------------+
|  1 | SIMPLE      | tt    | range  | PRIMARY,term_id_taxonomy,taxonomy | term_id_taxonomy | 106     | NULL                          |    3 | Using where; Using temporary; Using filesort |
|  1 | SIMPLE      | tr    | ref    | PRIMARY,term_taxonomy_id          | term_taxonomy_id | 8       | wordpress.tt.term_taxonomy_id |   11 |                                              |
|  1 | SIMPLE      | p     | eq_ref | PRIMARY,type_status_date          | PRIMARY          | 8       | wordpress.tr.object_id        |    1 | Using where                                  |
+----+-------------+-------+--------+-----------------------------------+------------------+---------+-------------------------------+------+----------------------------------------------+

[pento]

22112.3.patch​ is incorrect, it's failing the test_get_adjacent_post_exclude_self_term unit test.

It's incorrect because tt.term_id is in $term_array, which can never contain items from $excluded_terms, so the NOT IN will always be true.

The only way to exclude categories is with a subquery (as we currently have), or a LEFT JOIN, as demonstrated in 22112.4.patch​. This patch makes me sad, because it's harder to read than the subquery logic, and doesn't reduce the number of tables examined (so probably won't provide any performance benefits).

Unless there are clear performance benefits to 22112.4.patch​ (I didn't have time to benchmark it today), I'm fine with re-closing this ticket.

[helen]

Let's keep pento's last patch in our minds and hearts, but this is fixed for 4.0.

[pento]

In 30401:

Fix a MySQL warning in the test_get_adjacent_post_exclude_self_term unit test, caused by [30400].

See #21212, #22112.

[nacin]

In 30405:

In get_adjacent_post(), $excluded_terms should check term_id rather than term_taxonomy_id.

Merges [30263] (and [30264] [30401]) to the 4.0 branch.

props boonebgorges.
fixes #29663, see #22112.