Filtering get_tax_sql for advanced queries

[sc0ttkclark]

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

I suggest adding a filter ;)

[sc0ttkclark]

At the moment, it appears this function has disappeared from trunk for whatever reason. It's no longer in /wp-includes/classes.php so I'll keep an eye out for where it's being moved to before patching.

[scribu]

WP_Object_Query has been moved to wp-includes/class-wp-object-query.php

[sc0ttkclark]

Related: #15018, #15249

[sc0ttkclark]

Initial Patch including the new filter

[sc0ttkclark]

Added patch that includes the new filter in query.php (not within get_tax_sql function itself due to the lack of information the function is given like no access to $this)

[scribu]

Since get_tax_sql() is a method of WP_Object_Query, which is the parent class, you do have access to $this.

[scribu]

Why wouldn't passing $tax_query and $object_id_column to the filter be enough?

[sc0ttkclark]

If it has access to $this, then that would be fine within the function. It was just a misunderstanding on my side.

[scribu]

'get_tax_sql' filter with context

[scribu]

context.15250.patch applies the 'get_tax_sql' filter, passing a context too.

I'm still concerned about what would happen if we decide we also need to alter the JOIN clause, for example.

[scribu]

Hm... well now with [16414] the terms are first transformed into term_taxonomy_ids, which I guess would need additional handling.

[ryan]

Perhaps we should see how things go in 3.1 before exposing the guts of get_tax_sql().

[scribu]

Agreed.

[sc0ttkclark]

Why would we push this to another 3.x release if it's dependent on code that could change in 3.1, can't we just patch / implement once those dependencies are set in stone?

[scribu]

Which dependencies?

Also, no code is set in stone.

[sc0ttkclark]

Dependencies would be whatever happens to get_tax_sql that @ryan brought up concerns with.

[scribu]

I think what ryan was trying to say is that we should wait to see how people use get_tax_sql() after 3.1 is released, before adding the filter.

[sc0ttkclark]

I see that, but I also believe that losing out on implementing a filter out the gate means waiting a whole new dev cycle to be able to do things with core. The proposed filter applies to many needs by giving quite a bit of information to a function that wishes to extend the SQL generated by the function.

[scribu]

Please take another look at the code inside get_tax_sql(). It's quite different from what you made the patch against.

If you can give me an example that can efficiently leverage the filter you're proposing, i.e. without wasting queries made by _transform_terms(), I will commit it.

[sc0ttkclark]

Will do, the patch was before a lot of things happened so I'll update today.

[sc0ttkclark]

Updated patch for latest SVN Revision (16692)

[sc0ttkclark]

Refreshing this patch w/ 3.5 trunk, updated filter args with ones now used

[sc0ttkclark]

Refreshed this patch with the latest code from 3.5 trunk

[scribu]

Using apply_filters_ref_array() is not necessary anymore, since we dropped support for PHP 4. Can just use apply_filters().

[sc0ttkclark]

Changing to use of apply_filters

[wonderboymusic]

Refreshed - I am less in love with this one, but needs filter docs, nonetheless. We can at least make a decision one way or another.

[DrewAPicture]

Filter docs

[DrewAPicture]

15250.5.patch​ adds filter docs.

[helen]

Pretty late for this one for 4.0, would prefer for this to really get tested and soak. No consensus either.

[boonebgorges]

I'm strongly against adding filters directly to SQL queries. It's terrible for future backward compatibility, and it's an invitation for plugins to do things that end up causing conflicts with other plugins. (If we could remove the filters in Date and Meta queries without breaking existing plugins, I would.) The changes to WP_Tax_Query in WP 4.1 make it so that pretty much any kind of tax query can be expressed in the syntax of the class, reducing the need for this filter; and anyone who *really* needs to filter SQL for some reason can still use filters like 'posts_where' in WP_Query.

In the absence of a compelling use case, I'm going to say wontfix.