On Wed, Jun 3, 2026 at 4:18 PM Nisha Moond <[email protected]> wrote: > > On Wed, Jun 3, 2026 at 3:24 PM Nisha Moond <[email protected]> wrote: > > > > Thanks for the updated patches, please find my comments on the v44 > > patches below. These appear to apply to v45-001 and v45-002 as well, > > as both are unchanged. > > > > Here are couple more issues found during tests: > > 5) remote_commit_ts is available when streaming=on, in that case, we > are not updating it and hence see NULL in the CLT. > A simple test case: > -- publisher > ALTER SYSTEM SET logical_decoding_work_mem = '64kB'; > SELECT pg_reload_conf(); > CREATE TABLE t (a int PRIMARY KEY); > CREATE PUBLICATION pub FOR TABLE t; > > -- subscriber (pre-insert the conflicting row, so copy_data=false) > CREATE TABLE t (a int PRIMARY KEY); > INSERT INTO t VALUES (1); > CREATE SUBSCRIPTION sub > CONNECTION 'host=localhost port=9933 dbname=postgres' > PUBLICATION pub WITH (conflict_log_destination = 'all', streaming > = on, copy_data = false); > > Trigger a streaming conflict > -- publisher > BEGIN; > INSERT INTO t SELECT generate_series(2, 50000); > INSERT INTO t VALUES (1); -- conflicts with subscriber's existing row > COMMIT; > > Check the CLT - remote_commit_ts is NULL despite timestamp being known > -- subscriber > SELECT conflict_type, remote_commit_ts, remote_commit_lsn > FROM pg_conflict.pg_conflict_log_<subid>; > conflict_type | remote_commit_ts | remote_commit_lsn > ----------------+------------------+------------------- > insert_exists | | 0/1234ABC > > I think we need to update it in apply_handle_stream_commit() -> case > TRANS_LEADER_APPLY: > remote_commit_ts = commit_data.committime;
Yeah we need to fix this, I will fix in next version. > > 6) Different ERRORs for superuser vs non-superuser > I created a subscription owned by a non-superuser (nisha), and the CLT > for it is: > postgres=# \d > List of relations > Schema | Name | Type | Owner > -------------+-----------------------+-------+--------- > pg_conflict | pg_conflict_log_16469 | table | nisha > > > Now if I try to UPDATE the CLT as a superuser: > > postgres=# update pg_conflict_log_16469 set conflict_type= > 'INSERT_EXIST' where conflict_type='insert_exists'; > ERROR: cannot modify or insert data into conflict log table > "pg_conflict_log_16469" > DETAIL: Conflict log tables are system-managed and only support > cleanup via DELETE or TRUNCATE. > > However, running the same command as nisha (the sub/clt owner) results in: > > postgres=# SET SESSION AUTHORIZATION nisha; > SET > postgres=> update pg_conflict_log_16469 set conflict_type= > 'INSERT_EXIST' where conflict_type='insert_exists'; > ERROR: permission denied for table pg_conflict_log_16469 > > I think the error should be consistent with the superuser case, rather > than failing with a generic permission error. This has already been discussed, and the same behavior difference exists for the toast table as well, because for superuser we do not mask the permission at acl check instead we block at the operation level however for non superuser it is blocked at the acl check and we are keeping the same behavious for the conflict log table as well. -- Regards, Dilip Kumar Google
