From 759d9d67695783f6d04a85aba383a41c5382548c Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 28 Jan 2012 17:55:08 -0500
Subject: [PATCH] Add simple tests of EvalPlanQual using the isolationtester
 infrastructure.

Much more could be done here, but at least now we have *some* automated
test coverage of that mechanism.  In particular this tests the writable-CTE
case reported by Phil Sorber.

In passing, remove isolationtester's arbitrary restriction on the number of
steps in a permutation list.  I used this so that a single spec file could
be used to run several related test scenarios, but there are other possible
reasons to want a step series that's not exactly a permutation.  Improve
documentation and fix a couple other nits as well.
---
 src/test/isolation/README                     | 89 +++++++++++++------
 .../isolation/expected/eval-plan-qual.out     | 51 +++++++++++
 src/test/isolation/isolation_schedule         |  1 +
 src/test/isolation/isolationtester.c          | 17 ++--
 src/test/isolation/specs/eval-plan-qual.spec  | 56 ++++++++++++
 5 files changed, 176 insertions(+), 38 deletions(-)
 create mode 100644 src/test/isolation/expected/eval-plan-qual.out
 create mode 100644 src/test/isolation/specs/eval-plan-qual.spec

diff --git a/src/test/isolation/README b/src/test/isolation/README
index aeef7f6c3ab..65cedc84210 100644
--- a/src/test/isolation/README
+++ b/src/test/isolation/README
@@ -3,20 +3,33 @@ src/test/isolation/README
 Isolation tests
 ===============
 
-This directory contains a set of tests for the serializable isolation level.
-Testing isolation requires running multiple overlapping transactions,
-which requires multiple concurrent connections, and therefore can't be
-tested using the normal pg_regress program.
+This directory contains a set of tests for concurrent behaviors in
+PostgreSQL.  These tests require running multiple interacting transactions,
+which requires management of multiple concurrent connections, and therefore
+can't be tested using the normal pg_regress program.  The name "isolation"
+comes from the fact that the original motivation was to test the
+serializable isolation level; but tests for other sorts of concurrent
+behaviors have been added as well.
 
 To run the tests, you need to have a server running at the default port
 expected by libpq.  (You can set PGPORT and so forth in your environment
 to control this.)  Then run
     gmake installcheck
-Note that the prepared-transactions test will not pass unless you have
-the server's max_prepared_transactions parameter set to at least 3.
-
-To represent a test with overlapping transactions, we use a test specification
-file with a custom syntax, which is described in the next section.
+To run just specific test(s), you can do something like
+    ./pg_isolation_regress fk-contention fk-deadlock
+(look into the specs/ subdirectory to see the available tests).
+
+Note that the prepared-transactions test requires the server's
+max_prepared_transactions parameter to be set to at least 3.  We have
+provided a variant expected-output file that allows the test to "pass"
+when max_prepared_transactions has its default value of zero, but of
+course that is not really exercising the feature.
+
+To define tests with overlapping transactions, we use test specification
+files with a custom syntax, which is described in the next section.  To add
+a new test, place a spec file in the specs/ subdirectory, add the expected
+output in the expected/ subdirectory, and add the test's name to the
+isolation_schedule file.
 
 isolationtester is a program that uses libpq to open multiple connections,
 and executes a test specified by a spec file. A libpq connection string
@@ -24,7 +37,8 @@ specifies the server and database to connect to; defaults derived from
 environment variables are used otherwise.
 
 pg_isolation_regress is a tool similar to pg_regress, but instead of using
-psql to execute a test, it uses isolationtester.
+psql to execute a test, it uses isolationtester.  It accepts all the same
+command-line arguments as pg_regress.
 
 
 Test specification
@@ -36,48 +50,65 @@ subdirectory. A test specification consists of four parts, in this order:
 setup { <SQL> }
 
   The given SQL block is executed once, in one session only, before running
-  the test. Create any test tables or such objects here. This part is
-  optional.
+  the test. Create any test tables or other required objects here. This
+  part is optional.
 
 teardown { <SQL> }
 
   The teardown SQL block is executed once after the test is finished. Use
-  this to clean up, e.g dropping any test tables. This part is optional.
+  this to clean up in preparation for the next permutation, e.g dropping
+  any test tables created by setup. This part is optional.
 
 session "<name>"
 
-  Each session is executed in a separate connection. A session consists
-  of four parts: setup, teardown and one or more steps. The per-session
+  There are normally several "session" parts in a spec file. Each
+  session is executed in its own connection. A session part consists
+  of three parts: setup, teardown and one or more "steps". The per-session
   setup and teardown parts have the same syntax as the per-test setup and
