Make WordPress Core

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#42409 closed enhancement (fixed)

Add LIKE support to meta_key comparisons in WP_Meta_Query

Reported by: otto42's profile Otto42 Owned by: boonebgorges's profile boonebgorges
Milestone: 5.1 Priority: normal
Severity: normal Version: 4.9
Component: Query Keywords: has-unit-tests has-dev-note needs-codex
Focuses: Cc:

Description

So, it seems that a fair amount of code, custom snippets or code in plugins, is making changes to WordPress SQL queries directly.

The most common case that is causing people issues with 4.8.3 seems to be to allow for meta_key LIKE 'some_%_thing' or similar cases. Some of these come from plugins that allow for dynamically numbers of meta_keys, having the keys in a name_(number)_thing pattern.

From the samples I've seen of custom code doing this (again, largely snippets), the most common pattern seems to be to hook into posts_where, look for "meta_key = 'thing_%'" and then to str_replace it with a LIKE.

We should support this type of LIKE clause without having people resort to ill-advised string replacements in their SQL queries or query fragments directly.

Adding support for this is relatively simple. We add a new clause that can be passed to the meta_query array, in the form of "meta_key_compare" and allowing it to be "LIKE" with it defaulting to "=".

There are two lines that would be needed to be changed to allow for the LIKE to be used, and those would be lines 520 and 564 of class-wp-meta-query.php. Additional code would be needed to support the new argument, and since that argument would be in the $clauses variable, then it should be relatively straightforward.

There may be performance issues associated with using meta_key_compare => LIKE, however, the same performance issues exist for those cases where people are modifying the query already, and no such issues exist for core, which would likely not use the new argument.

Attachments (4)

42409.diff (4.8 KB) - added by mariovalney 6 years ago.
42409.2.diff (5.3 KB) - added by mariovalney 6 years ago.
42409.test.diff (904 bytes) - added by chasewg 6 years ago.
Adds unit test to /tests/phpunit/tests/meta/query.php
42409.3.diff (7.1 KB) - added by boonebgorges 6 years ago.

Download all attachments as: .zip

Change History (31)

@mariovalney
6 years ago

#1 @mariovalney
6 years ago

  • Keywords has-patch added; needs-patch removed

Hey!

Added meta_key_compare as suggested, allowing 'like' and '=' (default) and some comments as well.

Thanks for the great guidance in description.

I'll search how to add the unit tests :D
(but if someone wants to contributing with test, it would be appreciated)

#2 @dd32
6 years ago

There's a few more nuances as to how people have been using LIKE queries too, some of which this will cover, some of it which it won't (or will cause inconsistencies in our API).

For example, it looks like most people are passing the exact value in they wish to have in the LIKE query - that's not how our LIKE support works. When using LIKE with meta_value compares it is effectively a CONTAINS query instead, in other words, we don't support anchored searches like author\_% we only support %author%.

42409.diff adds just that inconsistency, the meta_key is used as-is and compared against in the LIKE expression, but the meta_value LIKE is escaped:

$query = new WP_Query( array(
    'meta_key' => 'key_%', 'meta_compare_key' => 'LIKE',
    'meta_value' => 'value_%', 'meta_compare' => 'LIKE'
) );

echo $wpdb->remove_placeholder_escape( $query->request );
// ... ( wp_postmeta.meta_key LIKE 'key_%' AND wp_postmeta.meta_value LIKE '%value\\_\\%%' ) ...

Ideally the behaviour between the two should remain consistent, and we should determine the best way to force it to be used as-is as the LIKE expression.
Currently the only way to do a anchored LIKE in a meta_value is to use the REGEXP support instead.

(cc @boonebgorges who has touched the meta query class a bunch)

#3 @Otto42
6 years ago

Underscore characters also have special cases in a LIKE with MySQL. Perhaps we want to properly handle those as well.

#4 @kokers
6 years ago

Plus, if we already touch this part of code, it would be nice to allow other compare options like it is now for meta value with 'LIKE', 'NOT LIKE','IN', 'NOT IN','EXISTS', 'NOT EXISTS', etc. I don't think '!=', '>', '>=', '<', '<=', 'BETWEEN', 'NOT BETWEEN' would be needed, as this is more value related, but i can see using those other operators.

#5 @mariovalney
6 years ago

I agree with @dd32 but I thought it would be elucidated by documentation. My mistake.

