Re: Fire create_upper_paths_hook for UPPERREL_PARTIAL_GROUP_AGG

On 2026-08-26 We 2:44 PM, Bryan Green wrote:
> On 8/12/26 17:32, Andrew Dunstan wrote:
>> Greetings
>>
>> create_partial_grouping_paths() builds the UPPERREL_PARTIAL_GROUP_AGG
>> upper relation.  It already calls the FDW callback GetForeignUpperPaths
>> there, but never calls create_upper_paths_hook, the general-purpose
>> hook that non-FDW extensions use to add paths. UPPERREL_PARTIAL_DISTINCT
>> doesn't have this gap: create_partial_distinct_paths() calls both
>> GetForeignUpperPaths and create_upper_paths_hook for it.
>>
>> This patch adds the missing create_upper_paths_hook call right next to
>> the existing GetForeignUpperPaths call, so a non-FDW extension can add
>> partial aggregation paths at the same point an FDW already can, before
>> those paths are gathered and a Finalize Aggregate is built on top.
>>
>> It's a small, self-contained planner change with no effect on existing
>> plans unless an extension registers create_upper_paths_hook and adds
>> paths at this new call site.
>>
>>
> Andrew,
>
> If you use enable_partitionwise_aggregate then
> create_partial_grouping_paths will run once for the parent and then
> again for each child. This leads to the
> upper_targets[UPPERREL_PARTIAL_GROUP_AGG] assignment being overwritten
> per child, resulting in upper_targets holding the last child's
> reltarget. This happens whenever partitionwise aggregation is
> considered-- it doesn't have to win. So any extension reading the slot
> after the grouping stage gets the wrong target. I reproduced this with a
> test hook.
>
> The hook already gets the target as output_rel->reltarget, so I think we
> could just drop the assignment?
>
>

Quite right. That was left over from an earlier draft, and is clearly an
error, thanks for catching it. Here's an updated patch.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com