On Wed, Jun 24, 2026 at 7:26 PM Dilip Kumar <[email protected]> wrote: > > Please find the updated patch set attached.
Hi Dilip, I ran a code coverage report on v56-0001 and v56-0002 and found a few cases not covered by existing tests. I added tests for the following: 1) check_publication_add_relation() – verify that adding a conflict table to a publication errors out 2) execMain.c: CheckValidRowMarkRel() – verify row locking on CLT is rejected for commands like SELECT ... FOR UPDATE 3) The test scenario shared by Shveta at [1] Please consider the patch if you are okay with it. The patch applies cleanly on top of latest v58-0002 patch, [1] https://www.postgresql.org/message-id/CAJpy0uC5x%2BZ%3Dfvurvz%3DK8EYQSCQ1SCouR2LqiHS27Hp_Tzi6qw%40mail.gmail.com -- Thanks, Nisha
From 15cea266810126ca3043775d4690c316d54d98a6 Mon Sep 17 00:00:00 2001 From: Nisha Moond <[email protected]> Date: Thu, 25 Jun 2026 19:38:40 +0530 Subject: [PATCH v56_nisha] Extend test coverage for missing scenarios --- src/test/regress/expected/subscription.out | 34 ++++++++++++++++++ src/test/regress/sql/subscription.sql | 34 ++++++++++++++++++ src/test/subscription/t/035_conflicts.pl | 41 ++++++++++++++++++++++ 3 files changed, 109 insertions(+) diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index e20ccd365bf..fda35293cbd 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -1136,6 +1136,40 @@ EXCEPTION WHEN insufficient_privilege THEN regexp_replace(err_detail, 'pg_conflict_log_\d+', 'pg_conflict_log_xxx'); END $$; NOTICE: Create index error: permission denied: "pg_conflict_log_xxx" is a conflict log table, detail: Conflict log tables are system-managed tables for logical replication conflicts. +-- fail - conflict log table cannot be added to a publication +DO $$ +DECLARE + tab_name text; + err_msg text; + err_detail text; +BEGIN + SELECT 'pg_conflict.' || relname INTO tab_name + FROM pg_class c JOIN pg_subscription s ON c.relname = 'pg_conflict_log_' || s.oid + WHERE s.subname = 'regress_conflict_protection_test'; + EXECUTE 'CREATE PUBLICATION testpub_for_clt FOR TABLE ' || tab_name; +EXCEPTION WHEN invalid_parameter_value THEN + GET STACKED DIAGNOSTICS err_msg = MESSAGE_TEXT, err_detail = PG_EXCEPTION_DETAIL; + RAISE NOTICE 'Publication error: %, detail: %', + regexp_replace(err_msg, 'pg_conflict_log_\d+', 'pg_conflict_log_xxx'), + err_detail; +END $$; +NOTICE: Publication error: cannot add relation "pg_conflict_log_xxx" to publication, detail: This operation is not supported for conflict log tables. +-- fail - cannot lock rows in conflict log table +DO $$ +DECLARE + tab_name text; + err_msg text; +BEGIN + SELECT 'pg_conflict.' || relname INTO tab_name + FROM pg_class c JOIN pg_subscription s ON c.relname = 'pg_conflict_log_' || s.oid + WHERE s.subname = 'regress_conflict_protection_test'; + EXECUTE 'SELECT 1 FROM ' || tab_name || ' FOR UPDATE'; +EXCEPTION WHEN wrong_object_type THEN + GET STACKED DIAGNOSTICS err_msg = MESSAGE_TEXT; + RAISE NOTICE 'Row lock error: %', + regexp_replace(err_msg, 'pg_conflict_log_\d+', 'pg_conflict_log_xxx'); +END $$; +NOTICE: Row lock error: cannot lock rows in the conflict log table "pg_conflict_log_xxx" -- Clean up trigger function DROP FUNCTION public.dummy_trigger_func(); SET client_min_messages = WARNING; diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index c557a74977e..f72869bbbd1 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -934,6 +934,40 @@ EXCEPTION WHEN insufficient_privilege THEN regexp_replace(err_detail, 'pg_conflict_log_\d+', 'pg_conflict_log_xxx'); END $$; +-- fail - conflict log table cannot be added to a publication +DO $$ +DECLARE + tab_name text; + err_msg text; + err_detail text; +BEGIN + SELECT 'pg_conflict.' || relname INTO tab_name + FROM pg_class c JOIN pg_subscription s ON c.relname = 'pg_conflict_log_' || s.oid + WHERE s.subname = 'regress_conflict_protection_test'; + EXECUTE 'CREATE PUBLICATION testpub_for_clt FOR TABLE ' || tab_name; +EXCEPTION WHEN invalid_parameter_value THEN + GET STACKED DIAGNOSTICS err_msg = MESSAGE_TEXT, err_detail = PG_EXCEPTION_DETAIL; + RAISE NOTICE 'Publication error: %, detail: %', + regexp_replace(err_msg, 'pg_conflict_log_\d+', 'pg_conflict_log_xxx'), + err_detail; +END $$; + +-- fail - cannot lock rows in conflict log table +DO $$ +DECLARE + tab_name text; + err_msg text; +BEGIN + SELECT 'pg_conflict.' || relname INTO tab_name + FROM pg_class c JOIN pg_subscription s ON c.relname = 'pg_conflict_log_' || s.oid + WHERE s.subname = 'regress_conflict_protection_test'; + EXECUTE 'SELECT 1 FROM ' || tab_name || ' FOR UPDATE'; +EXCEPTION WHEN wrong_object_type THEN + GET STACKED DIAGNOSTICS err_msg = MESSAGE_TEXT; + RAISE NOTICE 'Row lock error: %', + regexp_replace(err_msg, 'pg_conflict_log_\d+', 'pg_conflict_log_xxx'); +END $$; + -- Clean up trigger function DROP FUNCTION public.dummy_trigger_func(); diff --git a/src/test/subscription/t/035_conflicts.pl b/src/test/subscription/t/035_conflicts.pl index f23fe6af2a5..cf7c5ff2e9c 100644 --- a/src/test/subscription/t/035_conflicts.pl +++ b/src/test/subscription/t/035_conflicts.pl @@ -670,4 +670,45 @@ 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. +############################################################################### + +# 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 + CONNECTION 'dbname=nonexistent' + PUBLICATION pub + WITH (connect=false, conflict_log_destination='table')"); + +# Create a tablespace backed by a directory inside the data dir. +my $ts_dir = $node_subscriber->data_dir . '/backup_space'; +mkdir($ts_dir) + or die "could not create tablespace directory $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', + "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 +$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('postgres', "DROP DATABASE clt_ts_test"); +$node_subscriber->safe_psql('postgres', "DROP TABLESPACE backup_space"); + done_testing(); -- 2.50.1 (Apple Git-155)
