WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 11 months ago

Last modified 2 months ago

#17065 closed task (blessed) (fixed)

Independent ASC/DESC in multiple ORDER BY statement.

Reported by: ericmann Owned by: ericmann
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.1
Component: Query Keywords: has-patch
Focuses: Cc:

Description

WP_Query supports ordering results by multiple columns, but does not currently support independent ASC/DESC declarations on those columns. Instead, it concatenates the ORDER and ORDER BY statements together.

In some cases, it would be beneficial to independently set the order for these columns. An example offered on WP-Hackers:

I have a situation where I want to order a list of post both by author and date using WP_Query which isn't a problem since 'orderby' lets me do that. But I want the author part sorted ascending and date part sorted descending.

A normal MySql statement would end with ORDER BY author ASC, date DESC.

We should extend WP_Query to allow users to set their ordering parameters independently. This should be done in such a way as to not break backwards compatibility.

Attachments (8)

17065.diff (4.1 KB) - added by dd32 4 years ago.
24687.diff (1.5 KB) - added by andy 2 years ago.
17065.2.diff (6.1 KB) - added by wonderboymusic 12 months ago.
17065.3.diff (7.0 KB) - added by wonderboymusic 11 months ago.
17065.4.diff (7.1 KB) - added by wonderboymusic 11 months ago.
17065.5.diff (8.1 KB) - added by johnbillion 11 months ago.
17065.6.diff (8.2 KB) - added by DrewAPicture 11 months ago.
docs + coding standards
17065.7.diff (8.2 KB) - added by wonderboymusic 11 months ago.

Download all attachments as: .zip

Change History (85)

comment:1 @scribu4 years ago

Any syntax proposals?

comment:2 @ericmann4 years ago

Not at the moment. My first attempt allowed users to pass an array into 'orderby' rather than just a string. So:

$args = array(
    'post_type' => 'post'
    'orderby' => array(
        'author ASC',
        'date DESC'
    )
);

The advantage to this approach is that it wouldn't break backwards compat. Disadvantage is that is combines the behavior of 'orderby' and 'order' and could confuse people ... particularly if they use both parameters.

comment:3 @scribu4 years ago

That's a good start.

While thinking about this, we also have to consider meta queries: #15031

comment:4 @pento4 years ago

  • Cc gary@… added

c/p my syntax proposal from #15031. Allows for backwards compatibility, doesn't combine 'orderby' and 'order', allows for meta queries:

$args = array(
  'post_type => 'post',
  'orderby' => array(
    array(
      'meta_key' => 'bar',
      'order' => 'DESC',
    ),
    array(
      'key' => 'date',
      'order' => 'DESC',
    ),
    array(
      'meta_key' => 'foo',
      'order' => 'ASC',
    )
  )
)

The disadvantage is multi-level arrays, but we're already using them for 'tax_query'.

comment:5 @scribu4 years ago

I would replace 'key' with 'field', to avoid confusion with 'meta_key'.

comment:6 @linuxologos4 years ago

  • Cc linuxologos@… added

comment:7 @kucrut4 years ago

  • Cc kucrut added

comment:8 @maorb4 years ago

  • Cc maor@… added

comment:9 @dd324 years ago

An alternate syntax could be:

1. $args = array(
  'post_type => 'post',
  'orderby' => array(
    array(
      'meta_key' => 'bar',
      'order' => 'DESC',
    ),
    'date DESC'
  )
)

ie. Only use a multi dimension array when it's actually needed, Since we've currently got orderby as well as order, we could simplify it to (ie. abide by $order if set)

1b. $args = array(
  'post_type => 'post',
  'order' => 'DESC',
  'orderby' => array(
    array( 'meta_key' => 'bar' ),
    'date'
  )
)

The other obvious one, which I don't really like due to the "new" syntax of it:

2. $args = array(
  'post_type => 'post',
  'orderby' => array(
    'meta_key:bar DESC',
    'date DESC'
  )
)

And to drop a bombshell here, One thing that I've had to implement "often" when someone wants a custom ordering, is to float one value to the front of the list, such as:

