From: Antonin Houska <ah@cybertec.at>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
cc: "Fujii.Yuki@df.MitsubishiElectric.co.jp"
 <Fujii.Yuki@df.MitsubishiElectric.co.jp>,
    "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>,
    "david@pgmasters.net" <david@pgmasters.net>,
    "tgl@sss.pgh.pa.us" <tgl@sss.pgh.pa.us>,
    "zhihui.fan1213@gmail.com" <zhihui.fan1213@gmail.com>,
    "legrand_legrand@hotmail.com" <legrand_legrand@hotmail.com>,
    "daniel@yesql.se" <daniel@yesql.se>
Subject: Re: WIP: Aggregation push-down - take2
In-reply-to: <a2f704ab-aa50-b6af-5e92-0fece5ad07e5@enterprisedb.com>
References: 
 <OS3PR01MB66609589B896FBDE190209F495EE9@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <OS3PR01MB666003EBE5B7234FDFB9018E95869@OS3PR01MB6660.jpnprd01.prod.outlook.com>
 <a2f704ab-aa50-b6af-5e92-0fece5ad07e5@enterprisedb.com>
Comments: In-reply-to Tomas Vondra <tomas.vondra@enterprisedb.com>
   message dated "Mon, 31 Oct 2022 20:00:20 +0100."
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Date: Fri, 04 Nov 2022 15:17:15 +0100
Message-ID: <89085.1667571435@antos>
Archived-At: <https://www.postgresql.org/message-id/89085.1667571435%40antos>
Precedence: bulk

--=-=-=
Content-Type: text/plain

Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:

> Hi,
> 
> On 7/12/22 08:49, Fujii.Yuki@df.MitsubishiElectric.co.jp wrote:
> > Hi everyone.
> > 
> > I rebased the following patches which were submitted in [1].
> >     v17-0001-Introduce-RelInfoList-structure.patch
> >     v17-0002-Aggregate-push-down-basic-functionality.patch
> >     v17-0003-Use-also-partial-paths-as-the-input-for-grouped-path.patch
> > 
> > I checked I can apply the rebased patch to commit 2cd2569c72b8920048e35c31c9be30a6170e1410.
> > 
> > I'm going to register the rebased patch in next commitfest.
> > 
> I've started looking at this patch series again, but I wonder what's the
> plan. The last patch version no longer applies, so I rebased it - see
> the attachment. The failures were pretty minor, but there're two warnings:
> 
> pathnode.c:3174:11: warning: variable 'agg_exprs' set but not used
> [-Wunused-but-set-variable]
>         Node       *agg_exprs;
>                     ^
> pathnode.c:3252:11: warning: variable 'agg_exprs' set but not used
> [-Wunused-but-set-variable]
>         Node       *agg_exprs;
>                     ^
> 
> so there seem to be some loose ends. Moreover, there are two failures in
> make check, due to plan changes like this:
> 
> + Finalize GroupAggregate
>     Group Key: p.i
> -   ->  Nested Loop
> -         ->  Partial HashAggregate
> -               Group Key: c1.parent
> -               ->  Seq Scan on agg_pushdown_child1 c1
> -         ->  Index Scan using agg_pushdown_parent_pkey on ...
> -               Index Cond: (i = c1.parent)
> -(8 rows)
> +   ->  Sort
> +         Sort Key: p.i
> +         ->  Nested Loop
> +               ->  Partial HashAggregate
> +                     Group Key: c1.parent
> +                     ->  Seq Scan on agg_pushdown_child1 c1
> +               ->  Index Scan using agg_pushdown_parent_pkey on ...
> +                     Index Cond: (i = c1.parent)
> +(10 rows)
> 
> This seems somewhat strange - maybe the plan is correct, but the extra
> sort seems unnecessary.
> 
> However, maybe I'm confused/missing something? The above message says
> v17 having parts 0001-0003, but there's only one patch in v18. So maybe
> I failed to apply some prior patch?

I've rebased the last version I had on my workstation (v17), the regression
tests just worked. Maybe v18 was messed up. v20 is attached.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=v20-0001-Introduce-RelInfoList-structure.patch