#31045 closed feature request (fixed)
Ordering a Query Using Multiple Meta Keys
Reported by: | Funkatronic | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | 4.2 | Priority: | normal |
Severity: | normal | Version: | 4.2 |
Component: | Query | Keywords: | has-patch 2nd-opinion |
Focuses: | Cc: |
Description
Currently, WordPress doesn't support sorting a query by multiple meta keys; we can only sort by one, even though we can now query posts by multiple meta entries. My idea is that for meta queries, we allow the coder querying to assign the table aliases in the query arrays. Either by adding a new argument:
$args = array( 'post_type' => 'product', 'meta_query' => array( array( 'key' => 'color', 'value' => 'blue', 'compare' => 'NOT LIKE', 'alias' => 'color' ), ), 'orderby' => 'meta-color' //prefix to prevent any collisiona ); $query = new WP_Query( $args );
Or making array indexes the alias:
$args = array( 'post_type' => 'product', 'meta_query' => array( 'color' => array( 'key' => 'color', 'value' => 'blue', 'compare' => 'NOT LIKE', ), ), 'orderby' => 'meta-color' //prefix to prevent any collisiona ); $query = new WP_Query( $args );
Or just use the meta key as the alias:
$args = array( 'post_type' => 'product', 'meta_query' => array( array( 'key' => 'color', 'value' => 'blue', 'compare' => 'NOT LIKE', ), ), 'orderby' => 'meta-color' //prefix to prevent any collisiona ); $query = new WP_Query( $args );
In either case, the query would denote the alias used for the table in the mySQL query, then you can specify in the orderby clause which ones you want to sort by, allowing for multiple if the need be.
Attachments (7)
Change History (39)
#2
@
10 years ago
- Keywords needs-patch added
- Milestone changed from Awaiting Review to Future Release
#3
@
10 years ago
I have diff on applying custom aliases for meta queries. Next step is figuring out the syntax for using them in an orderby
. Any ideas, @boonebgorges?
#4
@
10 years ago
- Keywords needs-unit-tests added
Cool, thanks for the initial patch. For syntax, we should tap into the existing WP_Query orderby code. Check out parse_orderby()
https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/query.php#L2207. It gets a "primary_meta_key" from the meta query and checks it against the keys passed to orderby. We need to keep some of this logic, because orderby=meta_value
will continue to work this way. But for the more general case, I suppose we'll have to loop through the meta query clauses and compile a list of available aliases.
#5
@
10 years ago
So, 'orderby' => 'alias'
?
Also, a WP_Meta_Query
object stores its aliases in an array(WP_Meta_Query->table_aliases
), so we can use that to keep track of aliases. Still studying the logic so I can code something that doesn't break anything.
#6
@
10 years ago
I'm thinking:
$q = new WP_Query( array( 'meta_query' => array( array( 'key' => 'foo', 'value' => 'foo value', 'alias' => 'foo', ), array( 'key' => 'bar', 'value' => 'bar value', 'alias' => 'bar123', ), ), 'orderby' => array( 'foo' => 'ASC', 'bar123' => 'DESC', ), ) );
I believe the 'table_aliases' array is a flat array that's mainly used for counts and incrementing. It doesn't store any associations with the query clauses. I'm not sure if that'll be sufficient - needs testing.
#7
follow-up:
↓ 8
@
10 years ago
My idea to change parse_orderby
:
- Get all aliases, both custom and ones created by
WP_Meta_query
, in query - Check if
orderby
matches any of the aliases - If it matches, create an
ORDER BY
clause that uses that alias:alias.meta_value
You don't have to know what the related join clauses contain, just as long as you know the alias thats being used for the join, you can use it for the ORDER BY
, at least imho.
As far as I can tell, table_aliases
keeps track of all the aliases used, though like you said, we need to test to make sure we cover all bases. Also, does WP have any sanitation functions for table aliases?
#8
in reply to:
↑ 7
@
10 years ago
Replying to Funkatronic:
My idea to change
parse_orderby
:
- Get all aliases, both custom and ones created by
WP_Meta_query
, in query- Check if
orderby
matches any of the aliases- If it matches, create an
ORDER BY
clause that uses that alias:alias.meta_value
You don't have to know what the related join clauses contain, just as long as you know the alias thats being used for the join, you can use it for the
ORDER BY
, at least imho.
This all sounds good to me. You're probably right about not needing to know what the alias is associated with. I'd need to see the unit tests :)
As far as I can tell,
table_aliases
keeps track of all the aliases used, though like you said, we need to test to make sure we cover all bases. Also, does WP have any sanitation functions for table aliases?
Use sanitize_key()
. It's not specific to table aliases or even to MySQL, but it's the most restrictive of our sanitizers, and it's what register_post_type()
uses for post type strings, etc.
#9
@
10 years ago
And as I write the code, I realize you would need to know if the meta_query is typed so you can cast the orderby. Coding wouldn't be coding without challenges.
#10
@
10 years ago
New crazy idea that I think might work:
- Every meta query clause gets aliased, even the main one. Any
orderby
call tometa_value
will default to the first one. - let
WP_Meta_Query
create theORDER BY
clauses and store them in an array where the aliases are the keys parse_orderby
looks for aliases; if it finds one, it pulls the right clause from the meta_query object. If it findsmeta_value
ormeta_value_num
, it calls the first/main clause.
It'll take more rewriting than Id like but I think its a sound way to go forward. It'll also take care of casting the orderby
properly. Also, lots of testing. Does this sound like a good idea?
#11
@
10 years ago
Every meta query clause gets aliased, even the main one. Any orderby call to meta_value will default to the first one.
Yes. We pretty much already do this; see https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/meta.php#L1353. It makes sense to me to then fill in the 'alias' key for each clause with the calculated value.
let WP_Meta_Query create the ORDER BY clauses and store them in an array where the aliases are the keys
I hesitated at first, but on reflection I think it's a good idea for WP_Meta_Query
to do some of this work. That way, it'll be possible to do the same orderby magic in other places where meta queries are used (WP_Comment_Query
, WP_User_Query
). However, I'd rather not attempt to build partial SQL statements and pass them around. Here's an alternative thought: WP_Tax_Query
builds a flat list of "queried_terms" as it parses the arguments passed to the class. https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/taxonomy.php#L774 What we did something similar in WP_Meta_Query
? This flat array would contain aliases (and perhaps even be keyed by alias, though I don't think it matters too much), and could be used in WP_Query
to look up the necessary cast for the meta value. I think this is somewhat better than assembling the orderby clauses in WP_Meta_Query
itself, for two reasons: (1) it's not great practice to pass around partial SQL chunks between classes (my suggestion would pass a structured array instead, and leave it up to the "client" - WP_Query
- to build the SQL), and (2) my suggestion would leave the door open for different kinds of custom query manipulation than just ORDER BY, since all the clause data would be readily available in a flat array.
parse_orderby looks for aliases; if it finds one, it pulls the right clause from the meta_query object. If it finds meta_value or meta_value_num, it calls the first/main clause.
+1
Does this make enough sense to do a proof of concept?
#13
@
10 years ago
- Keywords has-patch added; needs-patch needs-unit-tests removed
Thanks for these patches. I started working on your patch last night, and it wasn't working properly for me - but maybe this was fixed by your version 2 :)
In any case, in the process of playing with table aliases, I started to have second thoughts. The way table aliases are currently built in WP_Meta_Query
is like this: the first JOINed table is aliased as $wpdb->postmeta
, and subsequent tables as mt1
, mt2
, etc. That first "unaliased" JOIN is then referenced in a number of places in WP_Query
, and my guess is that there are plugins that are depending on it as well. For this reason, I'm hesitant to change table aliases. And, notwithstanding my earlier comments, it's not really necessary for what we want to accomplish here anyway.
31045.diff takes a slightly different tack. Aliases are built in the same way as before. Meta queries now accept a 'name'
parameter. The meta query object now has a method get_clauses()
which returns a flattened array of query clauses. This array is keyed by 'name', and contains 'cast' values, which are then used by WP_Query
to build the ORDER BY clause. See the unit tests for examples.
What do you think?
#14
follow-up:
↓ 15
@
10 years ago
Looks good. Where can I find the unit test file?
Also, glad we can collaborate on this. I'm learning a bit about team coding.
#15
in reply to:
↑ 14
@
10 years ago
Replying to Funkatronic:
Looks good. Where can I find the unit test file?
Also, glad we can collaborate on this. I'm learning a bit about team coding.
Happy to collaborate!
The unit tests are at the bottom of 31045.diff.
#16
@
10 years ago
I meant the class those functions are in. Are they part of the WP download anywhere? I've never used the official unit test files (if there are any)
#17
@
10 years ago
Ah. Here's where the PHPUnit tests live in trun: http://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/ Get a checkout of develop.svn.wordpress.org or develop.git.wordpress.org to see them - they don't come with the regular build download. Check out https://make.wordpress.org/core/handbook/ - especially https://make.wordpress.org/core/handbook/automated-testing/
#18
@
10 years ago
Alright, fixed a bug I made in my second to last patch that you transferred into yours. I haven't run "official" unit tests yet, but I ran a query and output the resulting mySQL code and looks like it checks out.
#20
@
10 years ago
- Milestone changed from Future Release to 4.2
- Owner set to boonebgorges
- Status changed from new to reviewing
Thanks for your continued work on this, Funkatronic. It's funny - your "lapse in coding judgment" actually worked, because the array_key_exists()
check would resolve to true or false as appropriate, meaning that the previous logic actually worked, though of course it was not straightforward and perhaps accidental :)
Regarding the 'parse_orderby' filter: I'm going to remove it, as per our informal policy of not adding filters to SQL fragments unless they're absolutely necessary. Anyway, there is already a filter 'posts_orderby' that does essentially the same thing.
I'm going to do a bit more documentation cleanup and review, and then I think we're good to go with this. Thanks for the ticket and for the help in getting this enhancement ready to go.
#22
@
10 years ago
- Resolution fixed deleted
- Status changed from closed to reopened
I'd like to take a little more time to consider the implementation here. I really like the concept of the idea, I'm just not sold on the name
argument. I think it has the possibility of introducing confusion -- especially since we're effectively "aliasing" a key/value clause so we can reference it later in the orderby
argument.
Going from name
to referencing it in orderby
isn't obvious to me.
#23
@
10 years ago
Maybe id
or meta_id
? We were using alias before we decided not to mess with the mysql aliasing directly
#24
@
10 years ago
- Keywords needs-patch added; has-patch removed
After talking to @boonebgorges in Slack, my best suggestion is that we go with clause_id
for the argument name. I share his concerns about using the word "alias" in the argument name because of the implication that it has any effect on the actual alias used when building the query.
The purpose of the discussion is to -- as simply as possible -- draw a line between setting the value in the clause and referencing it in the orderby. By avoiding the use of the word "orderby" in the new argument name, we also don't lock ourselves into that vernacular should we find another good use for this argument in the future.
I'd also recommend updating the docs for the orderby
argument in WP_Query
.
#25
@
10 years ago
@DrewAPicture: one of my original ideas was to have the id just be the key of that particular clause:
$args = array( 'post_type' => 'product', 'meta_query' => array( 'post_color' => array( 'key' => 'color', 'value' => 'blue', 'compare' => 'NOT LIKE', ), ), 'orderby' => 'post_color' ); $query = new WP_Query( $args );
Does that make more sense as a syntax alternative?
#26
@
10 years ago
Just to bring the discussion from Slack back here to the ticket: I think Funkatronic's suggestion of using array indexes as orderby aliases is the most elegant solution to the problem. The main reason why I shied away from it when he suggested it in an earlier patch is because it doesn't seem WordPressy. I can't think of any other API where devs can pass arbitrary array indexes and have them interpreted in a meaningful way. The other consideration was that we do (or at least at one time we did) at times refer to the 0th item in meta_query
, a reference that would break if we allowed associative keys. We can fix our own references, but it's possible that some plugins are doing the same thing, which introduces the possibility of breakage (however remote that may be).
All of this being said, I'd be glad to go with the suggestion, as I think it's the most natural solution. So let's do it. We'll need a patch that updates the parsing logic, the unit tests, and the inline docs. We'll also need to make sure that we're not referencing the 0th member of the meta query anywhere - we should be using reset()
.
#27
@
10 years ago
- Keywords has-patch 2nd-opinion added; needs-patch removed
31045.2.diff changes the functionality so that the syntax works like Funkatronic suggests here: https://core.trac.wordpress.org/ticket/31045#comment:25
It's a pretty straightforward patch, but I'm struggling with the documentation. I've tried my best to describe the feature in the inline docs, but I'm afraid it's still pretty unclear. It's the kind of thing that's pretty obvious if you can see an example, but is hard to explain without getting jargony. I have a very slight suspicion that my difficulty explaining the feature suggests that maybe it's not the best implementation, but I'd be happy to be shown wrong on this point.
DrewAPicture and Funkatronic, would you mind having a look at the patch to see if the documentation changes are sufficient to explain what's going on?
#30
@
9 years ago
Hey there,
Has this feature been scratched or changed for 4.3/4.4?
Seeing a lot of reports of people having problems with it, and I am too. Just wanted to check before I possibly open a new ticket.
A big +1. I've had the same idea in the past; see #29447 and point 4 in https://core.trac.wordpress.org/ticket/25538#comment:26.
Adding an 'alias' argument to meta_query clauses seems like the most consistent way forward.
WP_Query
already supports multiple ORDER BY clauses (see https://make.wordpress.org/core/2014/08/29/a-more-powerful-order-by-in-wordpress-4-0/ and #17065), so the 'orderby' syntax would have to integrate with that. I would lean toward *not* using a'meta-'
prefix for aliases, since (a) the possibility of clashes is very slight, and (b) developers have full control over 'alias' and so it's not unreasonable to expect them to check for clashes themselves.Ideally, these aliases would also be used in building the SQL itself, which would make certain kinds of SQL filters easier to write. Aliases are currently generated here https://core.trac.wordpress.org/browser/tags/4.1/src/wp-includes/meta.php#L1353, and this would need a bit of reworking to ensure that ordering works properly with shared table aliases.