Re: POC: GROUP BY optimization

Alexander Korotkov <aekorotkov@gmail.com>

From: Alexander Korotkov <aekorotkov@gmail.com>

To: jian he <jian.universality@gmail.com>

Cc: Andrei Lepikhov <a.lepikhov@postgrespro.ru>, Tom Lane <tgl@sss.pgh.pa.us>, Richard Guo <guofenglinux@gmail.com>, Pavel Borisov <pashkin.elfe@gmail.com>, vignesh C <vignesh21@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Teodor Sigaev <teodor@sigaev.ru>, David Rowley <dgrowleyml@gmail.com>, "a.rybakina" <a.rybakina@postgrespro.ru>

Date: 2024-06-02T13:18:11Z

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 →

Restore preprocess_groupclause()
- 505c008ca37c 17.0 landed
Rename PathKeyInfo to GroupByOrdering
- 0c1af2c35c7b 17.0 landed
Add invariants check to get_useful_group_keys_orderings()
- 91143c03d4ca 17.0 landed
Fix asymmetry in setting EquivalenceClass.ec_sortref
- 199012a3d844 17.0 landed
Multiple revisions to the GROUP BY reordering tests
- 874d817baa16 17.0 landed
Get rid of pg_class usage in SJE regression tests
- e1b7fde418f2 17.0 landed
Rename index "abc" in aggregates.sql
- b91f91870828 17.0 landed
Explore alternative orderings of group-by pathkeys during optimization.
- 0452b461bc40 17.0 landed
Generalize the common code of adding sort before processing of grouping
- 7ab80ac1caf9 17.0 landed
Fix out-dated comment in preprocess_groupclause()
- f6c70b81802a 15.0 landed
- 78a9af1a2764 16.0 landed
Force parallelism in partition_aggregate
- 2fe6b2a806f2 16.0 landed
- 01474f56981a 15.0 landed
Optimize order of GROUP BY keys
- db0d67db2401 15.0 landed

Attachments

v4-0003-Rename-PathKeyInfo-to-GroupByOrdering.patch (application/octet-stream) patch v4-0003
v4-0004-Restore-preprocess_groupclause.patch (application/octet-stream) patch v4-0004
v4-0005-Teach-group_keys_reorder_by_pathkeys-about-redund.patch (application/octet-stream) patch v4-0005
v4-0001-Fix-asymmetry-in-setting-EquivalenceClass.ec_sort.patch (application/octet-stream) patch v4-0001
v4-0002-Add-invariants-check-to-get_useful_group_keys_ord.patch (application/octet-stream) patch v4-0002

Hi!

On Sun, Jun 2, 2024 at 10:55 AM jian he <jian.universality@gmail.com> wrote:
>
> On Fri, May 31, 2024 at 8:12 AM Alexander Korotkov <aekorotkov@gmail.com> wrote:
> >
> > I've revised some grammar including the sentence you've proposed.
> >
>
> -static List *groupclause_apply_groupingset(PlannerInfo *root, List *force);
> +static List *preprocess_groupclause(PlannerInfo *root, List *force);
>
> changing preprocess_groupclause the second argument
> from "force" to "gset" would be more intuitive, I think.

Probably, but my intention is to restore preprocess_groupclause() as
it was before 0452b461bc with minimal edits to support incremental
sort.  I'd rather avoid refactoring if this area for now.

> `elog(ERROR, "Order of group-by clauses doesn't correspond incoming
> sort order");`
>
> I think this error message makes people wonder what "incoming sort order" is.
> BTW, "correspond", generally people use  "correspond to".

Thank you.  On the second thought, I think it would be better to turn
this into an assertion like the checks before.

> I did some minor cosmetic changes, mainly changing foreach to foreach_node.
> Please check the attachment.

I would avoid refactoring of preprocess_groupclause() for the reason
described above.  But I picked the grammar fix for PlannerInfo's
comment.

------
Regards,
Alexander Korotkov
Supabase