posts_join_request filter is broken

[5um17]

One of the users reported ​an error with WP 6.0-RC1.

On checking, I found posts_join_request filter is stopped working since latest changes. First [52974] then [53175] for #54728.

I think this is an oversight, we missed adding compact() to

<?php
$clauses = (array) apply_filters_ref_array( 'posts_clauses_request', array( $clauses, &$this ) );

[johnbillion]

Thanks for the report, moving to 6.0 for visibility.

[5um17]

Thanks for the feedback @johnbillion!

[SergeyBiryukov]

Good catch, thanks!

[SergeyBiryukov]

I don't think we should rename the variable back to $pieces though, as using a consistent name was the point of [52974]. Just restoring the compact call, same as for the posts_clauses filter in [53175] would fix the issue.

[SergeyBiryukov]

55699.3.diff​ includes a unit test.

[5um17]

Replying to SergeyBiryukov:

I don't think we should rename the variable back to $pieces though, as using a consistent name was the point of [52974]. Just restoring the compact call, same as for the posts_clauses filter in [53175] would fix the issue.

Thanks @SergeyBiryukov! I renamed the variable because $clauses is being used two times in the code. And its value is changed after posts_clauses filter.

I can see in 55699.2.diff you have reinitialized the variable which is good too. Thanks!

[SergeyBiryukov]

55699.4.diff​ also includes a unit test for [53175].

It is worth noting that posts_clauses and posts_clauses_request were the only filters affected here. [53175] fixed the former, but missed the latter.

Other *_clauses filters adjusted in [52974] and [53175] are unaffected because there are no hooks between the $clauses array definition and the filter itself, so the logic for them has not changed.

[SergeyBiryukov]

In 53370:

Query: Restore late compact() call for the posts_clauses_request filter.

This addresses a backward compatibility break where posts_join_request and other filters were applied, but their results were subsequently discarded and earlier values were used instead.

Follow-up to [52974], [53175].

Props 5um17, johnbillion, SergeyBiryukov.
Fixes #55699.

[SergeyBiryukov]

Reopening for backporting to the 6.0 branch after a second committer's review.

[peterwilsoncc]

I'm concerned about [53370] and it needs additional work before it's ready to be merged in to the 6.0 branch.

In the new code, $clauses undergoes four main definitions:

1. The name of the MySQL sub-clauses
2. The value of the MySQL sub-clauses
3. The name of the MySQL sub-clauses
4. The value of the MySQL sub-clauses

It took me quite a long time to figure out what was happening as a result.

As a defensive coding measure, two variables need to be used one for the names, one for the values of the clauses.

I see in the original patch the suggestion was pieces and clauses. I think this is preferable to what is in trunk at the moment, despite the inconsistency with other query classes.

[SergeyBiryukov]

Replying to peterwilsoncc:

As a defensive coding measure, two variables need to be used one for the names, one for the values of the clauses.

I see in the original patch the suggestion was pieces and clauses. I think this is preferable to what is in trunk at the moment, despite the inconsistency with other query classes.

Sounds good, though if we're going to restore the $pieces name, I would ideally prefer to do that consistently for all query classes, essentially reverting [52974], as I don't see a reason for them to be different, and appreciate consistency where possible. In hindsight, the reason for [52974] was that $clauses seemed like a better name than $pieces for the *_clauses filter parameters, and I think that part can remain.

Does 55699.5.diff​ address the concern?

[peterwilsoncc]

I agree the consistency is a good idea.

For the purposes of the 6.0 back port , are you able to limit the commit to the buggy class only and do the renaming for consistency in a follow up commit.

Based on a visual review the patch and approach looks sound.

[SergeyBiryukov]

In 53375:

Coding Standards: Restore the $pieces variable in WP_Query::get_posts().

This is a defensive coding measure that aims to reduce confusion. With this change, $pieces is explicitly used for the names, and $clauses for the values of the clauses.

Follow-up to [52974], [53175], [53370].

Props peterwilsoncc.
See #55699.

[SergeyBiryukov]

In 53376:

Coding Standards: Restore the $pieces variable for SQL clauses in query classes.

This is a defensive coding measure that aims to reduce confusion. With this change, $pieces is explicitly used for the names, and $clauses for the values of the clauses.

Follow-up to [52974], [53175], [53370], [53375].

Props peterwilsoncc.
See #55699.

[SergeyBiryukov]

Marking [53375] for backporting to the 6.0 branch after a second committer's review.

[53376] can stay in trunk only, per comment:14.

[hellofromTonya]

Marking [53375] dev-reviewed for 6.0-branch backporting per #comment:14.

[SergeyBiryukov]

In 53379:

Query: Restore late compact() call for the posts_clauses_request filter.

This addresses a backward compatibility break where posts_join_request and other filters were applied, but their results were subsequently discarded and earlier values were used instead.

Follow-up to [52974], [53175].

Props 5um17, johnbillion, peterwilsoncc, hellofromTonya, SergeyBiryukov.
Merges [53370] and [53375] to the 6.0 branch.
Fixes #55699.