WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#9635 closed defect (bug) (fixed)

Comments Query Needs Filters

Reported by: filosofo Owned by:
Milestone: 2.8 Priority: normal
Severity: normal Version: 2.8
Component: Comments Keywords: comments-query has-patch tested commit dev-feedback
Focuses: Cc:

Description

Problem:
The comments query lacks order by and limit filters for its query, and it has a group by filter for only one of the two queries.

Example:
So, for example, if you wanted to select comments at random or some other means, you can't.

Solution:
Patch gives both comments queries the same 5 filters: join, where, groupby, orderby, limits.

Attachments (2)

comment_query_filters.9635.diff (1.9 KB) - added by filosofo 5 years ago.
comment_query_filters._consistent_prefix.9635.diff (2.7 KB) - added by filosofo 5 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Denis-de-Bernardy5 years ago

patch applies cleanly against today's trunk

comment:2 follow-up: Denis-de-Bernardy5 years ago

the groupby in the singular query isn't initialized properly though.

comment:3 in reply to: ↑ 2 filosofo5 years ago

Replying to Denis-de-Bernardy:

the groupby in the singular query isn't initialized properly though.

Why do you say that? If you mean that there's no default "GROUP BY" statement prepending it, that's the behavior of the current, non-singular filter. So users of the 'comment_feed_groupby' filter already must determine whether to prepend that.

comment:4 follow-up: Denis-de-Bernardy5 years ago

dunno I haven't actually tested the patch. but while scanning the patched file, I ended up wondering if the group by statement may end up containing junk that would mess the query up. testing would reveal if it does. it might not be very useful on the singular query, too.

comment:5 in reply to: ↑ 4 filosofo5 years ago

Replying to Denis-de-Bernardy:

dunno I haven't actually tested the patch. but while scanning the patched file, I ended up wondering if the group by statement may end up containing junk that would mess the query up. testing would reveal if it does.

I updated the patch to pass a blank string to the filter.

Replying to Denis-de-Bernardy:

it might not be very useful on the singular query, too.

Well, I can think of a couple of situations in which I could use a GROUP BY querying comments on a singular object, but that doesn't really matter for the purposes of this ticket. What matters is that this filter already exists for the other, non-singular comments query, so for consistency's sake it should be available with the singular one.

comment:6 Denis-de-Bernardy5 years ago

  • Keywords tested commit dev-feedback added

current patch is harmless.

adding dev-feedback keyword on one point, which is the group by.

in the posts query, group by is assumed to not contain any GROUP BY clause, i.e. if non-empty groupby then groupby = GROUP BY groupby.

whereas in the comments query, it's assumed to contain the GROUP BY clause. it's not much of a big deal, but at the risk of breaking a plugin or two we might want those two to be kept consistent.

comment:7 filosofo5 years ago

I've added a patch that makes the comments query order by and group by filters consistent with those of the main query.

comment:8 ryan5 years ago

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

(In [11211]) Comment query filters. Props filosofo. fixes #9635

Note: See TracTickets for help on using tickets.