From b4295b53194a4fb0f70ea2bb4ff97c920d8ce7e3 Mon Sep 17 00:00:00 2001 From: Jelte Fennema-Nio Date: Wed, 14 Oct 2026 21:36:06 +0200 Subject: [PATCH v3 5/5] postgres_fdw: Explain where remote tlist costing deliberately mimics the planner Without remote estimates, the costing of a pushed down aggregate is inaccurate in two places: remote target list expressions are only charged for the groups that pass the local HAVING quals, and remote expressions that are referenced more than once are counted more than once. Both look like mistakes, but create_agg_path() costs a local Agg the same way, and that's what the pushed down aggregate competes with. Fixing them only in postgres_fdw could make the planner fetch all input rows and aggregate locally, which is usually much slower. So this adds comments explaining that. Reported-by: Rui Zhao Discussion: https://postgr.es/m/DLRIUVXIEUOQ.2E2I0B7VZLHLI@jeltef.nl --- contrib/postgres_fdw/postgres_fdw.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index fae8de2dce1..8fe26e5151a 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -3720,7 +3720,18 @@ estimate_path_cost_size(PlannerInfo *root, run_cost += remote_cost.per_tuple * numGroups; } - /* Add in eval cost of the remotely computed tlist expressions */ + /* + * Add in eval cost of the remotely computed tlist expressions. + * + * The foreign server computes them for all retrieved rows, so + * using retrieved_rows here would be more accurate. But the + * planner only charges the target list of a local Agg for the + * rows that pass its HAVING quals (see create_agg_path()), and + * that's what this path competes with. So we mimic the planner's + * costing here, even though it's not completely accurate. + * Otherwise we can end up fetching all input rows and aggregating + * them locally, which is usually much slower. + */ startup_cost += remote_tlist_cost.startup; run_cost += remote_tlist_cost.per_tuple * rows; } @@ -3838,7 +3849,12 @@ estimate_path_cost_size(PlannerInfo *root, * only fetch their values. Their cost is already part of the remote * cost. It is either included in the EXPLAIN result (when using remote * estimates) or it was added by the heuristic upper relation costing - * above. So we subtract it here to avoid counting it twice. + * above. So we subtract it here to avoid counting it twice. If the + * target references such an expression more than once, e.g. inside a + * locally computed expression, it's still counted more than once. The + * planner costs the target list of a local Agg the same way (see + * create_agg_path()), so we mimic that here, even though it's not + * completely accurate. */ if (IS_UPPER_REL(foreignrel)) { -- 2.55.0