Horton

Re: POC: GROUP BY optimization

Alexander Korotkov <aekorotkov@gmail.com>

View thread
From: Alexander Korotkov <aekorotkov@gmail.com>
To: Andrei Lepikhov <a.lepikhov@postgrespro.ru>
Cc: 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>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2024-01-15T00:19:41Z
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

Attachments

On Sun, Jan 14, 2024 at 2:14 PM Andrei Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 13/1/2024 22:00, Alexander Korotkov wrote:
> > On Sat, Jan 13, 2024 at 11:09 AM Andrei Lepikhov
> > <a.lepikhov@postgrespro.ru> wrote:
> >> On 11/1/2024 18:30, Alexander Korotkov wrote:
> >>> On Tue, Jan 9, 2024 at 1:14 PM Pavel Borisov <pashkin.elfe@gmail.com> wrote:
> >>>>> Hmm, I don't see this old code in these patches. Resend 0002-* because
> >>>>> of trailing spaces.
> >>>>
> >>>>
> >>>> AFAIK, cfbot does not seek old versions of patchset parts in previous messages. So for it to run correctly, a new version of the whole patchset should be sent even if most patches are unchanged.
> >>>
> >>> Please, find the revised patchset with some refactoring and comments
> >>> improvement from me.  I'll continue looking into this.
> >> The patch looks better, thanks to your refactoring.
> >> I propose additional comments and tests to make the code more
> >> understandable (see attachment).
> >> I intended to look into this part of the code more, but the tests show a
> >> difference between PostgreSQL v.15 and v.16, which causes changes in the
> >> code of this feature.
> >
> > Makes sense.  I've incorporated your changes into the patchset.
> One more improvement. To underpin code change:
>
> -               return cur_ec;  /* Match! */
> +           {
> +               /*
> +                * Match!
> +                *
> +                * Copy the sortref if it wasn't set yet. That may happen if
> +                * the ec was constructed from WHERE clause, i.e. it doesn't
> +                * have a target reference at all.
> +                */
> +               if (cur_ec->ec_sortref == 0 && sortref > 0)
> +                   cur_ec->ec_sortref = sortref;
> +               return cur_ec;
> +           }
>
> I propose the test (see attachment). It shows why we introduce this
> change: GROUP-BY should juggle not only pathkeys generated by explicit
> sort operators but also planner-made, likewise in this example, by
> MergeJoin.

Thank you for providing the test case relevant for this code change.
The revised patch incorporating this change is attached.  Now the
patchset looks good to me.  I'm going to push it if there are no
objections.

------
Regards,
Alexander Korotkov