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
From 8187c721ccece56af8d2c75b0d8d3152e329fcc3 Mon Sep 17 00:00:00 2001
From: Andrew Dunstan <[email protected]>
Date: Sat, 9 May 2026 14:57:09 -0400
Subject: [PATCH v3] Fire create_upper_paths_hook for
UPPERREL_PARTIAL_GROUP_AGG
The planner already invites FDWs and extensions to add partial paths
on UPPERREL_PARTIAL_DISTINCT (see create_partial_distinct_paths), but
the symmetric create_upper_paths_hook call on UPPERREL_PARTIAL_GROUP_AGG
is missing. Without it, an extension cannot register a partial-aggregate
path through the standard hook surface: by the time its
create_upper_paths_hook runs on UPPERREL_GROUP_AGG, the planner has
already gathered partial paths from partially_grouped_rel and built
Finalize Aggregate on top, so paths added late are dead code.
Fire create_upper_paths_hook on partially_grouped_rel at the end of
create_partial_grouping_paths, right next to the existing
GetForeignUpperPaths call for UPPERREL_PARTIAL_GROUP_AGG. This mirrors
create_partial_distinct_paths, which pairs its GetForeignUpperPaths and
create_upper_paths_hook calls together at the end of that function for
UPPERREL_PARTIAL_DISTINCT. Extensions adding partial paths here are
picked up naturally by the caller's subsequent gather_grouping_paths
call and become candidates for the upstream Finalize Aggregate built by
add_paths_to_grouping_rel.
The motivating use case is a CustomScan that wants to participate in
partial aggregation -- e.g. a columnstore extension whose pushdown
runs cheaper than nodeAgg's standard transition path. Without this
hook, the extension is forced to choose between sequential agg
pushdown (losing parallelism) or letting nodeAgg run unmodified
above its parallel scan (losing the pushdown).
Heap and other AMs are unaffected: this commit only adds a single
create_upper_paths_hook call at a place where it wasn't being fired
before.
Reviewed-by: Bryan Green <[email protected]>
---
src/backend/optimizer/plan/planner.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index f4689e7c9f8..9b15e6342dd 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -8016,6 +8016,19 @@ create_partial_grouping_paths(PlannerInfo *root,
extra);
}
+ /*
+ * Let extensions possibly add some more partial paths, mirroring the
+ * FDW call above and the analogous extension hook on
+ * UPPERREL_PARTIAL_DISTINCT in create_partial_distinct_paths. Paths
+ * added here are picked up by the caller's subsequent
+ * gather_grouping_paths call and become candidates for the upstream
+ * Finalize Aggregate that add_paths_to_grouping_rel builds.
+ */
+ if (create_upper_paths_hook)
+ (*create_upper_paths_hook) (root, UPPERREL_PARTIAL_GROUP_AGG,
+ input_rel, partially_grouped_rel,
+ extra);
+
return partially_grouped_rel;
}
--
2.43.0
