WordPress.org

Make WordPress Core

Opened 3 years ago

Last modified 7 weeks ago

#29424 new enhancement

Query argument 'orderby' makes duplicated parameters possible; does not support some available columns

Reported by: ChriCo Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.5
Component: Query Keywords: needs-patch needs-unit-tests
Focuses: Cc:

Description (last modified by ocean90)

In your newest post in make-blog there a serveral errors in code.

  1. Duplicated parameters are possible. Following Code will output the "error":
$query = new WP_Query( array( 'orderby' => array( 'title' => 'DESC', 'post_title' => 'ASC' ) ) );

//ouputs: ORDER BY post_title DESC, post_title ASC
  1. Why is
  • "title" equal to "post_title"
  • "name" to "post_name"
  • "author" to "post_author"

and so on?

  1. By the way, "date_gmt", "post_status", "modifed_gmt", "content_filtered", "mime_type", "type" are missing...
  1. there are some columns missing in "orderby".

Attachments (3)

29424.diff (1.2 KB) - added by jesin 3 years ago.
Remove duplicate orderby column names
29424.2.diff (2.1 KB) - added by jesin 3 years ago.
Patch 29424.diff with unit tests
29424.3.diff (7.7 KB) - added by birgire 7 weeks ago.
Adds missing orderby fields + tests

Download all attachments as: .zip

Change History (12)

#1 @ocean90
3 years ago

  • Description modified (diff)
  • Summary changed from [WP 4.0] - orderby duplicated paramaters possible to Query argument 'orderby' makes duplicated parameters possible
  • Version set to trunk

@jesin
3 years ago

Remove duplicate orderby column names

#2 @jesin
3 years ago

Cause: The parse_orderby() function prefixes post_ to names like date, author, title in the "default" case of its switch block.

The 29424.diff patch checks if a column name exists already in the $orderby_array and skips a loop if it does.

I found another bug which was also patched in this file.

$query = new WP_Query( array( 'orderby' => 'post_title post_date post_author', 'order' => 'ASC' ) );

//Outputs: wp_posts.post_title ASC, wp_posts.post_date ASC, wp_posts.post_author

Notice the missing ASC for the last column. This is caused by the use of implode() to add the "order" index.

#3 @johnbillion
3 years ago

  • Keywords needs-patch needs-testing added
  • Milestone changed from Awaiting Review to 4.0

Moving to 4.0 for investigation

#4 @ocean90
3 years ago

  • Keywords needs-unit-tests added

@jesin
3 years ago

Patch 29424.diff with unit tests

#5 @jesin
3 years ago

Patch 29424.2.diff contains unit tests. The other bug I mentioned in comment 2 does not exist, it was my mistake.

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


3 years ago

#7 @johnbillion
3 years ago

  • Milestone changed from 4.0 to Awaiting Review
  • Version changed from trunk to 2.5

This orderby parameter is ambiguous at best and erroneous at worst:

'orderby' => array(
    'title' => 'DESC',
    'post_title' => 'ASC'
)

If you're performing a query such as this then your code is incorrect. What is the expected value of the ORDER BY clause in this case? 29424.2.diff actually makes this behaviour even more confusing, because it silently discards the second of the duplicated fields.

For these reasons, we don't plan to handle such values in this parameter and the current behaviour won't be changed.

Regarding the missing fields available in the orderby parameter ("date_gmt", "post_status", "modifed_gmt", "content_filtered", "mime_type", "post_type"), this is not a change in 4.0, so I'm punting this ticket while I look to see if this has already been reported in another ticket.

#8 @boonebgorges
3 years ago

  • Keywords needs-testing removed
  • Milestone changed from Awaiting Review to Future Release
  • Summary changed from Query argument 'orderby' makes duplicated parameters possible to Query argument 'orderby' makes duplicated parameters possible; does not support some available columns
  • Type changed from defect (bug) to enhancement

+1 to johnbillion's conclusions regarding duplicate parameters.

Regarding the missing fields available in the orderby parameter ("date_gmt", "post_status", "modifed_gmt", "content_filtered", "mime_type", "post_type"), this is not a change in 4.0, so I'm punting this ticket while I look to see if this has already been reported in another ticket.

I've done some searching and can't find another ticket. We can add these columns if we get a patch + unit tests.

@birgire
7 weeks ago

Adds missing orderby fields + tests

#9 @birgire
7 weeks ago

Regarding the missing orderby fields, 29424.3.diff adds support for:

  • post_date_gmt
  • date_gmt
  • post_modified_gmt
  • modified_gmt
  • post_status
  • comment_status
  • ping_status
  • post_mime_type
  • mime_type

with a test for each one.

Note that I skipped status here, as I think it wouldn't be clear what was being referred to: post_status, comment_status or ping_status.

ps: I also searched for any other ticket on this, but couldn't find it.

Note: See TracTickets for help on using tickets.