I.e. we already do reorder the group clauses to match ORDER BY, to only
require a single sort. This happens in preprocess_groupclause(), which
also explains the reasoning behind that.
Huh. I missed that. That means group_keys_reorder_by_pathkeys() call inside
get_cheapest_group_keys_order() duplicates work of preprocess_groupclause().
And so it's possible to replace it to simple counting number of the same
pathkeys in beginning of lists. Or remove most part of preprocess_groupclause()
- but it seems to me we should use first option, preprocess_groupclause() is
simpler, it doesn't operate with pathkeys only with SortGroupClause which is
simpler.
BTW, incremental sort path provides pathkeys_common(), exactly what we need.
I wonder if some of the new code reordering group pathkeys could/should
be moved here (not sure, maybe it's too early for those decisions). In
any case, it might be appropriate to update some of the comments before
preprocess_groupclause() which claim we don't do certain things added by
the proposed patches.
preprocess_groupclause() is too early step to use something like
group_keys_reorder_by_pathkeys() because pathkeys is not known here yet.
This probably also somewhat refutes my claim that the order of grouping
keys is currently fully determined by users (and so they may pick the
most efficient order), while the reorder-by-ndistinct patch would make
that impossible. Apparently when there's ORDER BY, we already mess with
the order of group clauses - there are ways to get around it (subquery
with OFFSET 0) but it's much less clear.
I like a moment when objections go away :)
--
Teodor Sigaev E-mail: teo...@sigaev.ru
WWW: http://www.sigaev.ru/
