From 415d707187070df88e11e9bb1059309ab102953e Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Date: Wed, 5 Mar 2025 07:06:58 +0000
Subject: [PATCH v3] Modify pg_logicalinspect isolation test

The previous version was relying on the fact that the test produces exactly
2 snapshots on disk, while in fact it can produce more. Changing the test knowing
that at least 2 snapshots are generated.

In passing, removing useless steps and adding some comments.

Per buildfarm member skink.
---
 .../expected/logical_inspect.out              | 44 +++++--------------
 .../specs/logical_inspect.spec                | 18 +++++---
 2 files changed, 24 insertions(+), 38 deletions(-)
  57.1% contrib/pg_logicalinspect/expected/
  42.8% contrib/pg_logicalinspect/specs/

diff --git a/contrib/pg_logicalinspect/expected/logical_inspect.out b/contrib/pg_logicalinspect/expected/logical_inspect.out
index d95efa4d1e5..c86711e471d 100644
--- a/contrib/pg_logicalinspect/expected/logical_inspect.out
+++ b/contrib/pg_logicalinspect/expected/logical_inspect.out
@@ -1,6 +1,6 @@
 Parsed test spec with 2 sessions
 
-starting permutation: s0_init s0_begin s0_savepoint s0_truncate s1_checkpoint s1_get_changes s0_commit s0_begin s0_insert s1_checkpoint s1_get_changes s0_commit s1_get_changes s1_get_logical_snapshot_info s1_get_logical_snapshot_meta
+starting permutation: s0_init s0_begin s0_savepoint s0_truncate s1_create_table s1_checkpoint s1_get_changes s1_get_logical_snapshot_info s1_get_logical_snapshot_meta s0_commit
 step s0_init: SELECT 'init' FROM pg_create_logical_replication_slot('isolation_slot', 'test_decoding');
 ?column?
 --------
@@ -10,43 +10,23 @@ init
 step s0_begin: BEGIN;
 step s0_savepoint: SAVEPOINT sp1;
 step s0_truncate: TRUNCATE tbl1;
+step s1_create_table: CREATE TABLE tbl2 (val1 integer, val2 integer);
 step s1_checkpoint: CHECKPOINT;
 step s1_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0');
 data
 ----
 (0 rows)
 
-step s0_commit: COMMIT;
-step s0_begin: BEGIN;
-step s0_insert: INSERT INTO tbl1 VALUES (1);
-step s1_checkpoint: CHECKPOINT;
-step s1_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0');
-data                                   
----------------------------------------
-BEGIN                                  
-table public.tbl1: TRUNCATE: (no-flags)
-COMMIT                                 
-(3 rows)
-
-step s0_commit: COMMIT;
-step s1_get_changes: SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0');
-data                                                         
--------------------------------------------------------------
-BEGIN                                                        
-table public.tbl1: INSERT: val1[integer]:1 val2[integer]:null
-COMMIT                                                       
-(3 rows)
-
-step s1_get_logical_snapshot_info: SELECT info.state, info.catchange_count, array_length(info.catchange_xip,1) AS catchange_array_length, info.committed_count, array_length(info.committed_xip,1) AS committed_array_length FROM pg_ls_logicalsnapdir(), pg_get_logical_snapshot_info(name) AS info ORDER BY 2;
-state     |catchange_count|catchange_array_length|committed_count|committed_array_length
-----------+---------------+----------------------+---------------+----------------------
-consistent|              0|                      |              2|                     2
-consistent|              2|                     2|              0|                      
-(2 rows)
+step s1_get_logical_snapshot_info: SELECT count(*) > 0 as has_info FROM pg_ls_logicalsnapdir(), pg_get_logical_snapshot_info(name) AS info where info.catchange_count >= 2 and array_length(info.catchange_xip,1) >= 2 and info.committed_count >= 1 and array_length(info.committed_xip,1) >= 1;
+has_info
+--------
+t       
+(1 row)
 
-step s1_get_logical_snapshot_meta: SELECT COUNT(meta.*) from pg_ls_logicalsnapdir(), pg_get_logical_snapshot_meta(name) as meta;
-count
------
-    2
+step s1_get_logical_snapshot_meta: SELECT COUNT(meta.*) > 0 AS has_meta from pg_ls_logicalsnapdir(), pg_get_logical_snapshot_meta(name) as meta;
+has_meta
+--------
+t       
 (1 row)
 