3. $args = array(
  'post_type => 'post',
  'orderby' => array(
    array(
      'meta_key' => 'bar',
      'meta_value' => 'first',
      'order' => 'DESC', // Ie. "ORDER BY (meta_key_bar === 'first') DESC"
    ),
    'date DESC'
  )
)

This one is a step over what we currently have however, and can easily be achieved through existing filters, so whatever is added here, we'll need to ensure that extending upon it is possible.

I'll whip up a patch for number 1 above and a example of implementing 3 in a plugin..

comment:10 @scribu4 years ago

I don't really like 'date DESC'. I'm ok with 'date' instead of array( 'field' => 'date' ) though.

comment:11 @scribu4 years ago

And using the 'order' query var as a default makes sense too.

comment:12 @scribu4 years ago

  • Severity changed from trivial to normal

comment:13 @dd324 years ago

I don't really like 'date DESC'

Hm, I thought we already supported that, but appears not, So disregard that syntax. (I have however seen people attempting to use that syntax, thanks to the way WP_Query handles it, it'd just silently ignore the DESC it seems)

@dd324 years ago

comment:14 @dd324 years ago

  • Keywords has-patch added; needs-patch removed

attachment 17065.diff added

  • Implements multiple orderby's
  • Orders by any Queried Meta Key Value
  • Retains full back compat
  • Implemented 3. from above, See the final binary example below, for interests sake, thats lines 2371 & 2372 in the patch - To implement that in a plugin would require to filter each orderby clause individually, or to duplicate most of the block, or to perform black magic (regular expressions)
  • If no valid ordering is specified, it falls back to post_date as before
Usage:
(current usage, which still works)
orderby: 'date'
orderby: 'date' order: ASC
orderby: 'date title' order: DESC (Orders by date DESC, and title DESC)
orderby: 'MyMetaKey'

(New functionality)
order: ASC // If the order field is not specified by a item, this is used.
orderby: array( 'date' )
orderby: array( 'date', 'title' )
orderby: array(
            array(
               'field' => 'date',
               'order' => 'DESC'
            )
          );
orderby: array(
            array(
               'field' => 'date',
               'order' => 'DESC'
            ),
            array(
               'field' => 'title',
            )
          );
orderby: array(
            array(
               'meta_key' => 'MyMetaKey', // meta_key || meta_query MUST query for this key, else it's ignored
               'order' => 'DESC'
            ),
            array(
               'field' => 'title',
               'order' => 'ASC'
            )
          );
orderby: array(
            array(
               'meta_key' => 'MyMetaKey', // defaults to ordering by meta_value
            ),
            array(
               'meta_key' => 'MyMetaKey', // meta_key || meta_query MUST query for this key, else it's ignored
               'field' => 'meta_value_num', // Lets numerically order it
               'order' => 'DESC'
            )
          );

Binary Ordering:
orderby: array(
            array(
               'field' => 'ID',
               'value' => 10, // Binary ordering, This moves ID 10 to the front, or back of the set. Can be used with any field.
               'order' => 'DESC'
            )
          );
orderby: array(
            array(
               'field' => 'ID',
               'value' => 10, 
               'order' => 'DESC'
            ),
            'rand' // post ID 10 is at the start of the list, Remaining items are randomly sorted.
          );

comment:15 @dd324 years ago

  • Keywords needs-testing added

comment:16 @scribu4 years ago

Marked #9979 as duplicate.

comment:17 @lukecarbis4 years ago

My first trac comment - inspired by WordCamp GC. :)

Possible stupid question - why do we require the orderby meta_key to be part of the meta_query?

What about the usage case where I want to return results including but not limited to items that include the meta_key, and order by that meta_key. For example, I want to order posts by custom_meta_item, but I want to include posts that don't have any custom_meta_item.

This could be achieved by adding a second postmeta INNER JOIN to $pieces [ 'join' ], and using CASE to order in $pieces [ 'order' ] .

We could use the same syntax, but avoid ignoring orderby [ 0 ] [ 'meta_key' ] if it's not included in the meta_query.

Thoughts?

