From 45382b0e23b1970aedb1791b6f15894e1d1e2d9d Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sat, 29 Aug 2026 17:10:34 +0000 Subject: [PATCH v5] 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 | 61 ++++++++++++++++++++++++++- src/test/regress/expected/cluster.out | 36 ++++++++++++++++ src/test/regress/sql/cluster.sql | 28 ++++++++++++ 5 files changed, 128 insertions(+), 5 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..875e735ea45 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -179,6 +179,7 @@ static List *get_tables_to_repack(RepackCommand cmd, bool usingindex, static List *get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel, MemoryContext permcxt); +static void warn_foreign_partition(RepackCommand cmd, Oid relid); static bool repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid, bool already_locked); @@ -2435,6 +2436,7 @@ get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel, inhoids = find_all_inheritors(relid, NoLock, NULL); foreach_oid(child_oid, inhoids) { + char relkind = get_rel_relkind(child_oid); Oid table_oid, index_oid; RelToCluster *rtc; @@ -2442,8 +2444,30 @@ get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel, if (rel_is_index) { + /* + * Foreign tables have no index partitions, so look for them among + * the partitions of each partitioned table in the tree. + */ + if (relkind == RELKIND_PARTITIONED_INDEX) + { + Oid parent_oid = IndexGetRelation(child_oid, true); + List *partoids; + + if (!OidIsValid(parent_oid)) + continue; + + partoids = find_inheritance_children(parent_oid, NoLock); + foreach_oid(part_oid, partoids) + { + if (get_rel_relkind(part_oid) == RELKIND_FOREIGN_TABLE) + warn_foreign_partition(stmt->command, part_oid); + } + list_free(partoids); + continue; + } + /* consider only leaf indexes */ - if (get_rel_relkind(child_oid) != RELKIND_INDEX) + if (relkind != RELKIND_INDEX) continue; /* @@ -2458,8 +2482,14 @@ get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel, } else { + if (relkind == RELKIND_FOREIGN_TABLE) + { + warn_foreign_partition(stmt->command, child_oid); + continue; + } + /* consider only leaf relations */ - if (get_rel_relkind(child_oid) != RELKIND_RELATION) + if (relkind != RELKIND_RELATION) continue; table_oid = child_oid; @@ -2490,6 +2520,33 @@ get_tables_to_repack_partitioned(RepackStmt *stmt, Relation rel, return rtcs; } +/* + * Warn that a foreign-table partition is skipped, as it has no local storage. + * + * Check privileges first, like VACUUM does, so that a user who may not + * process the partition gets the permission warning instead. We don't hold + * a lock on the partition's immediate parent if it's below the first level, + * so it may have been dropped concurrently. + */ +static void +warn_foreign_partition(RepackCommand cmd, Oid relid) +{ + char *relname; + + if (!repack_is_permitted_for_relation(cmd, relid, GetUserId(), false)) + return; + + relname = get_rel_name(relid); + if (relname == NULL) + return; + + ereport(WARNING, + /*- translator: second %s is the name of a SQL command, eg. REPACK */ + errmsg("skipping \"%s\" --- cannot execute %s on foreign tables", + relname, RepackCommandAsString(cmd))); + pfree(relname); +} + /* * Return whether userid has privileges to execute REPACK/CLUSTER on relid. diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out index 64a9c35fcd0..d99c90cced5 100644 --- a/src/test/regress/expected/cluster.out +++ b/src/test/regress/expected/cluster.out @@ -537,6 +537,42 @@ 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; +CREATE FOREIGN TABLE clstrpart121 PARTITION OF clstrpart12 FOR VALUES FROM (5) TO (10) SERVER clstr_fserv; +CLUSTER clstrpart USING clstrpart_idx; +WARNING: skipping "clstrpart4" --- cannot execute CLUSTER on foreign tables +WARNING: skipping "clstrpart121" --- cannot execute CLUSTER on foreign tables +REPACK clstrpart USING INDEX clstrpart_idx; +WARNING: skipping "clstrpart4" --- cannot execute REPACK on foreign tables +WARNING: skipping "clstrpart121" --- cannot execute REPACK on foreign tables +REPACK clstrpart; +WARNING: skipping "clstrpart4" --- cannot execute REPACK on foreign tables +WARNING: skipping "clstrpart121" --- 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, clstrpart121; +-- ... 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..eb0da1cf22e 100644 --- a/src/test/regress/sql/cluster.sql +++ b/src/test/regress/sql/cluster.sql @@ -248,6 +248,34 @@ 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; +CREATE FOREIGN TABLE clstrpart121 PARTITION OF clstrpart12 FOR VALUES FROM (5) TO (10) 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, clstrpart121; +-- ... 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.56.0