Make WordPress Core

Opened 14 years ago

Closed 12 years ago

Last modified 11 years ago

#13729 closed enhancement (fixed)

Post Query should be able to order by specific order passed in "post__in"

Reported by: jakemgold's profile jakemgold Owned by: jakemgold's profile jakemgold
Milestone: 3.5 Priority: normal
Severity: minor Version: 3.0
Component: Query Keywords: has-patch needs-codex
Focuses: Cc:

Description (last modified by scribu)

Post queries can specify the posts to retrieve by passing an array of post IDs to the post__in parameter.

There is no built in method for sorting those posts by the order specified. In fact, I would argue that when using post__in, the post query should, by default, order the posts in the same order specified in that array (I think this is the expected behavior by the developer).

Many custom themes allow the developer to specifically designate featured posts, controlling the order in which they appear. Many of these themes are forced to make independent queries for each post or execute an atypical loop by calling the result by ID (the post array's key). The ability to "fetch", properly order, and execute a standard loop for a specific set of posts in a specific order would reduce overhead and complexity in these situations.

I've developed a plug-in that does this as a workaround:

http://wordpress.org/extend/plugins/sort-query-by-post-in/

The 3.0+ specific code in there is *very* simple. It would be even more simple to do in base code (just another "case" in the orderby switch part of the query builder). In fact, I intend to submit a patch after 3.0 is released.

My plug-in provides the framework for the SQL to do this.

Attachments (3)

orderby post__in.diff (616 bytes) - added by jakemgold 13 years ago.
finally submitted a patch…
orderby post__in.2.diff (613 bytes) - added by jakemgold 13 years ago.
less is more with code changes…
13729-FIELD.diff (633 bytes) - added by markjaquith 12 years ago.

Download all attachments as: .zip

Change History (28)

#1 @nacin
14 years ago

  • Keywords 2nd-opinion added; post__in custom query orderby removed
  • Milestone changed from 3.1 to Future Release

We cannot change the functionality of *__in and not_in query vars. I would definitely not call it expected behavior.

Probably best left to a plugin to order by supplied IDs.

#2 @jakemgold
14 years ago

Nacin - I don't understand your feedback.

You don't need to change the default behavior of "in". I would simply recommend adding a new "orderby" option that - under the right circumstances - allows the user to retrieve the posts in the specified order. My plugin adds a "post in" orderby option, but you could probably use a more friendly value like "specified".

"not_in" is irrelevant, since your can't "order" excluded posts.

This could be done with two new lines of code in the orderby case statement, and would not break anything (a patch I'll be happy to submit).

It's hard for me to imagine WordPress evolving as a full featured CMS without the ability to retrieve a list of posts in a specifically desired order.

#3 @nacin
14 years ago

What caused me to say that we can't change the functionality of the *__in vars was that they should be ordered by default how they are passed. That would be a change of behavior. An orderby seems okay.

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

#4 @nacin
14 years ago

Oh Trac, I hate you.

What caused me to say that we can't change the functionality of the *__in vars was that they should be ordered by default how they are passed. That would be a change of behavior. An orderby seems okay.

#5 @jakemgold
14 years ago

Sounds like we're on the same page, nacin.

While I do think - if we were starting from scratch - that *__in should return in the order specified, I agree that we can't swap the default behavior for want of backwards compatibility.

The right way forward is a new orderby option.

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

#6 @scribu
13 years ago

I'm pretty sure there's another duplicate of this ticket somewhere, wherein it was mentioned that this type of orderby can be achieved by using MySQL's FIND_IN_SET() function.

@jakemgold
13 years ago

finally submitted a patch...

#7 @jakemgold
13 years ago

  • Keywords has-patch added
  • Resolution set to fixed
  • Status changed from new to closed
  • Version changed from 3.0 to 3.2

#8 @jakemgold
13 years ago

Finally submitted a patch... fairly easy to test since there's no need for multiple orderby paramaters when you're ordering by ID (given that it's unique, there should never be a secondary orderby need).

Only questionable area has to do with "order" argument. Since the query is explicitly provided IDs, I've simply left out "order". If they want it in reverse order, they could theoretically just enter them in reverse order. We could tack it back on, but it will be a bit tricky, since the expected order is ASC (the order supplied), which is the opposite of the "default" WordPress sets earlier (DESC).

#9 @dd32
13 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened
  • Version changed from 3.2 to 3.0

Tickets are marked as fixed once a patch has been applied to the trunk. The version field is to record the first-known/reported version that the ticket applies to.

Quick patch review: empty( $post__in ) could probably be used in the if() branch to allow it to fall through to the default (which i believe is order by date?)

@jakemgold
13 years ago

less is more with code changes...

#10 @jakemgold
13 years ago

  • Owner changed from ryan to dd32
  • Status changed from reopened to reviewing

Apologies for the inappropriate resolution... making my first core commits.

I've modified the diff to let it fall through to the "else" (where it will order by its default, date) if $post__in is empty. Arguably a tiny bit less efficient (since it takes the time to set the acceptable allowed keys and does some sanitizing), but probably prudent to not have the default orderby kept in more than one or two places.

We could, alternatively, set the default orderby before the if() branch, since it's already referenced in two places.

#11 follow-up: @scribu
13 years ago

making my first core commits.

What you're doing is uploading patches, probably created using svn diff.

A commit is what you do with the svn commit command.

#12 in reply to: ↑ 11 @jakemgold
13 years ago

Replying to scribu:

making my first core commits.

What you're doing is uploading patches, probably created using svn diff.

A commit is what you do with the svn commit command.

Meant to to say contributions...

I'm quite aware of the difference between a patch and commit, thanks. :-P

#13 @dd32
13 years ago

  • Owner dd32 deleted

#14 @jakemgold
12 years ago

  • Owner set to jakemgold
  • Status changed from reviewing to accepted

#15 follow-up: @nacin
12 years ago

I wonder what the performance and caching ramifications are for doing this in SQL, rather than just ordering $query->posts when done.

#16 @scribu
12 years ago

  • Description modified (diff)

#17 in reply to: ↑ 15 @scribu
12 years ago

Replying to nacin:

I wonder what the performance and caching ramifications are for doing this in SQL, rather than just ordering $query->posts when done.

If there are more items in 'post__in' than the limit set by 'posts_per_page', you'll get different results.

#18 @Otto42
12 years ago

ORDER BY FIELD also works. Not sure if there is a performance difference.

See my answer to this sort of problem here:
http://wordpress.stackexchange.com/questions/62546/orderby-none-not-working/62551#62551

#20 @markjaquith
12 years ago

  • Resolution set to fixed
  • Status changed from accepted to closed

2:29 PM nacin: after talking with wonderboymusic and pento yesterday, FIELD should be used instead
2:30 PM nacin: they are basically the same but FIELD can optimize for numbers, and it looks a lot cleaner.

#21 @markjaquith
12 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

What the heck? Didn't mean to close that.

#22 @markjaquith
12 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [21776]:

Allow orderby=postin, which uses the explicit order you provided in the postin parameter. fixes #13729. props jakemgold, Otto42.

#23 @scribu
12 years ago

  • Milestone changed from Future Release to 3.5

Woo!

#24 @DrewAPicture
11 years ago

  • Keywords needs-codex added

#25 @DrewAPicture
11 years ago

  • Keywords 2nd-opinion removed
Note: See TracTickets for help on using tickets.