Opened 12 years ago
Last modified 6 years ago
#29424 reviewing enhancement
Query argument 'orderby' makes duplicated parameters possible; does not support some available columns
| Reported by: | ChriCo | Owned by: | johnbillion |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Query | Version: | 2.5 |
| Severity: | normal | Keywords: | has-patch has-unit-tests needs-refresh |
| Cc: | Focuses: |
Description (last modified by )
In your newest post in make-blog there a serveral errors in code.
- 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
- Why is
- "title" equal to "post_title"
- "name" to "post_name"
- "author" to "post_author"
and so on?
- By the way, "date_gmt", "post_status", "modifed_gmt", "content_filtered", "mime_type", "type" are missing...
- there are some columns missing in "orderby".
Attachments (4)
Change History (18)
#1
@
12 years ago
- Description modified (diff)
- Summary [WP 4.0] - orderby duplicated paramaters possible → Query argument 'orderby' makes duplicated parameters possible
- Version → trunk
#2
@
12 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
@
12 years ago
- Keywords needs-patch needs-testing added
- Milestone Awaiting Review → 4.0
Moving to 4.0 for investigation
#5
@
12 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.
12 years ago
#7
@
12 years ago
- Milestone 4.0 → Awaiting Review
- Version trunk → 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
@
12 years ago
- Keywords needs-testing removed
- Milestone Awaiting Review → Future Release
- Summary Query argument 'orderby' makes duplicated parameters possible → Query argument 'orderby' makes duplicated parameters possible; does not support some available columns
- Type defect (bug) → 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.
#9
@
9 years ago
Regarding the missing orderby fields, 29424.3.diff adds support for:
post_date_gmtdate_gmtpost_modified_gmtmodified_gmtpost_statuscomment_statusping_statuspost_mime_typemime_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.
#10
@
8 years ago
- Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed
The patch 29424.4.diff refreshes 29424.3.diff as it no longer applies cleanly to trunk.
It uses dataprovider for the unit testing.
#11
@
8 years ago
This ticket came to mind (regarding the few missing order by columns) when looking into the support for sorting columns in WP_Privacy_Requests_Table in #43960, as it includes a status column.
#13
@
7 years ago
- Keywords needs-refresh added
- Milestone 5.3 → 5.4
The 29424.4.diff patch no longer applies to trunk cleanly. With version 5.3 Beta 1 landing tomorrow, this is being punted to 5.4.
#14
@
6 years ago
- Milestone 5.4 → Future Release
With 5.4 Beta 1 only a few days away and this ticket having seen little movement in some time, this is being moved to Future Release. If any committer or maintainer feels this can be refreshed in time or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
Remove duplicate orderby column names