Support complex (nested, multiple relation) queries in WP_Meta_Query

[boonebgorges]

The syntax of WP_Meta_Query currently limits the kind of queries you can use it for. It's only possible to use a single 'relation', which means that you can do a string of clauses connected by OR or AND. But you can't group these clauses together and use a mix of relations, which makes it impossible to do something like, say:

SELECT posts WHERE (
    ( foo = bar AND foo1 = bar )
    OR
    ( foo = baz AND foo1 = baz )
    OR
    foo = barry
)

I propose a restructuring of WP_Meta_Query so that it accepts arbitrary nested queries. For example, the pseudocode above would be represented as a meta_query argument that looks like this:

array(
    'relation' => 'OR',
    array(
        'relation' => 'AND',
        array(
            'key' => 'foo',
            'value' => 'bar',
        ),
        array(
            'key' => 'foo1',
            'value' => 'bar',
        ),
    ),
    array(
        'relation' => 'AND',
        array(
            'key' => 'foo',
            'value' => 'baz',
        ),
        array(
            'key' => 'foo1',
            'value' => 'baz',
        ),
    ),
    array(
        'key' => 'foo',
        'value' => 'barry',
    ),
),

The attached patch is a first pass at this restructuring. A few notes:

• Syntax for current queries is unchanged, as is the SQL that's generated. This means that backward compatibility should be maintained in all cases (though see below for some very edge-case questions). I've also included all optimizations that have been baked into WP_Meta_Query over the years (such as #19729).

• The attached patch contains simple unit tests for the new functionality. All existing unit tests are also passing. The extended unit test suite for WP_Meta_Query is also fully passing (#29560), with the exception of a couple very minor syntax quibbles that should not reflect any changes in functionality.

• One fairly significant change in the internal SQL-building logic is that I've broken EXISTS queries out into their own block. Currently, EXISTS falls back on keyless '=' queries in a pretty opaque way; my change does not change the SQL that results, but I think it makes the code much clearer.

• I have changed some of the codestyling of the SQL generated by the class. I've normalized the indentation so that the nesting is clear when debugging, eg

WHERE (
    ( meta_key = 'foo' AND meta_value = 'bar' )
    OR
    ( meta_key = 'foo1' AND meta_value = 'bar' )
)

I also normalized the spaces around the parentheses in the SQL being generated. It's possible that these minor codestyling changes will affect plugins that are performing certain kinds of string operations on the 'get_meta_sql' filter (think - explode( "\n", $sql['where'] ) ). I can roll back these formatting changes to eliminate the possibility of issues with these sorts of plugins, but I figured that the increase in usability might be worth the tradeoff in this case.

See #20312 for a related request that was closed as wontfix.

Feedback welcome.

[boonebgorges]

Side note - If people like this syntax, it's easy to imagine it being used in WP_Tax_Query and WP_Date_Query as well, with some of the recursive logic broken out into a shared abstract class. Might be a next step if others like what I've proposed here (and after those classes have full test coverage!).

[boonebgorges]

See also #24093 and patches there. This restructuring would be an opportunity to address the inefficiency of JOINing when clauses use the same meta key.

[boonebgorges]

29642.3.patch is a refresh that reflects the unit tests committed in r29799, along with some documentation fixes.

The main change in 3.patch is that I've broken the recursive logic into an abstract class WP_Recursive_Query, which WP_Meta_Query now extends. This exact recursive logic will be shared by WP_Tax_Query (#29738) and WP_Date_Query (no ticket yet, but I've got a patch almost done). Use of WP_Recursive_Query in a nutshell:

1. Your extending class is responsible for its own sanizitation/regularization of the params passed to the constructor. See WP_Meta_Query::sanitize_query(). It would be nice to provide some help in WP_Recursive_Query for this sanitization, but the requirements of the extending classes are just too different. The sanitized params should then be set to $this->queries in your constructor.

2. Your extending class should contain a public function get_sql(), which is responsible for accepting whatever params are required for your query logic, setting them to class properties as necessary, calling $this->get_sql_clauses() to start the recursion logic, manipulating the clauses returned as required (WP_Meta_Query just passes them through a filter for backward compatibility), and then returns them. The param and return value syntax of get_sql() in Meta, Tax, and Date are all different, so there's no way to provide this functionality in a shared base class (even as an abstract method - they take different default arguments, which violates strict standards).

3. Your extending class must contain a protected function is_first_order_clause( $query ), which should return true when $query is at the tip of a recursive branch, and protected function get_sql_for_clause( $clause, $parent_query ), which does all the specific SQL-building logic for your clause.

I've got the abstraction for Meta, Tax, and Date working locally. Obviously, they all depend on having WP_Recursive_Query, so I've opted to post the WP_Meta_Query version here first - mainly because Meta is, in a couple ways, the most complex case.

I'm looking for feedback on this proposed architecture. Splitting the recursive logic out like I've done is the DRY thing to do, and it paves the way for building future SQL-generating classes like this - I have a few in mind already ;). At the same time, I'm sensitive to the concern that we build APIs that are flexible and do not place overly rigorous demands on developers, so I've tried to be modest in the requirements placed on extending classes (eg with regard to shared param sanitization).

