From 1145c26b749a73419d32e4c997237e84fbbffa04 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Mon, 8 Oct 2018 16:16:36 -0400
Subject: [PATCH] Advance transaction timestamp for intra-procedure
 transactions.

Per discussion, this behavior seems less astonishing than not doing so.

Peter Eisentraut and Tom Lane

Discussion: https://postgr.es/m/20180920234040.GC29981@momjian.us
---
 src/backend/access/transam/xact.c             | 22 ++++++++-----
 src/backend/executor/spi.c                    | 13 ++++++++
 src/include/executor/spi.h                    |  1 +
 .../src/expected/plpgsql_transaction.out      | 30 +++++++++++++++++
 .../plpgsql/src/sql/plpgsql_transaction.sql   | 33 +++++++++++++++++++
 5 files changed, 91 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 88bf1f435ab..09e7e7f9a46 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1906,20 +1906,26 @@ StartTransaction(void)
 	TRACE_POSTGRESQL_TRANSACTION_START(vxid.localTransactionId);
 
 	/*
-	 * set transaction_timestamp() (a/k/a now()).  We want this to be the same
-	 * as the first command's statement_timestamp(), so don't do a fresh
-	 * GetCurrentTimestamp() call (which'd be expensive anyway).  In a
-	 * parallel worker, this should already have been provided by a call to
+	 * set transaction_timestamp() (a/k/a now()).  Normally, we want this to
+	 * be the same as the first command's statement_timestamp(), so don't do a
+	 * fresh GetCurrentTimestamp() call (which'd be expensive anyway).  But
+	 * for transactions started inside procedures (i.e., nonatomic SPI
+	 * contexts), we do need to advance the timestamp.  Also, in a parallel
+	 * worker, the timestamp should already have been provided by a call to
 	 * SetParallelStartTimestamps().
-	 *
-	 * Also, mark xactStopTimestamp as unset.
 	 */
 	if (!IsParallelWorker())
-		xactStartTimestamp = stmtStartTimestamp;
+	{
+		if (!SPI_inside_nonatomic_context())
+			xactStartTimestamp = stmtStartTimestamp;
+		else
+			xactStartTimestamp = GetCurrentTimestamp();
+	}
 	else
 		Assert(xactStartTimestamp != 0);
-	xactStopTimestamp = 0;
 	pgstat_report_xact_timestamp(xactStartTimestamp);
+	/* Mark xactStopTimestamp as unset. */
+	xactStopTimestamp = 0;
 
 	/*
 	 * initialize current transaction state fields
diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index 11ca800e4cd..fb36e762f28 100644
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -423,6 +423,19 @@ AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid)
 	}
 }
 
+/*
+ * Are we executing inside a procedure (that is, a nonatomic SPI context)?
+ */
+bool
+SPI_inside_nonatomic_context(void)
+{
+	if (_SPI_current == NULL)
+		return false;			/* not in any SPI context at all */
+	if (_SPI_current->atomic)
+		return false;			/* it's atomic (ie function not procedure) */
+	return true;
+}
+
 
 /* Parse, plan, and execute a query string */
 int
diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h
index 143a89a16c4..b16440cf004 100644
--- a/src/include/executor/spi.h
+++ b/src/include/executor/spi.h
@@ -166,5 +166,6 @@ extern void SPI_rollback(void);
 extern void SPICleanup(void);
 extern void AtEOXact_SPI(bool isCommit);
 extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid);
+extern bool SPI_inside_nonatomic_context(void);
 
 #endif							/* SPI_H */
diff --git a/src/pl/plpgsql/src/expected/plpgsql_transaction.out b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
index 77a83adab54..6eedb215a44 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_transaction.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_transaction.out
@@ -493,6 +493,36 @@ CALL transaction_test10b(10);
  9
 (1 row)
 
+-- transaction timestamp vs. statement timestamp
+CREATE PROCEDURE transaction_test11()
+LANGUAGE plpgsql
+AS $$
+DECLARE
+  s1 timestamp with time zone;
+  s2 timestamp with time zone;
+  s3 timestamp with time zone;
+  t1 timestamp with time zone;
+  t2 timestamp with time zone;
+  t3 timestamp with time zone;
+BEGIN
+  s1 := statement_timestamp();
+  t1 := transaction_timestamp();
+  ASSERT s1 = t1;
+  PERFORM pg_sleep(0.001);
+  COMMIT;
+  s2 := statement_timestamp();
+  t2 := transaction_timestamp();
+  ASSERT s2 = s1;
+  ASSERT t2 > t1;
+  PERFORM pg_sleep(0.001);
+  ROLLBACK;
+  s3 := statement_timestamp();
+  t3 := transaction_timestamp();
+  ASSERT s3 = s1;
+  ASSERT t3 > t2;
+END;
+$$;
+CALL transaction_test11();
 DROP TABLE test1;
 DROP TABLE test2;
 DROP TABLE test3;
diff --git a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
index 0ed9ab873a4..ac1361a8ceb 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_transaction.sql
@@ -412,6 +412,39 @@ $$;
 CALL transaction_test10b(10);
 
 
+-- transaction timestamp vs. statement timestamp
+CREATE PROCEDURE transaction_test11()
+LANGUAGE plpgsql
+AS $$
+DECLARE
+  s1 timestamp with time zone;
+  s2 timestamp with time zone;
+  s3 timestamp with time zone;
+  t1 timestamp with time zone;
+  t2 timestamp with time zone;
+  t3 timestamp with time zone;
+BEGIN
+  s1 := statement_timestamp();
+  t1 := transaction_timestamp();
+  ASSERT s1 = t1;
+  PERFORM pg_sleep(0.001);
+  COMMIT;
+  s2 := statement_timestamp();
+  t2 := transaction_timestamp();
+  ASSERT s2 = s1;
+  ASSERT t2 > t1;
+  PERFORM pg_sleep(0.001);
+  ROLLBACK;
+  s3 := statement_timestamp();
+  t3 := transaction_timestamp();
+  ASSERT s3 = s1;
+  ASSERT t3 > t2;
+END;
+$$;
+
+CALL transaction_test11();
+
+
 DROP TABLE test1;
 DROP TABLE test2;
 DROP TABLE test3;
-- 
GitLab