Last edited 4 years ago by lukecarbis (previous) (diff)

comment:18 @dd324 years ago

why do we require the orderby meta_key to be part of the meta_query?

Quite simply, because otherwise, it's not included in the JOIN's which are used. We could add an extra join for the meta_key that it's being ordered by, but with the move of the meta queries into a seperate function, that might be more difficult and dirty than it sounds.

inspired by WordCamp GC. :)

I'm glad it got at least one comment out of it, stick around and make another dozen yeah? :)

comment:19 @rosariobocca3 years ago

  • Cc rosariobocca added
  • Type changed from enhancement to feature request
Last edited 3 years ago by rosariobocca (previous) (diff)

comment:20 follow-up: @scribu3 years ago

rosariobocca, trac is for discussing code changes to WordPress Core.

For questions about how to do a particular thing using WordPress, please use the support forums:

http://wordpress.org/support/

If you tried that and got no reply, try posting to the wp-hackers mailing list.

In either case, here is not the best place to ask.

comment:21 in reply to: ↑ 20 @rosariobocca3 years ago

sorry.

Last edited 3 years ago by scribu (previous) (diff)

comment:22 @tomauger3 years ago

  • Cc tomaugerdotcom@… added

Really digging this new syntax. Great stuff.

comment:23 @ejdanderson3 years ago

  • Cc ejdanderson@… added

comment:24 @davecc3 years ago

dd32,

I'm using 3.3.1, and I really need to be able to sort by more than 1 custom field, has this been implemented? I can't find about this in the codex. Can I still implement your modifications? query.php has obvisouly changed over the past 11 months.

Thanks!

-Dave

comment:25 @tar.gz3 years ago

  • Cc code@… added

comment:26 @MikevHoenselaar3 years ago

Love to see this integrated as well. Apparently nobody needs this bad enough :-)

comment:27 @lukecarbis3 years ago

The new syntax looks great. Looking forward to seeing this happen as well.

comment:28 @nacin3 years ago

  • Keywords needs-unit-tests added
  • Type changed from feature request to enhancement

Might be nice to break this out into a new class, similar to WP_Meta_Query and WP_Tax_Query, especially for testing purposes.

For this to end up in core, we'll need two sets of tests: One, some unit tests that assert the resulting SQL based on various examples. Two, test how it works when integrated into WP_Query in terms of properly ordering posts. The integration tests should be based on a set of posts, similar to tests/query/results.php.

comment:29 follow-up: @scribu3 years ago

I still disagree with testing the actual SQL string. A single extra or missing whitespace can make the test fail, even though the SQL is actually correct.

Last edited 3 years ago by scribu (previous) (diff)

comment:30 in reply to: ↑ 29 @azaozz3 years ago

Replying to scribu:

...A single extra or missing whitespace can make the test fail...

Was looking at that when the test is on HTML code too (for wpautop). A simple normalize_whitespace regex is enough to fix this (needs to run on both strings before comparing them):

$str = preg_replace( '/[\r\n\t ]+/', ' ', $str );

\s is not good as it "eats" parts of some utf-8 chars.

comment:31 @scribu3 years ago

That was just one example. Here's another:

INNER JOIN wp_posts AS p_to_orderby

vs.

INNER JOIN wp_posts AS posts_orderby

The alias is arbitrary and doesn't affect the results, but it will make the test fail.

comment:32 @johnellmore3 years ago

  • Cc johnellmore added

comment:33 @cw63653 years ago

  • Cc cw6365 added

comment:34 @goldenapples2 years ago

  • Cc goldenapplesdesign@… added

comment:35 @swissspidy2 years ago

  • Cc hello@… added

comment:36 @mbijon2 years ago

  • Cc mike@… added

comment:37 @justindgivens2 years ago

  • Cc justingivens@… added

comment:38 @abrcam2 years ago

  • Cc abrcam added

Any plans to implement this in 3.6? It's been 2 years ...

comment:39 @vanjwilson2 years ago

  • Cc vanjwilson added

comment:40 @drozdz2 years ago

#24133 was marked as a duplicate.

