Some WP_XXX_Query::query() methods produce incorrect results when called in a loop

[pbiron]

While testing a patch for #37392 I came across what may be a bug in WP_Site_Query::query().

That patch creates a WP_Site_Query object and then in a loop does various different query()'s, as in:

$q = new WP_Site_Query();
$args   = array(
        'network_id'    => $network_id,
        'number'        => 1,
        'fields'        => 'ids',
        'no_found_rows' => false,
);

foreach ( array( 'public', 'archived', ... ) as $status ) {
        $_args = $args;
        $_args[ $status ] = 1;
        
        $q->query( $_args );
        // do something with the results of this site query.
}

However, calling query() in a loop like that doesn't produce the expected results other than the 1st time through the loop.

Why? Because when query() calls WP_Site_Query::get_site_ids() on subsequent iterations, the protected class member $sql_clauses still has its value from the previous iteration through the loop and the "new" query basically gets added to the previous queries.

In the case of the above code this results in the query for archive = 1 to actually be public = 1 AND archive = 1 which is not what is intended.

Looking at other WP_XXX_Query() classes, I think the following suffer from the same thing (although I haven't written code to test that):

1. WP_Network_Query
2. WP_Term_Query

but the following do not:

1. WP_Query (because it doesn't use a class member for the clauses)
2. WP_User_Query (because it uses a prepare_query( $query ) method which resets the class member(s))

So, I guess the question is: should these query() methods be expected to work when called multiple times in a loop (with different queries each time) or is that not an intended use?

[johnjamesjacoby]

From having worked closely with WP_Query and WP_User_Query more than any of the others, I can tell you that I wrongly expected the query() method in WP_Site_Query to work this way when used repeatedly.

[pbiron]

Replying to johnjamesjacoby:

From having worked closely with WP_Query and WP_User_Query more than any of the others, I can tell you that I wrongly expected the query() method in WP_Site_Query to work this way when used repeatedly.

I whole heartedly agree that consistency across similar classes/methods is a big advantage for developers (can't tell you how much time I've wasted over the years when dealing with a method in class X that works differently that a seemingly identical one on another class) but wanted to start this off as a discussion about expected behavior and reach consensus on that before jumping in with a patch right off the bat.