+step s0_commit: COMMIT;
diff --git a/contrib/pg_logicalinspect/specs/logical_inspect.spec b/contrib/pg_logicalinspect/specs/logical_inspect.spec
index 9851a6c18e4..a6bc58ae955 100644
--- a/contrib/pg_logicalinspect/specs/logical_inspect.spec
+++ b/contrib/pg_logicalinspect/specs/logical_inspect.spec
@@ -1,6 +1,6 @@
 # Test the pg_logicalinspect functions: that needs some permutation to
-# ensure that we are creating multiple logical snapshots and that one of them
-# contains ongoing catalogs changes.
+# ensure that we are creating at least one snapshot that contains ongoing and
+# committed catalogs changes.
 setup
 {
     DROP TABLE IF EXISTS tbl1;
@@ -11,6 +11,7 @@ setup
 teardown
 {
     DROP TABLE tbl1;
+    DROP TABLE tbl2;
     SELECT 'stop' FROM pg_drop_replication_slot('isolation_slot');
     DROP EXTENSION pg_logicalinspect;
 }
@@ -21,14 +22,19 @@ step "s0_init" { SELECT 'init' FROM pg_create_logical_replication_slot('isolatio
 step "s0_begin" { BEGIN; }
 step "s0_savepoint" { SAVEPOINT sp1; }
 step "s0_truncate" { TRUNCATE tbl1; }
-step "s0_insert" { INSERT INTO tbl1 VALUES (1); }
 step "s0_commit" { COMMIT; }
 
 session "s1"
 setup { SET synchronous_commit=on; }
 step "s1_checkpoint" { CHECKPOINT; }
+step "s1_create_table" { CREATE TABLE tbl2 (val1 integer, val2 integer); }
 step "s1_get_changes" { SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'skip-empty-xacts', '1', 'include-xids', '0'); }
-step "s1_get_logical_snapshot_meta" { SELECT COUNT(meta.*) from pg_ls_logicalsnapdir(), pg_get_logical_snapshot_meta(name) as meta;}
-step "s1_get_logical_snapshot_info" { SELECT info.state, info.catchange_count, array_length(info.catchange_xip,1) AS catchange_array_length, info.committed_count, array_length(info.committed_xip,1) AS committed_array_length FROM pg_ls_logicalsnapdir(), pg_get_logical_snapshot_info(name) AS info ORDER BY 2; }
+step "s1_get_logical_snapshot_meta" { SELECT COUNT(meta.*) > 0 AS has_meta from pg_ls_logicalsnapdir(), pg_get_logical_snapshot_meta(name) as meta; }
+step "s1_get_logical_snapshot_info" { SELECT count(*) > 0 as has_info FROM pg_ls_logicalsnapdir(), pg_get_logical_snapshot_info(name) AS info where info.catchange_count >= 2 and array_length(info.catchange_xip,1) >= 2 and info.committed_count >= 1 and array_length(info.committed_xip,1) >= 1; }
 
-permutation "s0_init" "s0_begin" "s0_savepoint" "s0_truncate" "s1_checkpoint" "s1_get_changes" "s0_commit" "s0_begin" "s0_insert" "s1_checkpoint" "s1_get_changes" "s0_commit" "s1_get_changes" "s1_get_logical_snapshot_info" "s1_get_logical_snapshot_meta"
+# s0 does not commit until the end of the test. This is needed to ensure that
+# a checkpoint will not remove any snapshots. s0 produces 2 ongoing catalog changes
+# (the truncate and its parent transaction). s1 produces a committed catalog change.
+# So that the get_changes produces (at least) one snapshot that contains 2
+# ongoing catalog changes and the committed catalog change.
+permutation "s0_init" "s0_begin" "s0_savepoint" "s0_truncate" "s1_create_table" "s1_checkpoint" "s1_get_changes" "s1_get_logical_snapshot_info" "s1_get_logical_snapshot_meta" "s0_commit"
-- 
2.34.1

