Re: POC: GROUP BY optimization

From: Alexander Korotkov <aekorotkov(at)gmail(dot)com>
To: jian he <jian(dot)universality(at)gmail(dot)com>
Cc: Andrei Lepikhov <a(dot)lepikhov(at)postgrespro(dot)ru>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Richard Guo <guofenglinux(at)gmail(dot)com>, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>, David Rowley <dgrowleyml(at)gmail(dot)com>, "a(dot)rybakina" <a(dot)rybakina(at)postgrespro(dot)ru>
Subject: Re: POC: GROUP BY optimization
Date: 2024-06-02 13:18:11
Message-ID: CAPpHfdsTUWnrNFRTenQpmv=JQ7yFdmoMbEADMSg8SNuHOZDFwQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi!

On Sun, Jun 2, 2024 at 10:55 AM jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Fri, May 31, 2024 at 8:12 AM Alexander Korotkov <aekorotkov(at)gmail(dot)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

Attachment Content-Type Size
v4-0003-Rename-PathKeyInfo-to-GroupByOrdering.patch application/octet-stream 7.0 KB
v4-0004-Restore-preprocess_groupclause.patch application/octet-stream 13.1 KB
v4-0005-Teach-group_keys_reorder_by_pathkeys-about-redund.patch application/octet-stream 3.2 KB
v4-0001-Fix-asymmetry-in-setting-EquivalenceClass.ec_sort.patch application/octet-stream 8.8 KB
v4-0002-Add-invariants-check-to-get_useful_group_keys_ord.patch application/octet-stream 1.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2024-06-02 18:39:02 Re: The xversion-upgrade test fails to stop server
Previous Message Pierre Forstmann 2024-06-02 12:32:55 Re: pltcl crashes due to a syntax error