Horton

Re: POC: GROUP BY optimization

Teodor Sigaev <teodor@sigaev.ru>

View thread
From: Teodor Sigaev <teodor@sigaev.ru>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>, Pgsql Hackers <pgsql-hackers@postgresql.org>
Date: 2018-06-13T16:56:00Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Restore preprocess_groupclause()

  2. Rename PathKeyInfo to GroupByOrdering

  3. Add invariants check to get_useful_group_keys_orderings()

  4. Fix asymmetry in setting EquivalenceClass.ec_sortref

  5. Multiple revisions to the GROUP BY reordering tests

  6. Get rid of pg_class usage in SJE regression tests

  7. Rename index "abc" in aggregates.sql

  8. Explore alternative orderings of group-by pathkeys during optimization.

  9. Generalize the common code of adding sort before processing of grouping

  10. Fix out-dated comment in preprocess_groupclause()

  11. Force parallelism in partition_aggregate

  12. Optimize order of GROUP BY keys

> 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/