Re: Proposal: Conflict log history table for Logical Replication

Thanks for the patches.

I have given more thoughts on below query which we also discussed in
[1] while discussing checks in ATSimplePermissions():

ALTER TABLE ALL IN TABLESPACE pg_default SET TABLESPACE backup_space;

Earlier we decided to error out on this for CLT. Refer [1].

On re-analyzing, I feel 'bulk movement of objects into new tablespace'
should be allowed and CLT should be skipped silently. The rationale is
that if a storage is full and user is trying to move all user created
tables etc to a new tablespace, then he should be allowed to do so. By
giving error in above, we will block him to do an important
maintenance task, leaving the user with no practical option other than
dropping the subscription or moving each object individually.

On checking the code for above point, I found that 'AlterTableMoveAll'
silently skips system tables as well. See the comment there:
'Do not move objects in pg_catalog as part of this'.

Current behaviour around it:

postgres=# SELECT relname, COALESCE(spcname, 'pg_default') FROM
pg_class LEFT JOIN pg_tablespace ON reltablespace = pg_tablespace.oid
WHERE relname IN ('tabnew', 'pg_class');
relname | coalesce
----------+--------------
tabnew | pg_default
pg_class | pg_default
(2 rows)

postgres=# ALTER TABLE ALL IN TABLESPACE pg_default SET TABLESPACE backup_space;
ALTER TABLE

--user table tabnew moved, pg_class skipped silently.
postgres=# SELECT relname, COALESCE(spcname, 'pg_default') FROM
pg_class LEFT JOIN pg_tablespace ON reltablespace = pg_tablespace.oid
WHERE relname IN ('tabnew', 'pg_class');
relname | coalesce
----------+--------------
tabnew | backup_space
pg_class | pg_default

--Now sub creation blocks the entire operation.
postgres=# create subscription sub1 connection '...' publication pub1
WITH ( conflict_log_destination='all');
NOTICE: created conflict log table
"pg_conflict.pg_conflict_log_16425" for subscription "sub1"
NOTICE: created replication slot "sub1" on publisher
CREATE SUBSCRIPTION

postgres=# ALTER TABLE ALL IN TABLESPACE pg_default SET TABLESPACE
backup_space;
ERROR: 42501: permission denied: "pg_conflict_log_16454" is a
conflict log table
DETAIL: Conflict log tables are system-managed tables for logical
replication conflicts.
LOCATION: ATSimplePermissions, tablecmds.c:6930

~~

IMO, we should silently skip CLT by expanding the check in
AlterTableMoveAll() similar to system-tables.

[1]: https://www.postgresql.org/message-id/CAJpy0uC5x%2BZ%3Dfvurvz%3DK8EYQSCQ1SCouR2LqiHS27Hp_Tzi6qw%40mail.gmail.com

thanks
Shveta