Meta query doesn't work with $wp_query->set()

[scribu]

The following doesn't work, apparently:

add_action( 'pre_get_posts', function( $query ) {
  $query->set('meta_key','oqp_guest_email');
  $query->set('meta_value',$dummy_email);
} );

Source: ​http://wordpress.org/support/topic/pre_get_posts-problem

Also this one generates some invalid sql

add_action( 'pre_get_posts', function( $query ) {
$query->set( 'meta_key', 'test' );
$query->set( 'orderby', 'meta_value' );
return $query;
} );

sql query

SELECT SQL_CALC_FOUND_ROWS wp_posts.* FROM wp_posts WHERE 1=1 AND wp_posts.post_type = 'post' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'private') GROUP BY wp_posts.ID ORDER BY wp_postmeta.meta_value DESC LIMIT 0, 10

I thing we're going to have to store the parsed meta_query in it's own property, like we do for the tax_query.

That will cause duplicate meta queries:

query_posts( array( 'meta_key' => 'foo' ) );

After the final call to _parse_meta_query(), 'meta_query' will be:

Array
(
    [0] => Array
        (
            [key] => foo
        )

    [1] => Array
        (
            [key] => foo
        )
)

Introduce WP_Meta_Query

16742.class.diff​ is a lot simpler than the diff suggests, but it's still a pretty big change for a point release.

Replying to scribu:

16742.class.diff​ is a lot simpler than the diff suggests, but it's still a pretty big change for a point release.

For 3.1.1 maybe we can use hashing(like tax query) and use WP_Meta_Query in 3.2

16742.hash.diff​ doesn't solve the original issue in this bug.

Actually, it does fix the original bug, but it stomps the original 'meta_query', if it's set:

query_posts( array(
	'meta_query' => array(
		array( 'key' => 'foo' )
	)
) );

After further testing, it seems to work as expected.

Maybe we should just leave it for 3.2, instead of using clever, yet contrived, workarounds like this.

If moving to 3.2, add codex so that plugin/theme authors are notified of the changes.

Is this not a regression?

Yes, it is. I've expressed my opinion in #comment:10. Your call.

Only a committer should be making a decision to punt a regression.

16742.hash.diff seems good for 3.1.1. We can leave this ticket open and consider scribu's fix for 3.2.

query_vars_changed

That patch may be too much for 3.1.1. It eliminates the multiple md5/serialize calls and sets one query_vars_changed flag. Setting the hash in parse_tax_query() seemed odd now that it is applying to more places.

Clean up comments in .2.diff

(In [17552]) Parse the meta query again if query vars change. Set a global query_vars_changed flag instead of doing multiple hash creation calls. Props greuben. see #16742 for trunk

(In [17553]) Parse the meta query again if query vars change. Set a global query_vars_changed flag instead of doing multiple hash creation calls. Props greuben. see #16742 for 3.1

I went ahead with the hash cleanup. Please review thoroughly. The hash is now computed at the end of parse_query() right before the parse_query action. It is computed again in get_posts() after pre_get_posts and fill_query_vars() have run. If the hash changed via the parse_query or pre_get_posts actions then the query_vars_changed flag is set to true so that the tax and meta parsers know to do their thing again. There are no other filters or actions that run between when the hash is recomputed and when the taxonomy and meta queries are parsed again.

Tested with init, pre_get_posts, and parse_query hooks that exercised a variety of meta and tax queries via set() and get_posts() calls on the main query and newly instantiated queries.

Looking at the current solution, I think it might have been safer to go with 16742.class.diff​ after all. :)

Replying to scribu:

Looking at the current solution, I think it might have been safer to go with 16742.class.diff​ after all. :)

Yeah, the little fix wasn't so little. Though the hash cleanup should benefit further meta query work.

Follow-up: #17165