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.

Author: jian he <jian.universality@gmail.com>
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Paul A. Jungwirth <pj@illuminatedcomputing.com>
Discussion: https://www.postgresql.org/message-id/flat/CA%2BrenyUte0_UJsJiDJQi82oaBsMJn%3Dcct0Wn%3DvOqXtuDn%3DYYJA%40mail.gmail.com

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 13853b8b72079852c7c088581a904cb727373a5d..0805c56cb1ba5b6f875528b6f4c9631dc4512a71 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -6335,9 +6335,10 @@ 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
+DETAIL:  "ft8" is a foreign table.
 SELECT * FROM ft8 WHERE c1 = '[1,2)' ORDER BY c1, c4;
   c1   | c2 |   c3   |           c4            
 -------+----+--------+-------------------------
@@ -6346,14 +6347,49 @@ 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
+DETAIL:  "ft8" is a foreign table.
 SELECT * FROM ft8 WHERE c1 = '[2,3)' ORDER BY c1, c4;
   c1   | c2 |   c3   |           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
+DETAIL:  "fpo_part_foreign" is a foreign table.
+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
+DETAIL:  "fpo_part_foreign" is a foreign table.
+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 697c4a92e2d036d595206b154915c403d0ba648d..8162c5496bfd29ef50f050d955d3ad4e7c2a4abd 100644 (file)
--- 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 0087585b2c4e118f7d84d8492afb1e849e88b0ae..80a527ed4c6422734f7e335d30fd12cf2817d61f 100644 (file)
--- 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 4b30f7686801a3e9ce75fe17a3bf0a7605f08e49..c3af96989ba3ebad8f6c90bb7cc86763b263a3ee 100644 (file)
--- 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,14 @@ 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"),
+                                               errdetail("\"%s\" is a foreign table.",
+                                                                 RelationGetRelationName(resultRel)));
+
                        /* 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 d96d4f9947b79227c59ae693404509ad3be9cbe9..33ec5bfde4c4dd99493d2ae089da1ccd028c2f02 100644 (file)
--- 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 846dc516b436611ea5a6ac4b9e25f77de2380e7f..c333d7139fae0723d70ace7cf18f82f14e66890b 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -5312,7 +5312,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 76758adefb6f18b2539d249acd8b256c550d9153..dc65a505c16572ee749e27cf4bb9acbb8694dbde 100644 (file)
--- 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 650baab3efcaad26e579ac1becd73f0bf20b5860..1798e6027d4c20301bef93329b6c84a45fc57cc0 100644 (file)
--- 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,