comment:41 @SergeyBiryukov2 years ago

#24687 was marked as a duplicate.

@andy2 years ago

comment:42 @andy2 years ago

  • Version changed from 3.1 to trunk

This discussion stalled over how many tests to add and whether it needs to support the full SQL syntax. An improvement as simple and obvious as this should never have descended into such fruitless areas of debate.

ORDER BY is such a simple part of MySQL that it doesn't even get its own page for documentation. Its use is fully described in a few bullet points under SELECT Syntax. There is no reason to introduce array types.

Newly attached patch provides a human-readable, single-string, backwards-compatible solution.

comment:43 @ocean902 years ago

  • Version changed from trunk to 3.1

Version field indicates when the enhancement was initially suggested.

comment:44 @drozdz2 years ago

It's hard not to agree with andy. It's been 2 years and discussion on cool (but complicated) solutions won't fix it for sure.

comment:45 @wonderboymusic22 months ago

#15031 was marked as a duplicate.

comment:46 @wonderboymusic22 months ago

  • Milestone changed from Awaiting Review to 3.7

Hard to argue with a Skelton patch 24687.diff

comment:47 @helen22 months ago

Yes please pretty please.

comment:48 follow-up: @dd3222 months ago

I like the patch.

I do have one question though:

// If the next word is ASC or DESC, use it 
// for this sort key and cancel $q['order'] 

Killing $q['order'] might not be the best thing here, for example, take:

