From: | Tender Wang <tndrwang(at)gmail(dot)com> |
---|---|
To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
Cc: | Paul George <p(dot)a(dot)george19(at)gmail(dot)com>, Andy Fan <zhihuifan1213(at)163(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com> |
Subject: | Re: Eager aggregation, take 3 |
Date: | 2024-09-05 01:40:18 |
Message-ID: | CAHewXN=tGYzuuHMCR_0dQWZX-oV-gJsSJnAkt-q2n6yRzFm7ww@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Richard Guo <guofenglinux(at)gmail(dot)com> 于2024年8月21日周三 15:11写道:
> On Fri, Aug 16, 2024 at 4:14 PM Richard Guo <guofenglinux(at)gmail(dot)com>
> wrote:
> > I had a self-review of this patchset and made some refactoring,
> > especially to the function that creates the RelAggInfo structure for a
> > given relation. While there were no major changes, the code should
> > now be simpler.
>
> I found a bug in v10 patchset: when we generate the GROUP BY clauses
> for the partial aggregation that is pushed down to a non-aggregated
> relation, we may produce a clause with a tleSortGroupRef that
> duplicates one already present in the query's groupClause, which would
> cause problems.
>
> Attached is the updated version of the patchset that fixes this bug
> and includes further code refactoring.
I review the v11 patch set, and here are a few of my thoughts:
1. in setup_eager_aggregation(), before calling create_agg_clause_infos(),
it does
some checks if eager aggregation is available. Can we move those checks
into a function,
for example, can_eager_agg(), like can_partial_agg() does?
2. I found that outside of joinrel.c we all use IS_DUMMY_REL, but in
joinrel.c, Tom always uses
is_dummy_rel(). Other commiters use IS_DUMMY_REL.
3. The attached patch does not consider FDW when creating a path for
grouped_rel or grouped_join.
Do we need to think about FDW?
I haven't finished reviewing the patch set. I will continue to learn this
feature.
--
Thanks,
Tender Wang
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2024-09-05 01:59:13 | Re: Test 041_checkpoint_at_promote.pl faild in installcheck due to missing injection_points |
Previous Message | Michael Paquier | 2024-09-05 00:03:26 | Re: Typos in the code and README |