> 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: teodor@sigaev.ru
WWW: http://www.sigaev.ru/