Detect unused steps in isolation specs and do some cleanup

This is useful for developers to find out if an isolation spec is
over-engineered or if it needs more work by warning at the end of a
test run if a step is not used, generating a failure with extra diffs.

While on it, clean up all the specs which include steps not used in any
permutations to simplify them.

This is a backpatch of 989d23b and 06fdc4e, as it is becoming useful to
make all the branches consistent for an upcoming patch that will improve
the output generated by isolationtester.

Author: Michael Paquier
Reviewed-by: Asim Praveen, Melanie Plageman
Discussion: https://postgr.es/m/20190819080820.GG18166@paquier.xyz
Discussion: https://postgr.es/m/794820.1623872009@sss.pgh.pa.us
Backpatch-through: 9.6

diff --git a/contrib/test_decoding/specs/concurrent_ddl_dml.spec b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
index e7cea37d307fbbb3e033b481d074aa9995e5924e..d16515e6f4843afa73cc1e536e9a7418fc77b5dc 100644 (file)
--- a/contrib/test_decoding/specs/concurrent_ddl_dml.spec
+++ b/contrib/test_decoding/specs/concurrent_ddl_dml.spec
@@ -19,7 +19,6 @@ setup { SET synchronous_commit=on; }
 step "s1_init" { SELECT 'init' FROM pg_create_logical_replication_slot('isolation_slot', 'test_decoding'); }
 step "s1_begin" { BEGIN; }
 step "s1_insert_tbl1" { INSERT INTO tbl1 (val1, val2) VALUES (1, 1); }
-step "s1_insert_tbl1_3col" { INSERT INTO tbl1 (val1, val2, val3) VALUES (1, 1, 1); }
 step "s1_insert_tbl2" { INSERT INTO tbl2 (val1, val2) VALUES (1, 1); }
 step "s1_insert_tbl2_3col" { INSERT INTO tbl2 (val1, val2, val3) VALUES (1, 1, 1); }
 step "s1_commit" { COMMIT; }
@@ -29,15 +28,8 @@ setup { SET synchronous_commit=on; }
 
 step "s2_alter_tbl1_float" { ALTER TABLE tbl1 ALTER COLUMN val2 TYPE float; }
 step "s2_alter_tbl1_char" { ALTER TABLE tbl1 ALTER COLUMN val2 TYPE character varying; }
-step "s2_alter_tbl1_text" { ALTER TABLE tbl1 ALTER COLUMN val2 TYPE text; }
 step "s2_alter_tbl1_boolean" { ALTER TABLE tbl1 ALTER COLUMN val2 TYPE boolean; }
 
-step "s2_alter_tbl1_add_int" { ALTER TABLE tbl1 ADD COLUMN val3 INTEGER; }
-step "s2_alter_tbl1_add_float" { ALTER TABLE tbl1 ADD COLUMN val3 FLOAT; }
-step "s2_alter_tbl1_add_char" { ALTER TABLE tbl1 ADD COLUMN val3 character varying; }
-step "s2_alter_tbl1_add_boolean" { ALTER TABLE tbl1 ADD COLUMN val3 BOOLEAN; }
-step "s2_alter_tbl1_add_text" { ALTER TABLE tbl1 ADD COLUMN val3 TEXT; }
-
 step "s2_alter_tbl2_float" { ALTER TABLE tbl2 ALTER COLUMN val2 TYPE float; }
 step "s2_alter_tbl2_char" { ALTER TABLE tbl2 ALTER COLUMN val2 TYPE character varying; }
 step "s2_alter_tbl2_text" { ALTER TABLE tbl2 ALTER COLUMN val2 TYPE text; }
