Filtering get_meta_sql for advanced queries

[sc0ttkclark]

Currently in the code for 3.1 (trunk) there are no filters running on the new function _wp_meta_sql which would be useful for plugins to perform more complex MySQL functionality on specific columns.

I suggest adding a filter ;)

To the tune of my last ticket - it checks if the value returned is null, if it's not null then it will return the value, otherwise it will continue on like normal.

DOWNSIDE: The downside to the filter running as-is right now is that this function is used by both get_users and get_posts (down the code pipeline) and get_posts >> WP_Query::query has a 'suppress_filters' option which when set to true skips over these types of filters. This being inside of _wp_meta_sql with no way to get at that 'suppress_filters' flag. Does it make sense to account for $q and $qv or alternatively &$this in the _wp_meta_sql function for checking that flag?

[sc0ttkclark]

Initial Patch including the new filter

[sc0ttkclark]

Also, my reference to my last ticket was meant to include #14766

[scribu]

I was thinking it would be more useful if you were able to modify $wp_query->meta_query, before it was sent to _wp_meta_sql().

This can be achieved by moving it up in parse_query() and then plugins could use existing filters, like 'parse_query' and 'pre_get_posts'.

[sc0ttkclark]

That would also be useful, as long as these filters remain to. The filters I'm suggesting would allow a plugin to intercept the meta info and return a more advanced query allowing a themed to continue using core theming functions vs using custom wpdb queries. One example is casting the meta value into a datetime and other various operations. Simply modifying the meta_query in the beginning wouldn't accomplish that. I'm not against your suggestion of adding that though.

[sc0ttkclark]

too* and themer*

Sorry just woke up and am posting from my iPhone.

[sc0ttkclark]

I must also note that these filters would make it possible for the Pods plugin to integrate with core where it currently cannot. Pods 2.0 will allow devs to choose the storage models on ther WP Object custom fields. In this case, WP Users and WP Post Types would be covered at the same time by adding these filters. That is just in addition to allowing other plugins that add custom meta boxes and fields to Post Types to be able to control it's fields and how it is compared or where the data is stored.

[sc0ttkclark]

Updated to reflect new name / location of function in 3.1

[sc0ttkclark]

Function name / location changed in 3.1, refreshed with a patch!

[scribu]

I think that in this case, the filter should be at the end, passing the normal string as the first argument.

[sc0ttkclark]

I have no objections. Will update patch tonight.

[jane]

Freeze in November 1, so try to get the patch up asap so that if there's new dev feedback that needs to be addressed, it can be done before this deadline for new enhancements.

[sc0ttkclark]

Thanks for your notice Jane!! Really appreciated, working on after this reply gets sent!

[sc0ttkclark]

Moved filter to end (simplifying in the process)

[sc0ttkclark]

Attached revised Patch which moved filter to end (simplifying in the process)

[scribu]

I've given some more thought to this and I don't think a filter would be appropriate here.

get_meta_sql() is supposed to be a protected method, used internally by classes that extend WP_Object_Query. If we add a filter, we will be forced to keep the method as is.

Besides, there will be other filters applied on the clauses later.

[sc0ttkclark]

More thought is welcome - however the filters later are filters later. What problems would the filters cause under protected method?

[scribu]

I'm wondering, wouldn't a filter on get_tax_sql() be needed also?

[sc0ttkclark]

I didn't notice that had been created, from the time I opened this ticket up to this point a bunch of reworking has been done regarding the organization of these functions. I'll take a look at that, but if you're looking at it from my point of view, yes that'd be necessary too.

[sc0ttkclark]

It makes most sense to start new tickets for get_tax_sql so will do that.

[scribu]

Related: #15249, #15250

[scribu]

(In [16106]) Add 'get_meta_sql' filter. Props sc0ttkclark for initial patch. Fixes #15018

[sc0ttkclark]

Standardizing implementation to match get_tax_sql and get_search_sql (implementation in specific location, for access to $this)

[sc0ttkclark]

Standardizing implementation to match get_tax_sql and get_search_sql (implementation in specific location, for access to $this)

[sc0ttkclark]

To standardize implementation to match get_tax_sql #15249 and get_search_sql #15250 (implementation in specific location, for access to $this) - I recommend we make those same modifications to this code as well.

Also, I submitted the wrong patch a moment ago (twice actually), I updated it (15018.3.patch should be used)

[scribu]

$this is no longer available. See [16266]

[sc0ttkclark]

So there's no way to pass $this into the function as an additional argument purely for filter sake?

For example:

$clauses = get_meta_sql( $qmeta_query?, 'post', $wpdb->posts, 'ID', &$this );

[sc0ttkclark]

Oops, example got eaten up!

$clauses = get_meta_sql( $q['meta_query'], 'post', $wpdb->posts, 'ID', &$this );

[westi]

A patch to add a context param to get_meta_sql and to use apply_filters_ref_array instead would be a good idea IMHO.

That way plugins can know which object is actually having it's query filtered otherwise it gets message.

[scribu]

That way plugins can know which object is actually having it's query filtered otherwise it gets message.

They already get the $meta_type as a parameter. Also, they can modify the 'meta_query' var before get_meta_sql() is called.

Also, what if I want to use get_meta_sql() in a plugin, but I don't have a $this to pass?

[sc0ttkclark]

Well, since it's gone this direction as a global function vs a class function, I guess it makes sense to not require the &$this but make it optional? Otherwise, I'm okay with leaving the modification out, it would just give more information to be able to be used on the filter in question.

[westi]

You still need the context if call from WP_Query else the function is next to useless as you can't target any extra query variables you may have added - this is why apply_filters_ref_array exists os that we can pass objects as context.

In the case of use in global scope without an object as context we should be able to cope with being passes either null or just a new Object().

[sc0ttkclark]

Thx @westi - Glad I'm not crazy alone here ;)

[scribu]

You still need the context if call from WP_Query else the function is next to useless as you can't target any extra query variables you may have added

Could you provide an example where you would need access to the other query variables from the 'get_meta_sql' filter?

[scribu]

(In [16286]) Add context to get_meta_sql(). See #15018

[nacin]

Is there an issue with optionally passing null by reference if context is not supplied?

[scribu]

$context is a variable, so it shouldn't matter if it's null or not.

[onokazu]

WP_Term_Query should also pass itself to WP_Meta_Query::get_sql() so that it can be accessed through the get_meta_sql filter.

wp-includes/class-wp-term-query.php: 567

$mq_sql = $this->meta_query->get_sql( 'term', 't', 'term_id' );

needs to be changed to:

$mq_sql = $this->meta_query->get_sql( 'term', 't', 'term_id', $this );