-  teardown described above, but they are executed in every session,
-  before and after each permutation. The setup part typically contains a
-  "BEGIN" command to begin a transaction.
+  teardown described above, but they are executed in each session. The
+  setup part typically contains a "BEGIN" command to begin a transaction.
 
-  Each step has a syntax of
+  Each step has the syntax
 
   step "<name>" { <SQL> }
 
-  where <name> is a unique name identifying this step, and SQL is a SQL
-  statement (or statements, separated by semicolons) that is executed in the
-  step.
+  where <name> is a name identifying this step, and SQL is a SQL statement
+  (or statements, separated by semicolons) that is executed in the step.
+  Step names must be unique across the whole spec file.
 
 permutation "<step name>" ...
 
   A permutation line specifies a list of steps that are run in that order.
-  If no permutation lines are given, the test program automatically generates
-  all possible overlapping orderings of the given sessions.
+  Any number of permutation lines can appear.  If no permutation lines are
+  given, the test program automatically generates all possible orderings
+  of the steps from each session (running the steps of any one session in
+  order).  Note that the list of steps in a manually specified
+  "permutation" line doesn't actually have to be a permutation of the
+  available steps; it could for instance repeat some steps more than once,
+  or leave others out.
 
 Lines beginning with a # are considered comments.
 
+For each permutation of the session steps (whether these are manually
+specified in the spec file, or automatically generated), the isolation
+tester runs the main setup part, then per-session setup parts, then
+the selected session steps, then per-session teardown, then the main
+teardown script.  Each selected step is sent to the connection associated
+with its session.
+
 
 Support for blocking commands
 =============================
 
-Each spec may contain commands that block until further action has been taken
+Each step may contain commands that block until further action has been taken
 (most likely, some other session runs a step that unblocks it or causes a
-deadlock).  Such a spec needs to be careful to manually specify valid
+deadlock).  A test that uses this ability must manually specify valid
 permutations, i.e. those that would not expect a blocked session to execute a
-command.  If the spec fails to follow that rule, the spec is aborted.
+command.  If the test fails to follow that rule, the test is aborted.
+
+Currently, at most one step can be waiting at a time.  As long as one
+step is waiting, subsequent steps are run to completion synchronously.
 
-Only one command can be waiting at a time.  As long as one command is waiting,
-other commands are run to completion synchronously.
+Note that isolationtester recognizes that a command has blocked by looking
+to see if it is shown as waiting in the pg_locks view; therefore, only
+blocks on heavyweight locks will be detected.
diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out
new file mode 100644
index 00000000000..ab778cbd7aa
--- /dev/null
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -0,0 +1,51 @@
+Parsed test spec with 3 sessions
+
+starting permutation: wx1 wx2 c1 c2 read
+step wx1: UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking';
+step wx2: UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking'; <waiting ...>
+step c1: COMMIT;
+step wx2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       850            
+savings        600            
+
+starting permutation: wy1 wy2 c1 c2 read
+step wy1: UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking';
+step wy2: UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000; <waiting ...>
+step c1: COMMIT;
+step wy2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       1100           
+savings        600            
+
+starting permutation: upsert1 upsert2 c1 c2 read
+step upsert1: 
+	WITH upsert AS
+	  (UPDATE accounts SET balance = balance + 500
+	   WHERE accountid = 'savings'
+	   RETURNING accountid)
+	INSERT INTO accounts SELECT 'savings', 500
+	  WHERE NOT EXISTS (SELECT 1 FROM upsert);
+
+step upsert2: 
+	WITH upsert AS
+	  (UPDATE accounts SET balance = balance + 1234
+	   WHERE accountid = 'savings'
+	   RETURNING accountid)
+	INSERT INTO accounts SELECT 'savings', 1234
+	  WHERE NOT EXISTS (SELECT 1 FROM upsert);
+ <waiting ...>
+step c1: COMMIT;
+step upsert2: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid      balance        
+
+checking       600            
+savings        2334           
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index c3fa8b31bcb..669c0f220c4 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -13,3 +13,4 @@ test: prepared-transactions
 test: fk-contention
 test: fk-deadlock
 test: fk-deadlock2
+test: eval-plan-qual
diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c
index ab1ef036045..0e681639ba1 100644
--- a/src/test/isolation/isolationtester.c
+++ b/src/test/isolation/isolationtester.c
@@ -395,15 +395,9 @@ run_named_permutations(TestSpec * testspec)
 		Permutation *p = testspec->permutations[i];
 		Step	  **steps;
 
