Re: WIP: Aggregation push-down - take2

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?


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company