setting meta_type doesn't CAST orderby value

[wonderboymusic]

Setting meta_type in WP_Query will cast the meta_value in WHERE clauses to whatever Meta_Query internally resolves it as, however it does not create an alias and does not order by that alias. It orders by an un-CAST'd meta_value which doesn't work for numbers. I realize that orderby set to meta_value_num will fix this, but that doesn't change the fact that meta_type is being ignored.

To test:

update_post_meta( 1, 'num_as_longtext', 123 );
update_post_meta( 2, 'num_as_longtext', 99 );

add_filter( 'query', function ( $sql ) { error_log( $sql ); return $sql; } );
$stuff = new WP_Query( array( 
	'fields' => 'ids', 
	'post_type' => 'any', 
	'meta_key' => 'num_as_longtext', 
	'meta_value' => '0',
	'meta_compare' => '>',
	'meta_type' => 'UNSIGNED', 
	'orderby' => 'meta_value', 
	'order' => 'ASC' 
) );

print_r( $stuff->posts );

exit();

That should return 2 then 1, it returns 1 then 2. It generates this SQL:

SELECT SQL_CALC_FOUND_ROWS  wp_posts.ID FROM wp_posts  
INNER JOIN wp_postmeta ON (wp_posts.ID = wp_postmeta.post_id) 
WHERE 1=1  AND wp_posts.post_type IN ('post', 'page', 'attachment') AND (wp_posts.post_status = 'publish') AND ( (wp_postmeta.meta_key = 'num_as_longtext' AND CAST(wp_postmeta.meta_value AS UNSIGNED) > '0') ) 
GROUP BY wp_posts.ID ORDER BY wp_postmeta.meta_value ASC LIMIT 0, 10

My patch returns:

SELECT SQL_CALC_FOUND_ROWS  wp_posts.ID FROM wp_posts  
INNER JOIN wp_postmeta ON (wp_posts.ID = wp_postmeta.post_id) 
WHERE 1=1  AND wp_posts.post_type IN ('post', 'page', 'attachment') AND (wp_posts.post_status = 'publish') AND ( (wp_postmeta.meta_key = 'num_as_longtext' AND CAST(wp_postmeta.meta_value AS UNSIGNED) > '0') ) 
GROUP BY wp_posts.ID ORDER BY CAST(wp_postmeta.meta_value AS UNSIGNED) ASC LIMIT 0, 10

[dd32]

Since we don't currently have a meta_type query var, it's expected that specifying it would result in nothing changing, changing this to an enhancement as a result of that.

In order to order by a meta value numerically, we currently have orderby: meta_value_num which uses a meta_value+0 syntax to convert the field value to a numeric value. Of course, this doesn't work for dates/times

[scribu]

Related: #15031

[nacin]

So this is to add a new meta_type query argument? Looks like this diff will need a bit more, then.

Should meta_value_num convert itself to meta_value = meta_value_num and meta_type = int?

get_meta_type() seems like it belongs as a static method on WP_Meta_Query. Maybe that's just me.

Given both #15031 and #17065, I don't know if this should be pushed into 3.5.

[wonderboymusic]

meta_type already exists:

foreach ( array( 'key', 'compare', 'type' ) as $key ) {
	if ( !empty( $qv[ "meta_$key" ] ) )
		$meta_query[0][ $key ] = $qv[ "meta_$key" ];
}

[nacin]

Replying to wonderboymusic:

meta_type already exists:

foreach ( array( 'key', 'compare', 'type' ) as $key ) {
	if ( !empty( $qv[ "meta_$key" ] ) )
		$meta_query[0][ $key ] = $qv[ "meta_$key" ];
}

Ah, interesting. I think that was an unintended side effect. Note how it is not specified anywhere else. The meta_query inner arrays take 'key', 'value', 'compare', and 'type'. When we convert standard query vars (like meta_key and meta_value) into a meta_query, we ended up looping over all of the possible values for meta_query, rather than the existing standard query variables.

[wonderboymusic]

I think this is mainly for top-level meta query - meta_key, meta_value and the like - meta_value_num is the only way to cast. And I think top-level is the main use case (I hope to jesus people aren't creating many arrays of meta_value LONGTEXT comparisons - barf).

My example from the ticket - I don't care if we ditch the patch for now, but it *does* work

$stuff = new WP_Query( array( 
	'fields' => 'ids', 
	'post_type' => 'any', 
	'meta_key' => 'num_as_longtext', 
	'meta_value' => '0',
	'meta_compare' => '>',
	'meta_type' => 'UNSIGNED', 
	'orderby' => 'meta_value', 
	'order' => 'ASC' 
) );

[nacin]

Yeah, I agree we should do what this ticket proposes. I was just trying to set expectations that meta_type was one of those accidents.

[wonderboymusic]

In 25255:

When meta_type is passed with orderby => meta_value, orderby must also use CAST() to avoid scenarios like: SELECTing by UNSIGNED and then ordering by CHAR. Adds unit test.

Fixes #21621.

[nacin]

[25255] looks good. get_meta_type() feels a bit generic for a function name. I have no idea what it does or returns when looking at it. We also already have get_meta_sql() and _get_meta_table() — between the three of them, who knows what this should be returning. It's also not for getting the type, it's for converting a type to a proper keyword for CAST.

It think it would make the most sense as a static method on WP_Meta_Query().

WP_Meta_Query::get_cast_for_type( $type ), perhaps? WP_Meta_Query::get_cast( $type )?

[wonderboymusic]

WP_Meta_Query::get_cast_for_type( $type ) sounds pretty good - will update in a short bit

[wonderboymusic]

In 25269:

Move get_meta_type() into the WP_Meta_Query class as get_cast_for_type(). WP_Query can then access it like: $this->meta_query->get_cast_for_type().

See #21621, [25255].

[nacin]

I'm fine with it either way, but my suggestion regarding [25269] was to make this a static method, given it clearly pertains to meta queries but is purely utility and does not rely on a WP_Meta_Query object.

[wonderboymusic]

yeah, I did static at first and it looked/felt weird when used in WP_Query - if it's there to generically resolve a CAST type, maybe shouldn't even be on WP_Meta_Query, but I can't think of any other uses for it currently in core. I will keep thinking about it