From 0b86578d2f0f5841b717ab8879f3732a5b33d7a0 Mon Sep 17 00:00:00 2001 From: Rui Zhao Date: Wed, 7 Oct 2026 19:26:56 +0800 Subject: [PATCH 2/2] Fix duplicate charges for remote expressions in local projections A local target can refer to the same remotely computed group key several times. Subtracting the remote target cost once still charges these references as local evaluations. Replace matching expressions with Vars in a copy used for costing, so only the remaining local work is charged. Keep constants as constants. Add coverage with local and remote estimates. --- .../postgres_fdw/expected/postgres_fdw.out | 56 +++++++++++++++++++ contrib/postgres_fdw/postgres_fdw.c | 40 ++++++++++--- contrib/postgres_fdw/sql/postgres_fdw.sql | 46 +++++++++++++++ 3 files changed, 134 insertions(+), 8 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 476f7ba7abb..7789668bbe4 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -367,6 +367,62 @@ FROM fdw_cost_plan('SELECT c2, remote_value(sum(c1)), count(*) FROM ft1 Foreign Scan | t (1 row) +-- Repeated references to a fetched group key need no local evaluation. +CREATE FUNCTION remote_key(int) RETURNS int +LANGUAGE plpgsql IMMUTABLE COST 100 AS $$ +BEGIN + RETURN $1; +END +$$; +ALTER EXTENSION postgres_fdw ADD FUNCTION remote_key(int); +ALTER FUNCTION local_project(int) COST 9; +SELECT (plan->>'Startup Cost')::numeric AS single_startup, + (plan->>'Total Cost')::numeric AS single_total, + (plan->>'Plan Rows')::numeric AS group_rows, + plan->>'Remote SQL' AS group_sql +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), count(*) FROM ft1 + GROUP BY remote_key(c2) ORDER BY remote_key(c2)') AS plan +\gset +SELECT plan->>'Node Type' AS node_type, + (plan->>'Startup Cost')::numeric = :single_startup AS same_startup, + plan->>'Remote SQL' = :'group_sql' AS same_remote_sql, + (plan->>'Total Cost')::numeric - :single_total = + :group_rows * 10 * current_setting('cpu_operator_cost')::numeric + AS only_local_cost +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), + local_project(remote_key(c2) + 1), count(*) + FROM ft1 GROUP BY remote_key(c2) + ORDER BY remote_key(c2)') AS plan; + node_type | same_startup | same_remote_sql | only_local_cost +--------------+--------------+-----------------+----------------- + Foreign Scan | t | t | t +(1 row) + +-- Check the same projection with remote estimates. +SELECT (plan->>'Startup Cost')::numeric AS single_startup, + (plan->>'Total Cost')::numeric AS single_total, + (plan->>'Plan Rows')::numeric AS group_rows, + plan->>'Remote SQL' AS group_sql +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), count(*) FROM ft2 + GROUP BY remote_key(c2) ORDER BY remote_key(c2)') AS plan +\gset +SELECT plan->>'Node Type' AS node_type, + (plan->>'Startup Cost')::numeric = :single_startup AS same_startup, + plan->>'Remote SQL' = :'group_sql' AS same_remote_sql, + (plan->>'Total Cost')::numeric - :single_total = + :group_rows * 10 * current_setting('cpu_operator_cost')::numeric + AS only_local_cost +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), + local_project(remote_key(c2) + 1), count(*) + FROM ft2 GROUP BY remote_key(c2) + ORDER BY remote_key(c2)') AS plan; + node_type | same_startup | same_remote_sql | only_local_cost +--------------+--------------+-----------------+----------------- + Foreign Scan | t | t | t +(1 row) + +ALTER EXTENSION postgres_fdw DROP FUNCTION remote_key(int); +DROP FUNCTION remote_key(int); DROP FUNCTION fdw_cost_plan(text); ALTER EXTENSION postgres_fdw DROP FUNCTION remote_value(bigint); DROP FUNCTION remote_value(bigint); diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index f46d453332e..4fb300e93e8 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -525,6 +525,7 @@ static void estimate_path_cost_size(PlannerInfo *root, double *p_rows, int *p_width, int *p_disabled_nodes, Cost *p_startup_cost, Cost *p_total_cost); +static Node *replace_remote_projection_exprs(Node *node, List *remote_tlist); static void get_remote_estimate(const char *sql, PGconn *conn, double *rows, @@ -3828,23 +3829,26 @@ estimate_path_cost_size(PlannerInfo *root, target = fpextra->target; else target = foreignrel->reltarget; - startup_cost += target->cost.startup; - run_cost += target->cost.per_tuple * rows; /* * The planner computed the cost of the target as if we evaluate all of * its expressions locally. That is not true for an upper relation. There * the foreign server computes the expressions in grouped_tlist and we - * 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. + * only fetch their values. Replace references to those expressions with + * Vars in a copy of the target before costing the local work. Subtracting + * the grouped_tlist cost once would still charge repeated references. */ + local_cost = target->cost; if (IS_UPPER_REL(foreignrel)) { - startup_cost -= remote_tlist_cost.startup; - run_cost -= remote_tlist_cost.per_tuple * rows; + List *local_exprs; + + local_exprs = (List *) replace_remote_projection_exprs((Node *) target->exprs, + fpinfo->grouped_tlist); + cost_qual_eval(&local_cost, local_exprs, root); } + startup_cost += local_cost.startup; + run_cost += local_cost.per_tuple * rows; /* * If we have LIMIT, we should prefer performing the restriction remotely @@ -3877,6 +3881,26 @@ estimate_path_cost_size(PlannerInfo *root, *p_total_cost = startup_cost + run_cost; } +/* Replace fetched expressions with zero-cost Vars in a copy used for costing. */ +static Node * +replace_remote_projection_exprs(Node *node, List *remote_tlist) +{ + TargetEntry *tle; + + if (node == NULL) + return NULL; + + if (IsA(node, Const)) + return copyObject(node); + + tle = tlist_member((Expr *) node, remote_tlist); + if (tle) + return (Node *) makeVarFromTargetEntry(INDEX_VAR, tle); + + return expression_tree_mutator(node, replace_remote_projection_exprs, + remote_tlist); +} + /* * Estimate costs of executing a SQL statement remotely. * The given "sql" must be an EXPLAIN command. diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 6872a75ef4e..eb3c0998155 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -315,6 +315,52 @@ SELECT plan->>'Node Type' AS node_type, FROM fdw_cost_plan('SELECT c2, remote_value(sum(c1)), count(*) FROM ft1 GROUP BY c2 HAVING local_filter(count(*)::int) ORDER BY c2') AS plan; +-- Repeated references to a fetched group key need no local evaluation. +CREATE FUNCTION remote_key(int) RETURNS int +LANGUAGE plpgsql IMMUTABLE COST 100 AS $$ +BEGIN + RETURN $1; +END +$$; +ALTER EXTENSION postgres_fdw ADD FUNCTION remote_key(int); +ALTER FUNCTION local_project(int) COST 9; +SELECT (plan->>'Startup Cost')::numeric AS single_startup, + (plan->>'Total Cost')::numeric AS single_total, + (plan->>'Plan Rows')::numeric AS group_rows, + plan->>'Remote SQL' AS group_sql +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), count(*) FROM ft1 + GROUP BY remote_key(c2) ORDER BY remote_key(c2)') AS plan +\gset +SELECT plan->>'Node Type' AS node_type, + (plan->>'Startup Cost')::numeric = :single_startup AS same_startup, + plan->>'Remote SQL' = :'group_sql' AS same_remote_sql, + (plan->>'Total Cost')::numeric - :single_total = + :group_rows * 10 * current_setting('cpu_operator_cost')::numeric + AS only_local_cost +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), + local_project(remote_key(c2) + 1), count(*) + FROM ft1 GROUP BY remote_key(c2) + ORDER BY remote_key(c2)') AS plan; +-- Check the same projection with remote estimates. +SELECT (plan->>'Startup Cost')::numeric AS single_startup, + (plan->>'Total Cost')::numeric AS single_total, + (plan->>'Plan Rows')::numeric AS group_rows, + plan->>'Remote SQL' AS group_sql +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), count(*) FROM ft2 + GROUP BY remote_key(c2) ORDER BY remote_key(c2)') AS plan +\gset +SELECT plan->>'Node Type' AS node_type, + (plan->>'Startup Cost')::numeric = :single_startup AS same_startup, + plan->>'Remote SQL' = :'group_sql' AS same_remote_sql, + (plan->>'Total Cost')::numeric - :single_total = + :group_rows * 10 * current_setting('cpu_operator_cost')::numeric + AS only_local_cost +FROM fdw_cost_plan('SELECT local_project(remote_key(c2)), + local_project(remote_key(c2) + 1), count(*) + FROM ft2 GROUP BY remote_key(c2) + ORDER BY remote_key(c2)') AS plan; +ALTER EXTENSION postgres_fdw DROP FUNCTION remote_key(int); +DROP FUNCTION remote_key(int); DROP FUNCTION fdw_cost_plan(text); ALTER EXTENSION postgres_fdw DROP FUNCTION remote_value(bigint); DROP FUNCTION remote_value(bigint);