On Thu, 25 Jul 2019 at 11:33, Andres Freund <and...@anarazel.de> wrote: > > On 2019-07-25 10:36:26 +1200, David Rowley wrote: > > 2) Planner trying to give nodeAgg.c a sorted path to work with on > > DISTINCT / ORDER BY aggs > > That'll have to be a best effort thing though, i.e. there'll always be > cases where we'll have to retain the current logic (or just regress > performance really badly)?
It's something we already do for windowing functions. We just don't do it for aggregates. It's slightly different since windowing functions just chain nodes together to evaluate multiple window definitions. Aggregates can't/don't do that since aggregates... well, aggregate, (i.e the input to the 2nd one can't be aggregated by the 1st one) but there's likely not much of a reason why standard_qp_callback couldn't choose some pathkeys for the first AggRef with a ORDER BY / DISTINCT clause. nodeAgg.c would still need to know how to change the sort order in order to evaluate other Aggrefs in some different order. I'm not quite sure where the regression would be. nodeAgg.c must perform the sort, or if we give the planner some pathkeys, then worst case the planner adds a Sort node. That seems equivalent to me. However, in the best case, there's a suitable index and no sorting is required anywhere. Probably then we can add combine function support for the remaining built-in aggregates. There was trouble doing that in [1] due to some concerns about messing up results for people who rely on the order of an aggregate without actually writing an ORDER BY. [1] https://www.postgresql.org/message-id/cakjs1f9sx_6gtcvd6tmuznntch0vhbzhx6fzqw17tgvfh-g...@mail.gmail.com -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