[boonebgorges]

29642.unified.patch​ is a single patch that contains the proposed WP_Recursive_Query plus the necessary mods to Meta, Date, and Tax queries, as requested ​http://irclogs.wordpress.org/chanlog.php?channel=wordpress-dev&day=2014-10-08&sort=asc#m937034. I've left out the unit tests to make it easier to read.

[boonebgorges]

After some thought and some discussion with nacin in IRC, I've decided to step back from the WP_Recursive_Query proposal for the time being. WP_Meta_Query will remain standalone for now.

29642.4.patch​ updates 3.patch with:

• Improved organization (to better match the other classes)
• Improved inline documentation

Before pulling the trigger on this, I'd like to have some feedback from DrewAPicture on the documentation aspect. It's a bit tricky to capture the nested nature of the params given our hash standard. I chose to do a few things:

1. Move the parameter documentation to the __construct() method
2. In the hash format, describe a "first order" clause, but then make reference to the fact that you can also pass "fully-formed" meta queries as parameters.
3. In the docblock for the class, I go into a little more detail and give a brief example for illustration.
4. I fixed up (I think) the parameter default documentation, using the format:

@type type $foo Description description description. Default: 'bar'. (bar|baz|barry)

[DrewAPicture]

29642.5.patch​ makes some adjustments to the inline docs in various places. Notable changes include:

• Replacing the piped accepted values with a comma-delimited list
• Removed the large block of explanatory text from the class block in favor of including that information directly in the reference article
• Add missing @since tags for new functionality and properties
• Add hash descriptions to array return blocks
• Other minor tweaks.

[DrewAPicture]

Docs adjustments

[DrewAPicture]

docs typo

[boonebgorges]

In 29887:

Introduce support for nested queries in WP_Meta_Query.

Previously, meta query arguments could be joined by a single AND or OR relation.
Now, these queries can be arbitrarily nested, allowing clauses to be linked
together with multiple relations.

Adds unit tests for the new nesting syntax. Modifies a few existing unit tests
that were overly specific for the old SQL syntax. Backward compatibility with
existing syntax is fully maintained.

Props boonebgorges, DrewAPicture.
See #29642.

[jorbin]

This is causing the Tests_Meta_Query::test_invalid_query_clauses test to fail on php 5.2 and 5.3

see: ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/37875073 and ​https://travis-ci.org/aaronjorbin/develop.wordpress/jobs/37875072

The above patch fixes this. The issue comes down to the fact that in php changed isset in 5.4 to cause "Checking non-numeric offsets of strings now returns FALSE." see ​http://php.net/manual/en/function.isset.php

We thus need to explicitly exclude non arrays.

[boonebgorges]

Thanks, jorbin - I'd noticed this too, but was too lazy to fix it :)

[boonebgorges]

In 29941:

Bail from cleaning meta query clause when it's not an array.

Later isset() checks on string values were causing notices on PHP < 5.4.

Props jorbin.
See #29642.

[DrewAPicture]

#29876 was marked as a duplicate.