@@ -46,7 +38,6 @@ step "s2_alter_tbl2_boolean" { ALTER TABLE tbl2 ALTER COLUMN val2 TYPE boolean;
 step "s2_alter_tbl2_add_int" { ALTER TABLE tbl2 ADD COLUMN val3 INTEGER; }
 step "s2_alter_tbl2_add_float" { ALTER TABLE tbl2 ADD COLUMN val3 FLOAT; }
 step "s2_alter_tbl2_add_char" { ALTER TABLE tbl2 ADD COLUMN val3 character varying; }
-step "s2_alter_tbl2_add_boolean" { ALTER TABLE tbl2 ADD COLUMN val3 BOOLEAN; }
 step "s2_alter_tbl2_add_text" { ALTER TABLE tbl2 ADD COLUMN val3 TEXT; }
 step "s2_alter_tbl2_drop_3rd_col" { ALTER TABLE tbl2 DROP COLUMN val3; }
 step "s2_alter_tbl2_3rd_char" { ALTER TABLE tbl2 ALTER COLUMN val3 TYPE character varying; }

diff --git a/contrib/test_decoding/specs/snapshot_transfer.spec b/contrib/test_decoding/specs/snapshot_transfer.spec
index 47db7fd90aec51a05037810293f39ff9da1fba87..b6f56cfa049f98fac1bdb7e46c57cb2f42400045 100644 (file)
--- a/contrib/test_decoding/specs/snapshot_transfer.spec
+++ b/contrib/test_decoding/specs/snapshot_transfer.spec
@@ -25,7 +25,6 @@ step "s0_sub_get_base_snap" { INSERT INTO dummy VALUES (0); }
 step "s0_insert" { INSERT INTO harvest VALUES (1, 2, 3); }
 step "s0_end_sub0" { RELEASE SAVEPOINT s0; }
 step "s0_end_sub1" { RELEASE SAVEPOINT s1; }
-step "s0_insert2" { INSERT INTO harvest VALUES (1, 2, 3, 4); }
 step "s0_commit" { COMMIT; }
 step "s0_get_changes" { SELECT data FROM pg_logical_slot_get_changes('isolation_slot', NULL, NULL, 'include-xids', '0', 'skip-empty-xacts', '1'); }
 

diff --git a/src/test/isolation/expected/eval-plan-qual-trigger.out b/src/test/isolation/expected/eval-plan-qual-trigger.out
index 6d7bad42cc2ef2487158e16f9362de94221ddb8d..6bacf2e378bb6e3acc79dea96127079ea02f0fda 100644 (file)
--- a/src/test/isolation/expected/eval-plan-qual-trigger.out
+++ b/src/test/isolation/expected/eval-plan-qual-trigger.out
@@ -1,4 +1,4 @@
-Parsed test spec with 4 sessions
+Parsed test spec with 3 sessions
 
 starting permutation: s1_trig_rep_b_u s1_trig_rep_a_u s1_ins_a s1_ins_b s1_b_rc s2_b_rc s1_upd_a_data s1_c s2_upd_a_data s2_c s0_rep
 step s1_trig_rep_b_u: CREATE TRIGGER rep_b_u BEFORE UPDATE ON trigtest FOR EACH ROW EXECUTE PROCEDURE trig_report();

diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index 20466e03b1e6fb78f01448acd081a0c9340ad5b6..ed18f1231614823333a23e649180fbab93cd545f 100644 (file)
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -88,7 +88,7 @@ main(int argc, char **argv)
                puts("isolationtester (PostgreSQL) " PG_VERSION);
                exit(0);
            default:
-               fprintf(stderr, "Usage: isolationtester [-n] [CONNINFO]\n");
+               fprintf(stderr, "Usage: isolationtester [CONNINFO]\n");
                return EXIT_FAILURE;
        }
    }
@@ -252,10 +252,23 @@ static int *piles;
 static void
 run_testspec(TestSpec *testspec)
 {
+   int         i;
+
    if (testspec->permutations)
        run_named_permutations(testspec);
    else
        run_all_permutations(testspec);
+
+   /*
+    * Verify that all steps have been used, complaining about anything
+    * defined but not used.
+    */
+   for (i = 0; i < testspec->nallsteps; i++)
+   {
+       if (!testspec->allsteps[i]->used)
+           fprintf(stderr, "unused step name: %s\n",
+                   testspec->allsteps[i]->name);
+   }
 }
 
 /*
@@ -455,7 +468,11 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps)
 
    printf("\nstarting permutation:");
    for (i = 0; i < nsteps; i++)
+   {
+       /* Track the permutation as in-use */
+       steps[i]->used = true;
        printf(" %s", steps[i]->name);
