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

diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index e3f668bf6abd36f304f1714ae5bcfaacfa5e29c8..59e021f887a710f5043808a2781c0d3414d7e333 100644 (file)
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1887,20 +1887,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 11ca800e4cd27812e489ac9e841392d8548d52ea..fb36e762f28e9b4109204021fd239a649fcca2d6 100644 (file)
--- 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 143a89a16c45ece119c8a0d8c2b7b2259554e55c..b16440cf004bdbc8e166de0c1bdf03e21b3d2bfa 100644 (file)
--- 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 77a83adab54bb13177ae35ea9805556d2a341f65..6eedb215a4423b3dc7da2db36d95a4005a5eee24 100644 (file)
--- 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 0ed9ab873a4daef1a3babec71d5624e5a3c5c5e1..ac1361a8ceb7866313a272361081da290653415f 100644 (file)
--- 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;