Particularly I was inclined to suggest to allow as-is as the LIKE expression, if there is a '%' inside meta_key value because it's more natural to developers and it's not trivial in meta_key names (keeping the "CONTAINS behavour" to backwards compatibility if there's no '%' in name) but as @Otto42 pointed we should take care of "_" too and it's very trivial to be used in meta_key / meta_value string.

So... maybe creating a "RAW_LIKE / SQL_LIKE" comparator? Or keeping the "CONTAINS behavour" and create a filter to allow as-is values?

Anyway 42409.2.diff corrects the consistency between meta_key and meta_value LIKEs so we can talk about using as-is expressions.

Last edited 6 years ago by mariovalney (previous) (diff)

@mariovalney
6 years ago

#6 @birgire
6 years ago

#42507 was marked as a duplicate.

#7 @joshf
6 years ago

attachment:42409.2.diff is a good start, and I'll echo @dd32's comments on anchored searches, particularly on the meta_key.

I've seen anchored meta_key searches on more than one client site, and it's not all that uncommon to see on sites using ACF. For example:

$cpt_ids = new WP_Query( array(
    'post_type'       => 'cpt',
    'meta_query'      => array(
        array(
            'key'     => 'styles_%_style_id', // ACF likes to place an unique identifier 
                                              // in the middle of the key name.
                                              // So let's match all keys, regardless 
                                              // of that unknown, unique identifier.
            'value'   => $style_id,
            'compare' => '=',
        )
    ), 
) );

This query returns zero results in 4.8.3.

Using attachment:42409.2.diff, the meta_key search has to be simplified in order to make that query work. (And then hope the meta_key search isn't overly vague.)

$cpt_ids = new WP_Query( array(
    'post_type'       => 'cpt',
    'meta_query'      => array(
        array(
            'key'     => 'styles',
            'value'   => $style_id, // This is an unique value.
            'compare' => 'LIKE',
            'compare_key' => 'LIKE'
        )
    ), 
) );

It's a workaround for now, but I can foresee potential issues with simplifying the meta_key search in this way. I like the idea of allowing an 'as-is' option as @mariovalney suggested. That would allow more advanced(?) queries to continue utilizing the WP_Query class.

@chasewg
6 years ago

Adds unit test to /tests/phpunit/tests/meta/query.php

#8 @chasewg
6 years ago

Submitted a unit test for this using a diff patch. Tried Following the contributor handbook, but I'm still not sure if I followed the correct process--so any comments/corrections are greatly appreciated!

<?php
public function test_compare_meta_key(){
            global $wpdb;
            $meta_key='foo_bar';
            $q=new WP_Meta_Query([array(
                'compare_key'=>'LIKE',
                'key'=>$meta_key
                    )]);
            $sql=$q->get_sql('post', $wpdb->posts, 'ID', $this);
            
            $this->assertRegexp("/{$wpdb->postmeta}\.meta_key LIKE %{$wpdb->esc_like($meta_key)}%/", $sql['where']);
            
        }

#9 @dd32
6 years ago

#42653 was marked as a duplicate.

#10 @DrewAPicture
6 years ago

  • Owner set to mariovalney
  • Status changed from new to assigned

Assigning to mark the good-first-bug as "claimed".

@boonebgorges Any chance you could review the patch? :-)

#11 @DrewAPicture
6 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

@boonebgorges
6 years ago

#12 @boonebgorges
6 years ago

  • Component changed from Database to Query
  • Keywords 2nd-opinion added

42409.3.diff cleans up documentation and adds unit tests that demonstrate compare_key and meta_compare_key behavior. Unfortunately, the test in 42409.test.diff doesn't work after recent changes to the way that SQL queries are generate by $wpdb.

I agree with the comments above that we ought to keep the behavior consistent with LIKE for values. It's unfortunately that it's poorly named, but it'd be worse to have it inconsistent between keys and values.

I agree with @kokers that we should add support for other compare operators if we're going to introduce compare_key. Again, this is for consistency with compare (also, why not?). An updated version of 42409.3.diff with less hardcoded logic regarding LIKE / = would be welcome. If not, we can handle this additional enhancement as part of a separate ticket.

More general wildcard support - ie, proper "LIKE" behavior with % - will probably have to be produced with a new operator name. Unfortunately it can't be "LIKE". "RAW_LIKE" is clunky but I'm having a hard time coming up with a better option. (The confusing thing here is that the upper-case convention implies that it corresponds to an actual MySQL operator, which RAW_LIKE does not.) Perhaps it'd be more semantically correct to add a parameter, something like:

'meta_query' => array(
    array(
        'key' => 'foo\_%\_bar',
        'compare_key' => 'LIKE',
        'escape_wildcards_key' => false,
    ),

    // For values
    array(
        'key' => 'foo',
        'value' => 'foo%bar',
        'compare' => 'LIKE',
        'escape_wildcards' => false,
    ),
),

escape_wildcards and escape_wildcards_key would default to true, for compatibility.

I am loath to introduce another parameter, but this feels more accurate to me than something like RAW_LIKE.

Two questions:

  1. What do others think of this suggested syntax?
  2. Is there any reason to hold off on the improvement in 42409.3.diff before making a decision about wildcard support?

2 is especially important because it's likely that "true" LIKE support is going to be very difficult (if even possible) to implement from a security point of view, so I don't want it to hold up the much more straightforward improvement being suggested in 42409.3.diff.

Last edited 6 years ago by boonebgorges (previous) (diff)

#13 @mariovalney
6 years ago

Thanks for the cleanup, @boonebgorges !

About the "escape_wildcards_key" I've been thinking about the alternative using a filter to avoid a new field. But I'm concerned about a misuse to break all LIKE WP_Queries and/or being a lot verbose to use it for a unique WP_Query call.

About the others operators ('NOT LIKE','IN', 'NOT IN','EXISTS', 'NOT EXISTS') I guess we should work in another ticket as enhancement we should talk about the use cases. For example, if we do:

<?php
'meta_query' => array(
    array(
        'key'           => 'foo',
        'compare_key'   => 'NOT EXISTS',
    ),
),

Will return all posts, if we just add the parameter "as it's" (in meta_value cases). Because every post has at least one [core] meta_key that it's different from "foo".

To create a way to consult if the meta_key foo is not presented we should do a little more code.

(I guess this last point helps to answer the second question: no reason to hold it - for another operators and for wildcard support.)

Last edited 6 years ago by mariovalney (previous) (diff)

#14 @boonebgorges
6 years ago

  • Milestone changed from Awaiting Review to 5.0
  • Owner changed from mariovalney to boonebgorges

@mariovalney Thank you for chiming in. Agreed that a filter-based approach doesn't feel right (for "true" LIKE behavior, that is), and you make a very good point about custom operators. We should have separate tickets for each of these different projects. Let me clear this one out first, and I'll open those up for discussion.

#15 @boonebgorges
6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 42768:

Allow LIKE queries against the 'key' value in meta queries.

The new compare_key=LIKE parameter works in conjunction with key in a
similar way to the compare=LIKE and value: by doing a "compares" LIKE
query. This allows developers to do partial matches against keys when
doing meta queries.

Props mariovalney, chasewg.
Fixes #42409.

#16 @boonebgorges
6 years ago

Two new tickets for interested parties:
#43445 - to discuss "true" LIKE queries (ie those that respect wildcards)
#43446 - to discuss more possible values for the meta_compare_key parameter

#17 @mariovalney
6 years ago

I would like to add to discussion the #42082 - Support compare custom fields values (using both meta_keys)

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


6 years ago

#19 @johnbillion
6 years ago

  • Keywords needs-patch added; good-first-bug has-patch 2nd-opinion removed
  • Milestone changed from 5.0 to 5.1
  • Resolution fixed deleted
  • Status changed from closed to reopened

@since needs updating

#20 @desrosj
5 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 44518:

Docs: Update since annotations for adding LIKE comparisons with meta keys.

Previously introduced in [42768].

Fixes #42409.

#21 @desrosj
5 years ago

  • Keywords needs-patch removed

#23 @mariovalney
5 years ago

  • Keywords needs-codex added

How about docs in WP_Query page?

Needs work? How can I help?

This ticket was mentioned in Slack in #docs by mariovalney. View the logs.


5 years ago

#25 @styledev
5 years ago

What happened to allowing % wildcards inside the value? This would help with searching serialized meta_values.

#26 @Otto42
5 years ago

Dunno, but you should not store serialized values as meta anyway. The meta functions have the ability to handle multiple values with the same key, and will store these values properly, as separate rows. Meta keys do not have to be unique per post in the database. See the documentation for the meta handling functions.

#27 @styledev
5 years ago

@Otto42 Advanced Custom Fields stores data in serialized arrays. This is a major plugin used on 1+ million plus websites (according to their site) and thus we are past "you should not store serialized values as meta anyway". That aside, MySQL handles wildcard percentages in the middle of a LIKE string and thus so should WordPress. If we need to escape a percentage then we (the developer) should be responsible for doing so and it shouldn't be done automatically on our behalf.

Note: See TracTickets for help on using tickets.