+   }
    printf("\n");
 
    /* Perform setup */

diff --git a/src/test/isolation/isolationtester.h b/src/test/isolation/isolationtester.h
index a4d989bd1a2e751bd6a3bb86b744a3e378029885..6655d37204626c06c537410ff90ffa8e6de48e5d 100644 (file)
--- a/src/test/isolation/isolationtester.h
+++ b/src/test/isolation/isolationtester.h
@@ -29,6 +29,7 @@ struct Session
 struct Step
 {
    int         session;
+   bool        used;
    char       *name;
    char       *sql;
    char       *errormsg;

diff --git a/src/test/isolation/specparse.y b/src/test/isolation/specparse.y
index 654716194c16be8a94510587da7eedfafe8bd234..a5462642ddb763cc8f5b8e2201d6462bb0a5783a 100644 (file)
--- a/src/test/isolation/specparse.y
+++ b/src/test/isolation/specparse.y
@@ -145,6 +145,7 @@ step:
                $$ = pg_malloc(sizeof(Step));
                $$->name = $2;
                $$->sql = $3;
+               $$->used = false;
                $$->errormsg = NULL;
            }
        ;

diff --git a/src/test/isolation/specs/eval-plan-qual-trigger.spec b/src/test/isolation/specs/eval-plan-qual-trigger.spec
index d3b55b7d409e3ddf628d32e8da843aa905d8494a..9876948f9f282832bdd43a4e284cd749d7549fa8 100644 (file)
--- a/src/test/isolation/specs/eval-plan-qual-trigger.spec
+++ b/src/test/isolation/specs/eval-plan-qual-trigger.spec
@@ -113,7 +113,7 @@ session "s2"
 step "s2_b_rc"     { BEGIN ISOLATION LEVEL READ COMMITTED; SELECT 1; }
 step "s2_b_rr"     { BEGIN ISOLATION LEVEL REPEATABLE READ; SELECT 1; }
 step "s2_c"     { COMMIT; }
