Starting a dedicated thread for this, based on [1]. To recap: - FOR PORTION OF needs to reject FDWs in the planner/executor, not in analysis, first to guarantee the status hasn't changed but also because we need to check child partitions as well. - We need to postpone checking for volatile functions too.
The first part is done by jian he's patch from that thread (with some test revisions). The second part is done by the second patch here. There is no commitfest entry yet, so I'll make one from this thread. [1] https://www.postgresql.org/message-id/[email protected] Yours, -- Paul ~{:-) [email protected]
From 15aedd144131149556a16bcf14c78e776c116b6b Mon Sep 17 00:00:00 2001 From: jian he <[email protected]> Date: Fri, 24 Apr 2026 12:43:50 +0800 Subject: [PATCH v8 1/2] Reject child partition FDWs in FOR PORTION OF We should defer validating FDW usage until after analysis. We have to guard against not just the topmost table, but also individual child partitions. Added the check to CheckValidResultRel, because it is called after looking up child partitions (accounting for pruning), but before the FDW can run a DirectModify update, which would bypass per-tuple executor work. discussion: https://postgr.es/m/626986.1776785090%40sss.pgh.pa.us commitfest entry: https://commitfest.postgresql.org/patch/ --- .../postgres_fdw/expected/postgres_fdw.out | 38 +++++++++++++++++-- contrib/postgres_fdw/sql/postgres_fdw.sql | 30 +++++++++++++-- src/backend/commands/copyfrom.c | 2 +- src/backend/executor/execMain.c | 9 ++++- src/backend/executor/execPartition.c | 4 +- src/backend/executor/nodeModifyTable.c | 2 +- src/backend/parser/analyze.c | 6 --- src/include/executor/executor.h | 2 +- 8 files changed, 75 insertions(+), 18 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index e90289e4ab1..97f01353fd2 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -6335,8 +6335,8 @@ DELETE FROM ft2 WHERE c1 = 1200 RETURNING tableoid::regclass; -- Test UPDATE FOR PORTION OF UPDATE ft8 FOR PORTION OF c4 FROM '2005-01-01' TO '2006-01-01' -SET c2 = c2 + 1 -WHERE c1 = '[1,2)'; + SET c2 = c2 + 1 + WHERE c1 = '[1,2)'; -- error ERROR: foreign tables don't support FOR PORTION OF SELECT * FROM ft8 WHERE c1 = '[1,2)' ORDER BY c1, c4; c1 | c2 | c3 | c4 @@ -6346,7 +6346,7 @@ SELECT * FROM ft8 WHERE c1 = '[1,2)' ORDER BY c1, c4; -- Test DELETE FOR PORTION OF DELETE FROM ft8 FOR PORTION OF c4 FROM '2005-01-01' TO '2006-01-01' -WHERE c1 = '[2,3)'; + WHERE c1 = '[2,3)'; -- error ERROR: foreign tables don't support FOR PORTION OF SELECT * FROM ft8 WHERE c1 = '[2,3)' ORDER BY c1, c4; c1 | c2 | c3 | c4 @@ -6354,6 +6354,38 @@ SELECT * FROM ft8 WHERE c1 = '[2,3)' ORDER BY c1, c4; [2,3) | 3 | AAA002 | [01-01-2000,01-01-2020) (1 row) +-- FOR PORTION OF fails if a child partition is a foreign table, even if the +-- root is not. But a child partition that is pruned doesn't cause an error. +CREATE TABLE fpo_part_parent ( + c1 int4range NOT NULL, + c2 int NOT NULL, + c3 text, + c4 daterange NOT NULL +) PARTITION BY LIST (c2); +CREATE TABLE fpo_part_local PARTITION OF fpo_part_parent FOR VALUES IN (1); +INSERT INTO fpo_part_local VALUES ('[1,2)', 1, 'one', '[2024-01-01,2024-12-31)'); +CREATE FOREIGN TABLE fpo_part_foreign + PARTITION OF fpo_part_parent FOR VALUES IN (6) + SERVER loopback OPTIONS (schema_name 'S 1', table_name 'T 5'); +DELETE FROM fpo_part_parent + FOR PORTION OF c4 FROM '2001-01-01' TO '2001-02-01' WHERE c2 = 6; -- error +ERROR: foreign tables don't support FOR PORTION OF +UPDATE fpo_part_parent + FOR PORTION OF c4 FROM '2001-01-01' TO '2001-02-01' SET c3 = 'x' WHERE c2 = 6; -- error +ERROR: foreign tables don't support FOR PORTION OF +UPDATE fpo_part_parent + FOR PORTION OF c4 FROM '2024-06-01' TO '2024-07-01' SET c3 = 'edited' WHERE c2 = 1; -- okay +DELETE FROM fpo_part_parent + FOR PORTION OF c4 FROM '2024-06-01' TO '2024-06-15' WHERE c2 = 1; -- okay +SELECT c1, c2, c3, c4 FROM fpo_part_local ORDER BY c4; + c1 | c2 | c3 | c4 +-------+----+--------+------------------------- + [1,2) | 1 | one | [01-01-2024,06-01-2024) + [1,2) | 1 | edited | [06-15-2024,07-01-2024) + [1,2) | 1 | one | [07-01-2024,12-31-2024) +(3 rows) + +DROP TABLE fpo_part_parent; -- Test UPDATE/DELETE with RETURNING on a three-table join INSERT INTO ft2 (c1,c2,c3) SELECT id, id - 1200, to_char(id, 'FM00000') FROM generate_series(1201, 1300) id; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index dfc58beb0d2..73303837a47 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -1578,15 +1578,39 @@ DELETE FROM ft2 WHERE c1 = 1200 RETURNING tableoid::regclass; -- Test UPDATE FOR PORTION OF UPDATE ft8 FOR PORTION OF c4 FROM '2005-01-01' TO '2006-01-01' -SET c2 = c2 + 1 -WHERE c1 = '[1,2)'; + SET c2 = c2 + 1 + WHERE c1 = '[1,2)'; -- error SELECT * FROM ft8 WHERE c1 = '[1,2)' ORDER BY c1, c4; -- Test DELETE FOR PORTION OF DELETE FROM ft8 FOR PORTION OF c4 FROM '2005-01-01' TO '2006-01-01' -WHERE c1 = '[2,3)'; + WHERE c1 = '[2,3)'; -- error SELECT * FROM ft8 WHERE c1 = '[2,3)' ORDER BY c1, c4; +-- FOR PORTION OF fails if a child partition is a foreign table, even if the +-- root is not. But a child partition that is pruned doesn't cause an error. +CREATE TABLE fpo_part_parent ( + c1 int4range NOT NULL, + c2 int NOT NULL, + c3 text, + c4 daterange NOT NULL +) PARTITION BY LIST (c2); +CREATE TABLE fpo_part_local PARTITION OF fpo_part_parent FOR VALUES IN (1); +INSERT INTO fpo_part_local VALUES ('[1,2)', 1, 'one', '[2024-01-01,2024-12-31)'); +CREATE FOREIGN TABLE fpo_part_foreign + PARTITION OF fpo_part_parent FOR VALUES IN (6) + SERVER loopback OPTIONS (schema_name 'S 1', table_name 'T 5'); +DELETE FROM fpo_part_parent + FOR PORTION OF c4 FROM '2001-01-01' TO '2001-02-01' WHERE c2 = 6; -- error +UPDATE fpo_part_parent + FOR PORTION OF c4 FROM '2001-01-01' TO '2001-02-01' SET c3 = 'x' WHERE c2 = 6; -- error +UPDATE fpo_part_parent + FOR PORTION OF c4 FROM '2024-06-01' TO '2024-07-01' SET c3 = 'edited' WHERE c2 = 1; -- okay +DELETE FROM fpo_part_parent + FOR PORTION OF c4 FROM '2024-06-01' TO '2024-06-15' WHERE c2 = 1; -- okay +SELECT c1, c2, c3, c4 FROM fpo_part_local ORDER BY c4; +DROP TABLE fpo_part_parent; + -- Test UPDATE/DELETE with RETURNING on a three-table join INSERT INTO ft2 (c1,c2,c3) SELECT id, id - 1200, to_char(id, 'FM00000') FROM generate_series(1201, 1300) id; diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 64ac3063c61..775b1b5a641 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -921,7 +921,7 @@ CopyFrom(CopyFromState cstate) ExecInitResultRelation(estate, resultRelInfo, 1); /* Verify the named relation is a valid target for INSERT */ - CheckValidResultRel(resultRelInfo, CMD_INSERT, ONCONFLICT_NONE, NIL); + CheckValidResultRel(resultRelInfo, CMD_INSERT, ONCONFLICT_NONE, NIL, NULL); ExecOpenIndices(resultRelInfo, false); diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 4b30f768680..06de3a4461c 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -1063,7 +1063,8 @@ InitPlan(QueryDesc *queryDesc, int eflags) */ void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, - OnConflictAction onConflictAction, List *mergeActions) + OnConflictAction onConflictAction, List *mergeActions, + ModifyTable *mtnode) { Relation resultRel = resultRelInfo->ri_RelationDesc; FdwRoutine *fdwroutine; @@ -1126,6 +1127,12 @@ CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, RelationGetRelationName(resultRel)))); break; case RELKIND_FOREIGN_TABLE: + /* We don't support FOR PORTION OF FDW queries. */ + if (mtnode && mtnode->forPortionOf) + ereport(ERROR, + errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("foreign tables don't support FOR PORTION OF")); + /* Okay only if the FDW supports it */ fdwroutine = resultRelInfo->ri_FdwRoutine; switch (operation) diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c index d96d4f9947b..33ec5bfde4c 100644 --- a/src/backend/executor/execPartition.c +++ b/src/backend/executor/execPartition.c @@ -368,7 +368,7 @@ ExecFindPartition(ModifyTableState *mtstate, /* Verify this ResultRelInfo allows INSERTs */ CheckValidResultRel(rri, CMD_INSERT, node ? node->onConflictAction : ONCONFLICT_NONE, - NIL); + NIL, node); /* * Initialize information needed to insert this and @@ -594,7 +594,7 @@ ExecInitPartitionInfo(ModifyTableState *mtstate, EState *estate, * required when the operation is CMD_UPDATE. */ CheckValidResultRel(leaf_part_rri, CMD_INSERT, - node ? node->onConflictAction : ONCONFLICT_NONE, NIL); + node ? node->onConflictAction : ONCONFLICT_NONE, NIL, node); /* * Open partition indices. The user may have asked to check for conflicts diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c index 478cb01783c..d953a4c2e1b 100644 --- a/src/backend/executor/nodeModifyTable.c +++ b/src/backend/executor/nodeModifyTable.c @@ -5297,7 +5297,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags) * Verify result relation is a valid target for the current operation */ CheckValidResultRel(resultRelInfo, operation, node->onConflictAction, - mergeActions); + mergeActions, node); resultRelInfo++; i++; diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index ffcf25a6be7..04bae9939b9 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -1335,12 +1335,6 @@ transformForPortionOfClause(ParseState *pstate, ForPortionOfExpr *result; Var *rangeVar; - /* We don't support FOR PORTION OF FDW queries. */ - if (targetrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("foreign tables don't support FOR PORTION OF"))); - result = makeNode(ForPortionOfExpr); /* Look up the FOR PORTION OF name requested. */ diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index 33bbdbfeffb..c690b230b45 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -249,7 +249,7 @@ extern bool ExecCheckPermissions(List *rangeTable, extern bool ExecCheckOneRelPerms(RTEPermissionInfo *perminfo); extern void CheckValidResultRel(ResultRelInfo *resultRelInfo, CmdType operation, OnConflictAction onConflictAction, - List *mergeActions); + List *mergeActions, ModifyTable *mtnode); extern void InitResultRelInfo(ResultRelInfo *resultRelInfo, Relation resultRelationDesc, Index resultRelationIndex, -- 2.47.3
From 5f4acf427b443dd85b1cdfe2654d6714a95821e5 Mon Sep 17 00:00:00 2001 From: "Paul A. Jungwirth" <[email protected]> Date: Fri, 15 May 2026 13:43:10 -0700 Subject: [PATCH v8 2/2] Move FOR PORTION OF volatile check into planner Like checking for GENERATED columns, this needs to be wary of the function volatility changing after we check it. It's easy to do that if FOR PORTION OF appears inside a BEGIN ATOMIC SQL function. --- src/backend/optimizer/plan/planner.c | 11 +++++++++++ src/backend/parser/analyze.c | 3 --- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index f4689e7c9f8..df6ac74ffa9 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -1081,6 +1081,17 @@ subquery_planner(PlannerGlobal *glob, Query *parse, char *plan_name, /* exclRelTlist contains only Vars, so no preprocessing needed */ } + if (parse->forPortionOf) + { + parse->forPortionOf->targetRange = + preprocess_expression(root, + parse->forPortionOf->targetRange, + EXPRKIND_TARGET); + if (contain_volatile_functions(parse->forPortionOf->targetRange)) + ereport(ERROR, + (errmsg("FOR PORTION OF bounds cannot contain volatile functions"))); + } + foreach(l, parse->mergeActionList) { MergeAction *action = (MergeAction *) lfirst(l); diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 04bae9939b9..285c9817b1c 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -1483,9 +1483,6 @@ transformForPortionOfClause(ParseState *pstate, args, InvalidOid, InvalidOid, COERCE_EXPLICIT_CALL); } - if (contain_volatile_functions_after_planning((Expr *) result->targetRange)) - ereport(ERROR, - (errmsg("FOR PORTION OF bounds cannot contain volatile functions"))); /* * Build overlapsExpr to use as an extra qual. This means we only hit rows -- 2.47.3