-		if (p->nsteps != nallsteps)
-		{
-			fprintf(stderr, "invalid number of steps in permutation %d\n", i + 1);
-			exit_nicely();
-		}
-
 		steps = malloc(p->nsteps * sizeof(Step *));
 
-		/* Find all the named steps from the lookup table */
+		/* Find all the named steps using the lookup table */
 		for (j = 0; j < p->nsteps; j++)
 		{
 			Step	**this = (Step **) bsearch(p->stepnames[j], allsteps,
@@ -418,7 +412,9 @@ run_named_permutations(TestSpec * testspec)
 			steps[j] = *this;
 		}
 
+		/* And run them */
 		run_permutation(testspec, p->nsteps, steps);
+
 		free(steps);
 	}
 }
@@ -483,6 +479,8 @@ report_two_error_messages(Step *step1, Step *step2)
 		free(step2->errormsg);
 		step2->errormsg = NULL;
 	}
+
+	free(prefix);
 }
 
 /*
@@ -700,7 +698,7 @@ try_complete_step(Step *step, int flags)
 
 	FD_ZERO(&read_set);
 
-	while (flags & STEP_NONBLOCK && PQisBusy(conn))
+	while ((flags & STEP_NONBLOCK) && PQisBusy(conn))
 	{
 		FD_SET(sock, &read_set);
 		timeout.tv_sec = 0;
@@ -739,7 +737,8 @@ try_complete_step(Step *step, int flags)
 		}
 		else if (!PQconsumeInput(conn)) /* select(): data available */
 		{
-			fprintf(stderr, "PQconsumeInput failed: %s", PQerrorMessage(conn));
+			fprintf(stderr, "PQconsumeInput failed: %s\n",
+					PQerrorMessage(conn));
 			exit_nicely();
 		}
 	}
diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec
new file mode 100644
index 00000000000..786b7913652
--- /dev/null
+++ b/src/test/isolation/specs/eval-plan-qual.spec
@@ -0,0 +1,56 @@
+# Tests for the EvalPlanQual mechanism
+#
+# EvalPlanQual is used in READ COMMITTED isolation level to attempt to
+# re-execute UPDATE and DELETE operations against rows that were updated
+# by some concurrent transaction.
+
+setup
+{
+ CREATE TABLE accounts (accountid text PRIMARY KEY, balance numeric not null);
+ INSERT INTO accounts VALUES ('checking', 600), ('savings', 600);
+}
+
+teardown
+{
+ DROP TABLE accounts;
+}
+
+session "s1"
+setup		{ BEGIN ISOLATION LEVEL READ COMMITTED; }
+# wx1 then wx2 checks the basic case of re-fetching up-to-date values
+step "wx1"	{ UPDATE accounts SET balance = balance - 200 WHERE accountid = 'checking'; }
+# wy1 then wy2 checks the case where quals pass then fail
+step "wy1"	{ UPDATE accounts SET balance = balance + 500 WHERE accountid = 'checking'; }
+# upsert tests are to check writable-CTE cases
+step "upsert1"	{
+	WITH upsert AS
+	  (UPDATE accounts SET balance = balance + 500
+	   WHERE accountid = 'savings'
+	   RETURNING accountid)
+	INSERT INTO accounts SELECT 'savings', 500
+	  WHERE NOT EXISTS (SELECT 1 FROM upsert);
+}
+step "c1"	{ COMMIT; }
+
+session "s2"
+setup		{ BEGIN ISOLATION LEVEL READ COMMITTED; }
+step "wx2"	{ UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking'; }
+step "wy2"	{ UPDATE accounts SET balance = balance + 1000 WHERE accountid = 'checking' AND balance < 1000; }
+step "upsert2"	{
+	WITH upsert AS
+	  (UPDATE accounts SET balance = balance + 1234
+	   WHERE accountid = 'savings'
+	   RETURNING accountid)
+	INSERT INTO accounts SELECT 'savings', 1234
+	  WHERE NOT EXISTS (SELECT 1 FROM upsert);
+}
+step "c2"	{ COMMIT; }
+
+session "s3"
+setup		{ BEGIN ISOLATION LEVEL READ COMMITTED; }
+step "read"	{ SELECT * FROM accounts ORDER BY accountid; }
+teardown	{ COMMIT; }
+
+permutation "wx1" "wx2" "c1" "c2" "read"
+permutation "wy1" "wy2" "c1" "c2" "read"
+permutation "upsert1" "upsert2" "c1" "c2" "read"
-- 
GitLab