WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 6 weeks ago

#17065 new enhancement

Independent ASC/DESC in multiple ORDER BY statement.

Reported by: ericmann Owned by: ericmann
Priority: normal Milestone: Awaiting Review
Component: Query Version: 3.1
Severity: normal Keywords: has-patch needs-testing needs-unit-tests
Cc: gary@…, linuxologos@…, kucrut, maor@…, rosariobocca, tomaugerdotcom@…, ejdanderson@…, code@…, johnellmore, cw6365, goldenapplesdesign@…, hello@…, mike@…, justingivens@…, abrcam, vanjwilson

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 (1)

17065.diff (4.1 KB) - added by dd32 2 years ago.

Download all attachments as: .zip

Change History (40)

comment:1 scribu2 years ago

Any syntax proposals?

comment:2 ericmann2 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 scribu2 years ago

That's a good start.

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

comment:4 pento2 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 scribu2 years ago

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

comment:6 linuxologos2 years ago

  • Cc linuxologos@… added

comment:7 kucrut2 years ago

  • Cc kucrut added

comment:8 maorb2 years ago

  • Cc maor@… added

comment:9 dd322 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 scribu2 years ago

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

comment:11 scribu2 years ago

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

comment:12 scribu2 years ago

  • Severity changed from trivial to normal

comment:13 dd322 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)

dd322 years ago

comment:14 dd322 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 dd322 years ago

  • Keywords needs-testing added

comment:16 scribu23 months ago

Marked #9979 as duplicate.

comment:17 lukecarbis20 months 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 20 months ago by lukecarbis (previous) (diff)

comment:18 dd3220 months 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 rosariobocca19 months ago

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

comment:20 follow-up: scribu19 months 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 rosariobocca19 months ago

sorry.

Replying to scribu:

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.

Version 0, edited 19 months ago by rosariobocca (next)

comment:22 tomauger19 months ago

  • Cc tomaugerdotcom@… added

Really digging this new syntax. Great stuff.

comment:23 ejdanderson18 months ago

  • Cc ejdanderson@… added

comment:24 davecc16 months 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.gz10 months ago

  • Cc code@… added

comment:26 MikevHoenselaar10 months ago

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

comment:27 lukecarbis10 months ago

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

comment:28 nacin10 months 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: scribu10 months 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 10 months ago by scribu (previous) (diff)

comment:30 in reply to: ↑ 29 azaozz10 months 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 scribu10 months 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 johnellmore9 months ago

  • Cc johnellmore added

comment:33 cw63659 months ago

  • Cc cw6365 added

comment:34 goldenapples5 months ago

  • Cc goldenapplesdesign@… added

comment:35 swissspidy5 months ago

  • Cc hello@… added

comment:36 mbijon5 months ago

  • Cc mike@… added

comment:37 justindgivens4 months ago

  • Cc justingivens@… added

comment:38 abrcam4 months ago

  • Cc abrcam added

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

comment:39 vanjwilson6 weeks ago

  • Cc vanjwilson added
Note: See TracTickets for help on using tickets.