-step "s2_r"     { ROLLBACK; }
+#step "s2_r"     { ROLLBACK; }
 step "s2_ins_a" { INSERT INTO trigtest VALUES ('key-a', 'val-a-s2') RETURNING *; }
 step "s2_del_a" {
     DELETE FROM trigtest
@@ -153,25 +153,25 @@ step "s2_upsert_a_data" {
     RETURNING *;
 }
 
-session "s3"
+#session "s3"
 #setup          {  }
-step "s3_b_rc"     { BEGIN ISOLATION LEVEL READ COMMITTED; SELECT 1; }
-step "s3_c"     { COMMIT; }
-step "s3_r"     { ROLLBACK; }
-step "s3_del_a" {
-    DELETE FROM trigtest
-    WHERE
-        noisy_oper('upd', key, '=', 'key-a') AND
-        noisy_oper('upk', data, '<>', 'mismatch')
-    RETURNING *
-}
-step "s3_upd_a_data" {
-    UPDATE trigtest SET data = data || '-ups3'
-    WHERE
-        noisy_oper('upd', key, '=', 'key-a') AND
-        noisy_oper('upk', data, '<>', 'mismatch')
-    RETURNING *;
-}
+#step "s3_b_rc"     { BEGIN ISOLATION LEVEL READ COMMITTED; SELECT 1; }
+#step "s3_c"     { COMMIT; }
+#step "s3_r"     { ROLLBACK; }
+#step "s3_del_a" {
+#    DELETE FROM trigtest
+#    WHERE
+#        noisy_oper('upd', key, '=', 'key-a') AND
+#        noisy_oper('upk', data, '<>', 'mismatch')
+#    RETURNING *
+#}
+#step "s3_upd_a_data" {
+#    UPDATE trigtest SET data = data || '-ups3'
+#    WHERE
+#        noisy_oper('upd', key, '=', 'key-a') AND
+#        noisy_oper('upk', data, '<>', 'mismatch')
+#    RETURNING *;
+#}
 
 ### base case verifying that triggers see performed modifications
 # s1 updates, s1 commits, s2 updates

diff --git a/src/test/isolation/specs/freeze-the-dead.spec b/src/test/isolation/specs/freeze-the-dead.spec
index e24d7d5d116b2e96672dd44962ff8d665912a345..219242841d4f6f2013ce8e3d24ff46147ae5e732 100644 (file)
--- a/src/test/isolation/specs/freeze-the-dead.spec
+++ b/src/test/isolation/specs/freeze-the-dead.spec
@@ -19,7 +19,6 @@ session "s1"
 step "s1_begin"        { BEGIN; }
 step "s1_update"   { UPDATE tab_freeze SET x = x + 1 WHERE id = 3; }
 step "s1_commit"   { COMMIT; }
-step "s1_vacuum"   { VACUUM FREEZE tab_freeze; }
 step "s1_selectone"    {
     BEGIN;
     SET LOCAL enable_seqscan = false;
@@ -28,7 +27,6 @@ step "s1_selectone"   {
     COMMIT;
 }
 step "s1_selectall"    { SELECT * FROM tab_freeze ORDER BY name, id; }
-step "s1_reindex"  { REINDEX TABLE tab_freeze; }
 
 session "s2"
 step "s2_begin"        { BEGIN; }
@@ -40,7 +38,6 @@ session "s3"
 step "s3_begin"        { BEGIN; }
 step "s3_key_share"    { SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE; }
 step "s3_commit"   { COMMIT; }
-step "s3_vacuum"   { VACUUM FREEZE tab_freeze; }
 
 # This permutation verfies that a previous bug
 #     https://postgr.es/m/E5711E62-8FDF-4DCA-A888-C200BF6B5742@amazon.com

diff --git a/src/test/isolation/specs/insert-conflict-do-nothing.spec b/src/test/isolation/specs/insert-conflict-do-nothing.spec
index 9b92c35cec6654c5c2b9625129834adcc8db0d4d..71acc380c7a941d3bb9641024f5566229b024337 100644 (file)
--- a/src/test/isolation/specs/insert-conflict-do-nothing.spec
+++ b/src/test/isolation/specs/insert-conflict-do-nothing.spec
@@ -33,7 +33,6 @@ setup
 step "donothing2" { INSERT INTO ints(key, val) VALUES(1, 'donothing2') ON CONFLICT DO NOTHING; }
 step "select2" { SELECT * FROM ints; }
 step "c2" { COMMIT; }
-step "a2" { ABORT; }
 
 # Regular case where one session block-waits on another to determine if it
 # should proceed with an insert or do nothing.

diff --git a/src/test/isolation/specs/insert-conflict-do-update-2.spec b/src/test/isolation/specs/insert-conflict-do-update-2.spec
index f5b4f601b58c88838fe8421928389d47f2eac1a6..12f6be8000f990cac5f2b8fac77fff860962a003 100644 (file)
--- a/src/test/isolation/specs/insert-conflict-do-update-2.spec
+++ b/src/test/isolation/specs/insert-conflict-do-update-2.spec
@@ -32,7 +32,6 @@ setup
 step "insert2" { INSERT INTO upsert(key, payload) VALUES('FOOFOO', 'insert2') ON CONFLICT (lower(key)) DO UPDATE set key = EXCLUDED.key, payload = upsert.payload || ' updated by insert2'; }
 step "select2" { SELECT * FROM upsert; }
 step "c2" { COMMIT; }
-step "a2" { ABORT; }
 
 # One session (session 2) block-waits on another (session 1) to determine if it
 # should proceed with an insert or update.  The user can still usefully UPDATE

diff --git a/src/test/isolation/specs/insert-conflict-do-update.spec b/src/test/isolation/specs/insert-conflict-do-update.spec
index 5d335a34449574056ee7da9d362d05aec7419390..7c8cb4710025318fda5c32db8376d890d267d2f3 100644 (file)
--- a/src/test/isolation/specs/insert-conflict-do-update.spec
+++ b/src/test/isolation/specs/insert-conflict-do-update.spec
@@ -30,7 +30,6 @@ setup
 step "insert2" { INSERT INTO upsert(key, val) VALUES(1, 'insert2') ON CONFLICT (key) DO UPDATE set val = upsert.val || ' updated by insert2'; }
 step "select2" { SELECT * FROM upsert; }
 step "c2" { COMMIT; }
-step "a2" { ABORT; }
 
 # One session (session 2) block-waits on another (session 1) to determine if it
 # should proceed with an insert or update.  Notably, this entails updating a

diff --git a/src/test/isolation/specs/partition-key-update-1.spec b/src/test/isolation/specs/partition-key-update-1.spec
index 8393f47c596e07e5bc3b7561783507cc0885d69a..dcdd6c03b104b550f17f5d9219f10d4e2b3761d1 100644 (file)
--- a/src/test/isolation/specs/partition-key-update-1.spec
+++ b/src/test/isolation/specs/partition-key-update-1.spec
@@ -57,7 +57,6 @@ step "s1r"    { ROLLBACK; }
 
 session "s2"
 step "s2b" { BEGIN ISOLATION LEVEL READ COMMITTED; }
-step "s2u" { UPDATE foo SET b='EFG' WHERE a=1; }
 step "s2u2"    { UPDATE footrg SET b='XYZ' WHERE a=1; }
 step "s2i" { INSERT INTO bar VALUES(7); }
 step "s2d" { DELETE FROM foo WHERE a=1; }

diff --git a/src/test/isolation/specs/sequence-ddl.spec b/src/test/isolation/specs/sequence-ddl.spec
index 5c51fcdae686c44854e8bbef000320ac24bcf6b9..9ff9fd0259f83edf02909596e7369ef3bd604b9f 100644 (file)
--- a/src/test/isolation/specs/sequence-ddl.spec
+++ b/src/test/isolation/specs/sequence-ddl.spec
@@ -15,7 +15,6 @@ setup           { BEGIN; }
 step "s1alter"  { ALTER SEQUENCE seq1 MAXVALUE 10; }
 step "s1alter2" { ALTER SEQUENCE seq1 MAXVALUE 20; }
 step "s1restart" { ALTER SEQUENCE seq1 RESTART WITH 5; }
-step "s1setval" { SELECT setval('seq1', 5); }
 step "s1commit" { COMMIT; }
 
 session "s2"

diff --git a/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec b/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec
index 73f71b17a7afb4bcdc298e2a2634022547281949..106c2465c0aebabe26212be55f8dc707c677abc5 100644 (file)
--- a/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec
+++ b/src/test/isolation/specs/tuplelock-upgrade-no-deadlock.spec
@@ -19,7 +19,6 @@ teardown
 session "s0"
 step "s0_begin" { begin; }
 step "s0_keyshare" { select id from tlu_job where id = 1 for key share;}
-step "s0_share" { select id from tlu_job where id = 1 for share;}
 step "s0_rollback" { rollback; }
 
 session "s1"
@@ -28,7 +27,6 @@ step "s1_keyshare" { select id from tlu_job where id = 1 for key share;}
 step "s1_share" { select id from tlu_job where id = 1 for share; }
 step "s1_fornokeyupd" { select id from tlu_job where id = 1 for no key update; }
 step "s1_update" { update tlu_job set name = 'b' where id = 1;  }
-step "s1_delete" { delete from tlu_job where id = 1; }
 step "s1_savept_e" { savepoint s1_e; }
 step "s1_savept_f" { savepoint s1_f; }
 step "s1_rollback_e" { rollback to s1_e; }
@@ -44,7 +42,6 @@ step "s2_for_update" { select id from tlu_job where id = 1 for update; }
 step "s2_update" { update tlu_job set name = 'b' where id = 1; }
 step "s2_delete" { delete from tlu_job where id = 1; }
 step "s2_rollback" { rollback; }
-step "s2_commit" { commit; }
 
 session "s3"
 setup { begin; }