array(
   'order' => 'ASC',
   'order_by' => 'ID DESC, name'

To me that means order by ID desc, then name ASC, but that's no longer explicit.

Perhaps we'd be best to simply handle $q['order'] as a default ordering, and append a order to every matched item?

comment:49 in reply to: ↑ 48 @andy22 months ago

Replying to dd32:

Perhaps we'd be best to simply handle $q['order'] as a default ordering, and append a order to every matched item?

My patch was incorrect.

What makes it difficult to improve orderby is that WordPress defaults to DESC whereas MySQL defaults to ASC.

I had proposed to treat the new style like MySQL because it seemed natural to expect MySQL-like inputs to produce MySQL-like results. But I've changed my mind because the old orderby style was equally MySQL-like in input but could be opposite in output simply by omitting ASC and DESC.

It is possible to add ASC/DESC support to orderby without breaking back compat but not without making orderby confusing and error-prone for users.

So I abandon my previous patch attempting to improve orderby and instead propose a new query arg called order_by which is intended to be MySQL-like.

comment:50 @jameslnewell21 months ago

I just submitted a github PR for this before seeing the comments on the other PRs to use trac (apologies).

As described there my proposed syntax is:

$query = new WP_Query(array(
    'orderby' => 'menu_order date',
    'order'   => 'DESC ASC',
));

I *think* this patch should just work with the existing meta columns too.

P.S. There wouldn't be much to change if you want to keep the current (broken?) behaviour when only a single order is provided for multiple fields.

Last edited 21 months ago by jameslnewell (previous) (diff)

comment:51 @buffler21 months ago

  • Cc jeremy.buller@… added

comment:52 @nacin21 months ago

  • Milestone changed from 3.7 to Future Release

No final, agreed upon syntax or patch. Needs unit tests. I like this, though. 3.8? Would be good to follow up date queries with this.

comment:53 @nacin20 months ago

The patch in #23044 is dependent on this.

comment:54 @mboynes18 months ago

  • Cc mboynes@… added

comment:55 @toto17 months ago

Is there any help or hack?
The idea:

'order'   => 'DESC ASC',

is very good how to use it?

Now im using this hack to order multiple:

        'orderby'       => 'post_name meta_value_num',
        'order'         => 'DESC',

And add this hack

add_filter( 'posts_orderby', 'filter_query', 999);
function filter_query( $query ) {
    $query .= ', '.$wpdb->posts.post_name.' ASC';
    return $query;
}

Here we can order meta_value_num width 'order' => 'DESC',
and we can order - post_name width the hack.

Dont forget to remove the hack after your query with

remove_filter( 'posts_orderby', 'filter_query' );
Last edited 17 months ago by toto (previous) (diff)

comment:56 @qsz15 months ago

Hello,

I want to order data by double parameters such as meta_value_num and title.

Code

$args = array(
		'posts_per_page' => 500,
		'orderby' => 'meta_value_num title',
		'meta_key' => '_ss_views',
		'order' => 'DESC',
		'ignore_sticky_posts' => 1
	);

In this case all post are ordered by title (DESC) then by meta_value_num (ASC)

What I want is vice versa - First to order meta_value_num from the highest to lowest (DESC) and then order by title in alphabetical order (ASC).

Can you suggest some solutions?

comment:57 @ircbot15 months ago

This ticket was mentioned in IRC in #wordpress-dev by SergeyBiryukov_. View the logs.

comment:58 @wonderboymusic13 months ago

  • Milestone changed from Future Release to 4.0

I would like to shepherd this into existence

comment:59 @wonderboymusic12 months ago

In 28541:

Apply order to each passed value for orderby in WP_Query:

  • Since orderby in WP_Query can accept space-delimited sets, yet only one order value: when multiple values are passed (and DESC is the order), the default sort order ASC is being applied to all values before the last in the set.
  • There is a unit test that sporadically fails since 3.6 in tests/post/revision due to multiple posts having the same post_date from being added so rapidly
  • When ordering revisions in wp_get_post_revisions(), order by post_date ID
  • Change the order value in wp_get_post_revisions() to ASC. This will produce SQL like: ORDER BY $wpdb->posts.post_date ASC, $wpdb->posts.ID ASC. Previously, this would have produced SQL like: ORDER BY $wpdb->posts.post_date DESC, and with the addition of ID: ORDER BY $wpdb->posts.post_date ASC, $wpdb->posts.ID DESC. Clearly, wrong. The original SQL produced: ORDER BY $wpdb->posts.post_date DESC. As such, return the reversions in reverse order using array_reverse(). Not doing so would break "Preview Changes."
  • Add unit tests to assert that all of this works.
  • All existing unit tests pass with the change to ordering multiple orderbys in WP_Query.
  • In the future, we should support independent order for each orderby, see #17065.

Props SergeyBiryukov, wonderboymusic.
Fixes #26042.

@wonderboymusic12 months ago

comment:60 follow-up: @wonderboymusic12 months ago

  • Keywords needs-testing needs-unit-tests removed

In 17065.2.diff, use this syntax:

$q = new WP_Query( array( 
	'orderby' => array( 
		'type' => 'DESC', 
		'name' => 'ASC' 
 	) 
) ); 

Basically, if you pass an array, we know you are doing this. You must pass field_alias => order. This is easy to understand and hard to screw up. order, in this case, is ignored if passed.

Added unit tests. If you pass orderby => array(), there will be no ORDER BY clause created.

comment:61 @johnbillion11 months ago

Why on earth do we still have addslashes_gpc() in use here? Yikes. It's the only place in core we use this function.

That aside, wonderboymusic's suggested syntax is much better. +1 from me.

Another small note, I don't like the post fields in the allowed keys array not having a post_ prefix. We should support both (ie. type and post_type).

We can still get this into 4.0. Tweaked patch coming up.

comment:62 in reply to: ↑ 60 @andy11 months ago

Replying to wonderboymusic:

In 17065.2.diff, use this syntax:

+1

comment:63 @ircbot11 months ago

This ticket was mentioned in IRC in #wordpress-dev by DrewAPicture. View the logs.

@wonderboymusic11 months ago

@wonderboymusic11 months ago

@johnbillion11 months ago

comment:64 @johnbillion11 months ago

17065.5.diff builds upon wonderboymusic's latest patch:

  • Always cast the order to ASC or DESC
  • Allow post_ prefixed post fields in the allowed keys for orderby
  • Allow empty array or boolean false to blank out the orderby clause

@DrewAPicture11 months ago

docs + coding standards

comment:65 @DrewAPicture11 months ago

17065.6.diff tweaks the docs and fixes some coding standards.

@wonderboymusic11 months ago

comment:66 @ircbot11 months ago

This ticket was mentioned in IRC in #wordpress-dev by wonderboymusic. View the logs.

comment:67 @wonderboymusic11 months ago

In 29027:

Allow an array() to be passed as the value for orderby to WP_Query. Allows for an independent order value for each key.

Example: 'orderby' => array( 'title' => 'DESC', 'menu_order' => 'ASC' ).

Adds docs and unit tests.

Props wonderboymusic, johnbillion, DrewAPicture, dd32, andy.
See #17065.

comment:68 @nacin11 months ago

  • Resolution set to fixed
  • Status changed from new to closed
  • Type changed from enhancement to task (blessed)

comment:69 @stephenharris11 months ago

In this latest patch, would it be an idea to add a filter to parse_orderby()? My reasoning is that a plug-in may add their own sort key, and this would be lost.

As it stands you would have to use the posts_orderby filter - but when there are multiple sort keys you have to then rebuild the SQL segment. That in itself is OK, but if there are multiple 'unknown' keys from different plug-ins, it becomes impossible to produce the desired SQL.

It's an edge case, for sure, but I thought I'd put it out there.

comment:70 @RossCurrie10 months ago

I'm not sure that this is a meaningful contribution, or even the right thread for it, but I think it's worth pointing out somewhere that ordering by multiple attributes doesn't really work well when date/post_date is used, as it's sorted as a datetime value. Even posts on the same date are still ordered by the time they were created.

So if I want to sort by date,meta or date,title it just doesn't work.

comment:71 @ircbot9 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:72 @lavrenuk9 months ago

Not working

'orderby' => array ('meta_value_num' => 'DESC', 'title' => 'ASC'),
'meta_key' => 'count',

comment:73 @joshcanhelp2 months ago

I can also confirm that this is not working for me when I try to sort by date, then by meta_value. In a pre_get_posts action:

if ( version_compare( get_bloginfo( 'version' ), 4.0, 'ge' ) ) {
    $query->set( 'meta_key', 'wpri_rating' );
    $query->set( 'orderby', array(
        'post_date'           => 'DESC',
        'meta_value_num' => 'DESC'
     ) );
}

It sorts by date but not by meta_value

comment:74 @boonebgorges2 months ago

RossCurrie, lavrenuk, joshcanhelp - Can I ask one of you to open a new ticket describing the CAST issue, with detailed instructions on how to reproduce? A unit test demonstrating the bug would earn you karma points.

comment:75 follow-up: @joshcanhelp2 months ago

Happy to help ... but I'm not familiar with the term "CAST issue." Is there a format you'd like me to follow?

Also happy to write a test but have never done so for core before and the docs section covering that is a TODO:

https://make.wordpress.org/core/handbook/automated-testing/#writing-tests

comment:76 in reply to: ↑ 75 @boonebgorges2 months ago

Replying to joshcanhelp:

Happy to help ... but I'm not familiar with the term "CAST issue." Is there a format you'd like me to follow?

Also happy to write a test but have never done so for core before and the docs section covering that is a TODO:

https://make.wordpress.org/core/handbook/automated-testing/#writing-tests

Sorry - my reading of the reports above make it look like this is a problem with the data types used when WP_Query uses CAST when parsing orderby tokens https://core.trac.wordpress.org/browser/trunk/src/wp-includes/query.php?marks=2290#L2259. This is totally a guess though, as I haven't looked into it :)

Regarding the unit test. Here is an example of a simple unit test that tests 'orderby' using 'meta_query' on WP_Query https://core.trac.wordpress.org/browser/trunk/tests/phpunit/tests/query/metaQuery.php#L1264. If you want to have a go at it, you might copy that one and rewrite it to test your post_date + meta_value_num combination. But feel free to open the ticket without the unit test!

comment:77 @joshcanhelp2 months ago

Thanks for the link, that's very straightforward.

I'll put in the new ticket in a day or so.

EDIT: Realized that my problem was the same as RossCurie above (comment #70), not the orderby

Last edited 2 months ago by joshcanhelp (previous) (diff)
Note: See TracTickets for help on using tickets.