From 7f38d9c4476239777aa902c5381a7859d0056e4f Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sat, 29 Aug 2026 17:10:34 +0000 Subject: [PATCH v4] Warn when REPACK or CLUSTER skips a foreign-table partition Processing a partitioned table silently skipped partitions that are foreign tables, both when expanding the table's partition tree and in the USING INDEX case, where foreign tables cannot appear in the index tree at all. That is inconsistent with the surrounding behavior: VACUUM FULL warns about such partitions, and a direct REPACK or CLUSTER of a foreign table is an error. Emit a warning, like VACUUM does. --- doc/src/sgml/ref/cluster.sgml | 5 +++-- doc/src/sgml/ref/repack.sgml | 3 ++- src/backend/commands/repack.c | 26 +++++++++++++++++++++- src/test/regress/expected/cluster.out | 32 +++++++++++++++++++++++++++ src/test/regress/sql/cluster.sql | 27 ++++++++++++++++++++++ 5 files changed, 89 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index ffb3ff898c6..77069fd6f02 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -118,8 +118,9 @@ CLUSTER [ ( option [, ...] ) ] [ Clustering a partitioned table clusters each of its partitions using the partition of the specified partitioned index. When clustering a partitioned - table, the index may not be omitted. CLUSTER on a - partitioned table cannot be executed inside a transaction block. + table, the index may not be omitted. Partitions that are foreign tables + are skipped. CLUSTER on a partitioned table cannot be + executed inside a transaction block. diff --git a/doc/src/sgml/ref/repack.sgml b/doc/src/sgml/ref/repack.sgml index 14651715013..4cd3cfa0b11 100644 --- a/doc/src/sgml/ref/repack.sgml +++ b/doc/src/sgml/ref/repack.sgml @@ -202,7 +202,8 @@ REPACK [ ( option [, ...] ) ] USING Repacking a partitioned table repacks each of its partitions. If an index is specified, each partition is repacked using the partition of that - index. REPACK on a partitioned table cannot be executed + index. Partitions that are foreign tables are skipped. + REPACK on a partitioned table cannot be executed inside a transaction block. diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index a584e138ada..9bc1922c610 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -2432,7 +2432,31 @@ get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel, * Do not lock the children until they're processed. Note that we do hold * a lock on the parent partitioned table. */ - inhoids = find_all_inheritors(relid, NoLock, NULL); + inhoids = find_all_inheritors(RelationGetRelid(rel), NoLock, NULL); + + /* + * Warn about foreign-table partitions, which have no local storage and + * would otherwise be skipped silently. This has to look at the table's + * partition tree: when an index is specified, the walk below covers the + * index's partition tree, which cannot contain foreign tables. Check + * privileges first, like VACUUM does, so that a user who may not process + * the partition gets the permission warning instead. + */ + foreach_oid(tableoid, inhoids) + { + if (get_rel_relkind(tableoid) == RELKIND_FOREIGN_TABLE && + repack_is_permitted_for_relation(stmt->command, tableoid, + GetUserId(), false)) + ereport(WARNING, + /*- translator: second %s is the name of a SQL command, eg. REPACK */ + errmsg("skipping \"%s\" --- cannot execute %s on foreign tables", + get_rel_name(tableoid), + RepackCommandAsString(stmt->command))); + } + + if (rel_is_index) + inhoids = find_all_inheritors(relid, NoLock, NULL); + foreach_oid(child_oid, inhoids) { Oid table_oid, diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 64a9c35fcd0..14ca0aeabe8 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -537,6 +537,38 @@ SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM o clstrpart33 | 2 | r | f (7 rows) +-- Check that partitions that are foreign tables are skipped, with a warning +CREATE FOREIGN DATA WRAPPER clstr_fdw; +CREATE SERVER clstr_fserv FOREIGN DATA WRAPPER clstr_fdw; +CREATE FOREIGN TABLE clstrpart4 PARTITION OF clstrpart FOR VALUES FROM (20) TO (30) SERVER clstr_fserv; +CLUSTER clstrpart USING clstrpart_idx; +WARNING: skipping "clstrpart4" --- cannot execute CLUSTER on foreign tables +REPACK clstrpart USING INDEX clstrpart_idx; +WARNING: skipping "clstrpart4" --- cannot execute REPACK on foreign tables +REPACK clstrpart; +WARNING: skipping "clstrpart4" --- cannot execute REPACK on foreign tables +-- ... while processing a foreign table directly is still an error +REPACK clstrpart4; +ERROR: "clstrpart4" is not a table or materialized view +DROP FOREIGN TABLE clstrpart4; +-- ... and privileges are checked first, giving one warning per partition +CREATE TABLE clstrfpart (a int) PARTITION BY RANGE (a); +CREATE INDEX clstrfpart_idx ON clstrfpart (a); +CREATE TABLE clstrfpart1 PARTITION OF clstrfpart FOR VALUES FROM (0) TO (10); +CREATE FOREIGN TABLE clstrfpart2 PARTITION OF clstrfpart FOR VALUES FROM (10) TO (20) SERVER clstr_fserv; +CREATE ROLE regress_clstr_fowner; +ALTER TABLE clstrfpart OWNER TO regress_clstr_fowner; +ALTER TABLE clstrfpart1 OWNER TO regress_clstr_fowner; +SET SESSION AUTHORIZATION regress_clstr_fowner; +REPACK clstrfpart; +WARNING: permission denied to execute REPACK on "clstrfpart2", skipping it +REPACK clstrfpart USING INDEX clstrfpart_idx; +WARNING: permission denied to execute REPACK on "clstrfpart2", skipping it +RESET SESSION AUTHORIZATION; +DROP TABLE clstrfpart; +DROP ROLE regress_clstr_fowner; +DROP SERVER clstr_fserv; +DROP FOREIGN DATA WRAPPER clstr_fdw; -- Ownership of partitions is checked CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql index 8c8df698b31..7a802a95ade 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -248,6 +248,33 @@ REPACK clstrpart; CREATE TEMP TABLE new_cluster_info AS SELECT relname, level, relfilenode, relkind FROM pg_partition_tree('clstrpart'::regclass) AS tree JOIN pg_class c ON c.oid=tree.relid ; SELECT relname, old.level, old.relkind, old.relfilenode = new.relfilenode FROM old_cluster_info AS old JOIN new_cluster_info AS new USING (relname) ORDER BY relname COLLATE "C"; +-- Check that partitions that are foreign tables are skipped, with a warning +CREATE FOREIGN DATA WRAPPER clstr_fdw; +CREATE SERVER clstr_fserv FOREIGN DATA WRAPPER clstr_fdw; +CREATE FOREIGN TABLE clstrpart4 PARTITION OF clstrpart FOR VALUES FROM (20) TO (30) SERVER clstr_fserv; +CLUSTER clstrpart USING clstrpart_idx; +REPACK clstrpart USING INDEX clstrpart_idx; +REPACK clstrpart; +-- ... while processing a foreign table directly is still an error +REPACK clstrpart4; +DROP FOREIGN TABLE clstrpart4; +-- ... and privileges are checked first, giving one warning per partition +CREATE TABLE clstrfpart (a int) PARTITION BY RANGE (a); +CREATE INDEX clstrfpart_idx ON clstrfpart (a); +CREATE TABLE clstrfpart1 PARTITION OF clstrfpart FOR VALUES FROM (0) TO (10); +CREATE FOREIGN TABLE clstrfpart2 PARTITION OF clstrfpart FOR VALUES FROM (10) TO (20) SERVER clstr_fserv; +CREATE ROLE regress_clstr_fowner; +ALTER TABLE clstrfpart OWNER TO regress_clstr_fowner; +ALTER TABLE clstrfpart1 OWNER TO regress_clstr_fowner; +SET SESSION AUTHORIZATION regress_clstr_fowner; +REPACK clstrfpart; +REPACK clstrfpart USING INDEX clstrfpart_idx; +RESET SESSION AUTHORIZATION; +DROP TABLE clstrfpart; +DROP ROLE regress_clstr_fowner; +DROP SERVER clstr_fserv; +DROP FOREIGN DATA WRAPPER clstr_fdw; + -- Ownership of partitions is checked CREATE TABLE ptnowner(i int unique not null) PARTITION BY LIST (i); CREATE INDEX ptnowner_i_idx ON ptnowner(i); -- 2.55.0