On Mon, Jun 29, 2026 at 2:21 PM shveta malik <[email protected]> wrote:
>
> IMO, we should silently skip CLT by expanding the check in
> AlterTableMoveAll() similar to system-tables.
>
Agreed, otherwise, it will break the entire bulk operation. I have
tried to fix this in the attached and modified the required tests. I
think we should merge 0001 and 0002, and move these tablespace related
tests to 0001.
--
With Regards,
Amit Kapila.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 973d3607838..a3cda01d80f 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -6924,7 +6924,11 @@ ATSimplePermissions(AlterTableType cmdtype, Relation
rel, int allowed_targets)
/*
* Conflict log tables are used internally for logical replication
conflict
* logging and should not be altered directly, as it could disrupt
conflict
- * logging.
+ * logging. Direct ALTER commands are already rejected during relation
+ * lookup in RangeVarCallbackForAlterRelation(), and AlterTableMoveAll()
+ * skips these tables, so a conflict log table does not normally reach
here;
+ * this check guards any internal caller that arrives via
+ * AlterTableInternal().
*/
if (IsConflictLogTableClass(rel->rd_rel))
ereport(ERROR,
@@ -17592,11 +17596,16 @@ AlterTableMoveAll(AlterTableMoveAllStmt *stmt)
*
* Also, explicitly avoid any shared tables, temp tables, or
TOAST
* (TOAST will be moved with the main table).
+ *
+ * Conflict log tables are system-managed for logical
replication and
+ * cannot be altered directly, so skip them as well; otherwise
a single
+ * such table in the source tablespace would abort the whole
bulk move.
*/
if (IsCatalogNamespace(relForm->relnamespace) ||
relForm->relisshared ||
isAnyTempNamespace(relForm->relnamespace) ||
- IsToastNamespace(relForm->relnamespace))
+ IsToastNamespace(relForm->relnamespace) ||
+ IsConflictLogTableNamespace(relForm->relnamespace))
continue;
/* Only move the object type requested */
diff --git a/src/test/subscription/t/035_conflicts.pl
b/src/test/subscription/t/035_conflicts.pl
index 86d3f8903c5..4260c4a5803 100644
--- a/src/test/subscription/t/035_conflicts.pl
+++ b/src/test/subscription/t/035_conflicts.pl
@@ -716,12 +716,23 @@ ok( $node_A->poll_query_until(
"the slot 'pg_conflict_detection' has been dropped on Node A");
###############################################################################
-# Verify that ATSimplePermissions blocks moving a CLT via
-# ALTER TABLE ALL IN TABLESPACE. We use an isolated database containing only
-# the CLT so no other tables are moved before the error fires.
+# A conflict log table is system-managed and cannot be altered directly, so
+# moving it to another tablespace must be rejected.
+###############################################################################
+
+(undef, undef, $stderr) = $node_subscriber->psql('postgres',
+ "ALTER TABLE $clt SET TABLESPACE pg_default");
+like(
+ $stderr,
+ qr/permission denied: "pg_conflict_log_\d+" is a conflict log table/,
+ "moving a conflict log table with ALTER TABLE SET TABLESPACE is
rejected");
+
+###############################################################################
+# ALTER TABLE ALL IN TABLESPACE must skip conflict log tables, the same way it
+# skips catalog and TOAST tables, instead of failing. Use an isolated database
+# so the bulk move only touches the objects created here.
###############################################################################
-# Create an isolated database with a single CLT and no user tables.
$node_subscriber->safe_psql('postgres', "CREATE DATABASE clt_ts_test");
$node_subscriber->safe_psql('clt_ts_test',
"CREATE SUBSCRIPTION sub_ts_test
@@ -729,6 +740,9 @@ $node_subscriber->safe_psql('clt_ts_test',
PUBLICATION pub
WITH (connect=false, conflict_log_destination='table')");
+# A plain user table that should be moved, alongside the CLT that must not be.
+$node_subscriber->safe_psql('clt_ts_test', "CREATE TABLE user_tbl (i int)");
+
# Create a tablespace backed by a directory inside the data dir.
my $ts_dir = $node_subscriber->data_dir . '/backup_space';
mkdir($ts_dir)
@@ -736,23 +750,31 @@ mkdir($ts_dir)
$node_subscriber->safe_psql('postgres',
"CREATE TABLESPACE backup_space LOCATION '$ts_dir'");
-# The CLT is the only non-system table in pg_default of clt_ts_test, so
-# ALTER TABLE ALL IN TABLESPACE hits it immediately and ATSimplePermissions
-# raises the expected error.
-(undef, undef, $stderr) = $node_subscriber->psql('clt_ts_test',
+# The bulk move succeeds: the user table is relocated while the CLT is skipped.
+$node_subscriber->safe_psql('clt_ts_test',
"ALTER TABLE ALL IN TABLESPACE pg_default SET TABLESPACE backup_space");
-like(
- $stderr,
- qr/permission denied: "pg_conflict_log_\d+" is a conflict log table/,
- "ATSimplePermissions blocks moving CLT via ALTER TABLE ALL IN
TABLESPACE");
-# Cleanup
+is( $node_subscriber->safe_psql(
+ 'clt_ts_test',
+ "SELECT reltablespace <> 0 FROM pg_class WHERE relname =
'user_tbl'"),
+ 't',
+ "ALTER TABLE ALL IN TABLESPACE moves an ordinary user table");
+
+is( $node_subscriber->safe_psql(
+ 'clt_ts_test',
+ "SELECT count(*) FROM pg_class c JOIN pg_subscription s
+ ON c.relname = 'pg_conflict_log_' || s.oid
+ WHERE s.subname = 'sub_ts_test' AND c.reltablespace <> 0"),
+ '0',
+ "ALTER TABLE ALL IN TABLESPACE skips the conflict log table");
+
+# Cleanup. The subscription has no real publisher connection, so detach its
+# slot before dropping it.
$node_subscriber->safe_psql('clt_ts_test',
"ALTER SUBSCRIPTION sub_ts_test DISABLE");
$node_subscriber->safe_psql('clt_ts_test',
"ALTER SUBSCRIPTION sub_ts_test SET (slot_name = NONE)");
-$node_subscriber->safe_psql('clt_ts_test',
- "DROP SUBSCRIPTION sub_ts_test");
+$node_subscriber->safe_psql('clt_ts_test', "DROP SUBSCRIPTION sub_ts_test");
$node_subscriber->safe_psql('postgres', "DROP DATABASE clt_ts_test");
$node_subscriber->safe_psql('postgres', "DROP TABLESPACE backup_space");
