Use meta_value in a meta query to decide type format in SQL clause

[ericlewis]

The SQL clause generated for a meta query ​quotes the `meta_value` in a string.

This means that if there's a post with a postmeta field for likes set to 2 and you run the query looking for posts with 10 or more likes

<?php
$query = new WP_Query( array(
        'meta_query' => array(
                array(
                        'key' => 'likes',
                        'value' => 10,
                        'compare' => '>='
                )
        )
) );

the query will return the post with 2 likes. This is because the SQL is doing a string comparison, as both the column value and the compared-to value are strings.

The fix for the developer is to supply a type parameter like NUMERIC in the meta query clause which coerces a numeric MySQL comparison.

We could use the meta_value's type to decide the type format the value takes in the SQL clause, so that a query like this works as expected without the type parameter.

This was suggested by @boone in #27272.

[boonebgorges]

36652.diff​ is a first pass at how this might work. High-level overview:

• Takes a conservative approach to determining when 'type' can be inferred:
  • is_int( $value ) || is_float( $value ), and 'compare' is of the type that makes sense with numeric casts (=, !=, <, <=, >, >=)
  • $value is an array, and *each* member of the array is a float or an integer, and 'compare' is either BETWEEN or NOT BETWEEN
• Inferring 'type' is not enough. We also have to infer the proper type selectors for $wpdb->prepare() - %d, %f, %s

The tests in 36652.diff​ should spell this out pretty clearly.

A few questions for consideration.

1. 36652.diff​ opts to infer SIGNED when 'compare' is = or !=. This means that certain kinds of meta queries common in core are CAST() to SIGNED when they probably don't have to be. This is especially true in places where you're storing IDs in meta (see eg wp_get_associated_nav_menu_items()) - meta_value = '3' is the same as CAST(meta_value AS SIGNED) = 3 for these purposes. The CAST may be slightly slower, and breaks the use of any index that might exist. On the flip side, the CAST is important if you want mathematical matches (3 == 3.00). So I'm a bit torn on the expected behavior.
2. I'm using the term "type selector" for %s etc because that's what the PHP docs use ​http://php.net/manual/en/function.sprintf.php. If anyone has a better idea ("placeholder"?), shoot.
3. Possible compat breaks are cases where one is passing numeric values but is expecting string behavior. Eg: "10" < "1" (alpha sort) but 10 > 1 (numerical sort). It's only in certain corner cases that this problem will ever arise. My guess is that the type-inferring suggested in this ticket will cause a number of bugs like this that is far smaller than the number of type-related bugs it'll fix. But a second opinion would be helpful.

[ocean90]

@ericlewis Can you review @boonebgorges's patch and decide if this is still something for 4.6? Looks like the change should go in ASAP.

[ericlewis]

Awesome to see an implementation for this already :D

Replying to boonebgorges:

1. 36652.diff​ opts to infer SIGNED when 'compare' is = or !=.

Casting float values to SIGNED will make them integers, and miss matches. e.g.

update_post_meta( 1, 'glerf', '1.1' );

$q = new WP_Query(array(
  'meta_query' => (
    array(
      array(
        'key' => 'glerf',
        'value' => 1.1
      )
    )
  )
));

2. I'm using the term "type selector" for %s etc because that's what the PHP docs use ​http://php.net/manual/en/function.sprintf.php. If anyone has a better idea ("placeholder"?), shoot.

Adhering to PHP's verbiage here would be nice.

3. Possible compat breaks are cases where one is passing numeric values but is expecting string behavior. Eg: "10" < "1" (alpha sort) but 10 > 1 (numerical sort). It's only in certain corner cases that this problem will ever arise. My guess is that the type-inferring suggested in this ticket will cause a number of bugs like this that is far smaller than the number of type-related bugs it'll fix. But a second opinion would be helpful.

That's a good question. In this ticket we would introduce support for passing the meta_value parameter as
an int or float, which has been supported previously, but not in the inline documentation. The case you suggest
does seem rather edge — expecting alpha sort on comparing 10 to 1. Sidenote, we should
probably change the docs for meta_value if we move forward here.

[ocean90]

Thanks for the review @ericlewis. Punting since it needs a refresh and beta 1 is tomorrow.