Hi all

While implementing support for traceable transactions (finding out after
the fact whether an xact committed or aborted), I've found that Pg is very
inconsistent with what it considers a transaction ID from a user facing
point of view, to the point where I think it's hard for users to write
correct queries.

txid_current() returns a 64-bit xid in which the higher 32 bits are the xid
epoch. This providers users with wraparound protection and means they don't
have to deal with the moving xid threshold.

Many other functions accept and return 'xid', the 32-bit type that isn't
wraparound protected. Presumably they assume you'll only use them with
recent transaction IDs, but there are a couple of problems with this:

* We can't ensure they're only used with recent XIDs and can't detect if
they're passed a wrapped around xid

* There's no good way to _get_ a 32-bit xid for the current xact since
txid_current() returns a 64-bit bigint xid.

(I have to admit that in the past I always blindly assumed that
txid_current() returned bigint for historical reasons, because we don't
have a uint32 type and the xid type didn't exist yet. So I'd do things like
get the value of txid_current() and pass it to pg_xact_commit_timestamp()
later on. This turns out to be wrong, it just happens to work until the
epoch counter increments for the first time. Similarly, working around the
seeming oversight of a missing bigint to xid cast with ::text::xid is wrong
but will seem fine at first.)

I'm surprised the 32-bit xid was ever exposed to the user, rather than a
64-bit epoch-extended xid.

It's not clear to me how a user is supposed to correctly pass the result of
txid_current() to anything like pg_xact_commit_timestamp(xid). They'd have
to get the epoch from a new txid_current() call, split both into two 32-bit
values, and do wraparound checking. Exceedingly unwieldy and hard to get
right.

Since I don't think we can get rid of the 32-bit xid, I think we need a
function to get the 32-bit xid from a 64-bit epoch-and-xid with wraparound
protection.

Here's a patch for that, adding a function txid_recent(bigint) => xid that
returns the low 32 bits of a 64-bit xid like that returned from
txid_current if the xid isn't wrapped around. If it's past the wraparound
threshold the function returns null, since most functions that take xid are
strict and will in turn return null. The alternative, an ERROR, seems
harder for users to handle without resorting to plpgsql. It does ERROR on
XIDs in the future though, since there's no good reason to see those. The
epoch is ignored for permanent XIDs.

I don't like the name much, but haven't come up with a better one yet.

Thoughts?


IMO some functions that take 'xid' should be considered for a bigint
variant:

 age    (as txid_age(bigint))
 pg_xact_commit_timestamp

[ select proname from pg_proc where 'xid'::regtype = ANY
(proargtypes::regtype[]) ; ]

and most or all the system views that expose xid should switch to bigint
for 10.0:

 pg_class.relfrozenxid
 pg_class.relminmxid
 pg_database.datfrozenxid
 pg_database.datminmxid
 pg_locks.transactionid
 pg_prepared_xacts.transaction
 pg_stat_activity.backend_xid
 pg_stat_activity.backend_xmin
 pg_stat_replication.backend_xmin
 pg_replication_slots.xmin
 pg_replication_slots.catalog_xmin

[ select attrelid::regclass || '.' || attname from pg_attribute  where
atttypid = 'xid'::regtype AND attnum >= 0; ]

... or if folks find using bigint too ugly, a new xid64 type. "bigxid"?


-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From e8af137358b1d89fb334ad1d715c3f81c15ba5cf Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Tue, 16 Aug 2016 16:54:00 +0800
Subject: [PATCH] Add txid_recent(bigint) => xid

Provide a function to get the 32-bit xid from a bigint extended xid-with-epoch
as returned by txid_current() etc, for use in functions that expect an xid
argument.
---
 doc/src/sgml/func.sgml             | 17 +++++++--
 src/backend/utils/adt/txid.c       | 56 ++++++++++++++++++++++++++++++
 src/include/catalog/pg_proc.h      |  2 ++
 src/test/regress/expected/txid.out | 70 ++++++++++++++++++++++++++++++++++++++
 src/test/regress/sql/txid.sql      | 40 ++++++++++++++++++++++
 5 files changed, 182 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 02b25f9..4ae621f 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -16956,6 +16956,11 @@ SELECT collation for ('foo' COLLATE "de_DE");
        <entry><type>boolean</type></entry>
        <entry>is transaction ID visible in snapshot? (do not use with subtransaction ids)</entry>
       </row>
+      <row>
+       <entry><literal><function>txid_recent(<parameter>bigint</parameter>)</function></literal></entry>
+       <entry><type>xid</type></entry>
+       <entry>return the 32-bit <type>xid</> for a 64-bit transaction ID if it isn't wrapped around, otherwise return null</entry>
+      </row>
      </tbody>
     </tgroup>
    </table>
@@ -16964,9 +16969,15 @@ SELECT collation for ('foo' COLLATE "de_DE");
     The internal transaction ID type (<type>xid</>) is 32 bits wide and
     wraps around every 4 billion transactions.  However, these functions
     export a 64-bit format that is extended with an <quote>epoch</> counter
-    so it will not wrap around during the life of an installation.
-    The data type used by these functions, <type>txid_snapshot</type>,
-    stores information about transaction ID
+    so it will not wrap around during the life of an installation. For that
+    reason you cannot cast a bigint transaction ID directly to <type>xid</>
+    and must use <function>txid_recent(bigint)</function> instead of casting
+    to <type>xid</>.
+   </para>
+
+   <para>
+    The data type used by the xid snapshot functions,
+    <type>txid_snapshot</type>, stores information about transaction ID
     visibility at a particular moment in time.  Its components are
     described in <xref linkend="functions-txid-snapshot-parts">.
    </para>
diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c
index 4fa3cdb..9fdf9c7 100644
--- a/src/backend/utils/adt/txid.c
+++ b/src/backend/utils/adt/txid.c
@@ -117,6 +117,50 @@ convert_xid(TransactionId xid, const TxidEpoch *state)
 }
 
 /*
+ * Helper to get a TransactionId from a 64-bit txid with wraparound detection.
+ * ERRORs if the txid is in the future. Returns permanent XIDs unchanged.
+ * Otherwise returns the 32-bit xid and sets the wraparound param to true
+ * if wraparound is detected, false otherwise.
+ */
+static TransactionId
+get_xid_in_recent_past(txid xid_with_epoch, bool *wraparound)
+{
+	uint32 xid_epoch = (uint32)(xid_with_epoch >>32);
+	TransactionId xid = (TransactionId)(xid_with_epoch);
+	TxidEpoch now_epoch;
+
+	load_xid_epoch(&now_epoch);
+
+	*wraparound = false;
+
+	if (!TransactionIdIsNormal(xid))
+	{
+		/* must be a permanent XID, ignore the epoch and return unchanged */
+		return xid;
+	}
+	else if (xid_epoch > now_epoch.epoch
+			 || (xid_epoch == now_epoch.epoch && xid > now_epoch.last_xid))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("transaction ID "UINT64_FORMAT" is in the future",
+					xid_with_epoch)));
+	}
+	else if (xid_epoch + 1 < now_epoch.epoch
+			 || (xid_epoch + 1 == now_epoch.epoch && xid < now_epoch.last_xid))
+	{
+		/* xid too far in the past */
+		*wraparound = true;
+	}
+	else
+	{
+		Assert(TransactionIdPrecedes(xid, now_epoch.last_xid));
+	}
+
+	return xid;
+}
+
+/*
  * txid comparator for qsort/bsearch
  */
 static int
@@ -640,3 +684,15 @@ txid_snapshot_xip(PG_FUNCTION_ARGS)
 		SRF_RETURN_DONE(fctx);
 	}
 }
+
+Datum
+txid_recent(PG_FUNCTION_ARGS)
+{
+	bool wraparound;
+	TransactionId xid = get_xid_in_recent_past(PG_GETARG_INT64(0), &wraparound);
+
+	if (wraparound)
+		PG_RETURN_NULL();
+	else
+		return TransactionIdGetDatum(xid);
+}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 270dd21..dffe3b8 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4903,6 +4903,8 @@ DATA(insert OID = 2947 (  txid_snapshot_xip			PGNSP PGUID 12 1 50 0 0 f f f f t
 DESCR("get set of in-progress txids in snapshot");
 DATA(insert OID = 2948 (  txid_visible_in_snapshot	PGNSP PGUID 12 1  0 0 0 f f f f t f i s 2 0 16 "20 2970" _null_ _null_ _null_ _null_ _null_ txid_visible_in_snapshot _null_ _null_ _null_ ));
 DESCR("is txid visible in snapshot?");
+DATA(insert OID = 3344 (  txid_recent				PGNSP PGUID 12 1  0 0 0 f f f f t f v s 1 0 28 "20" _null_ _null_ _null_ _null_ _null_ txid_recent _null_ _null_ _null_ ));
+DESCR("get the xid from a bigint transaction id if not wrapped around");
 
 /* record comparison using normal comparison rules */
 DATA(insert OID = 2981 (  record_eq		   PGNSP PGUID 12 1 0 0 0 f f f f t f i s 2 0 16 "2249 2249" _null_ _null_ _null_ _null_ _null_ record_eq _null_ _null_ _null_ ));
diff --git a/src/test/regress/expected/txid.out b/src/test/regress/expected/txid.out
index ddd217e..ca8edb7 100644
--- a/src/test/regress/expected/txid.out
+++ b/src/test/regress/expected/txid.out
@@ -238,3 +238,73 @@ SELECT txid_snapshot '1:9223372036854775808:3';
 ERROR:  invalid input syntax for type txid_snapshot: "1:9223372036854775808:3"
 LINE 1: SELECT txid_snapshot '1:9223372036854775808:3';
                              ^
+-- test xid status functions
+BEGIN;
+SELECT txid_current() AS committed \gset
+COMMIT;
+BEGIN;
+SELECT txid_current() AS rolledback \gset
+ROLLBACK;
+BEGIN;
+SELECT txid_current() AS inprogress \gset
+-- We can reasonably assume we haven't hit the first xid
+-- wraparound here, so:
+SELECT txid_recent(:committed) = :'committed'::xid;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_recent(:rolledback) = :'rolledback'::xid;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_recent(:inprogress) = :'inprogress'::xid;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_recent(0) = '0'::xid; -- InvalidTransactionId
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_recent(1) = '1'::xid; -- BootstrapTransactionId
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_recent(2) = '2'::xid; -- FrozenTransactionId
+ ?column? 
+----------
+ t
+(1 row)
+
+COMMIT;
+BEGIN;
+CREATE FUNCTION test_future_xid(bigint)
+RETURNS void
+LANGUAGE plpgsql
+AS
+$$
+BEGIN
+  PERFORM txid_recent($1);
+  RAISE EXCEPTION 'didn''t ERROR at xid in the future as expected';
+EXCEPTION
+  WHEN invalid_parameter_value THEN
+    RAISE NOTICE 'Got expected error for xid in the future';
+END;
+$$;
+SELECT test_future_xid(:inprogress + 100);
+NOTICE:  Got expected error for xid in the future
+ test_future_xid 
+-----------------
+ 
+(1 row)
+
+ROLLBACK;
diff --git a/src/test/regress/sql/txid.sql b/src/test/regress/sql/txid.sql
index b6650b9..4c1d548 100644
--- a/src/test/regress/sql/txid.sql
+++ b/src/test/regress/sql/txid.sql
@@ -52,3 +52,43 @@ select txid_visible_in_snapshot('1000100010001015', '1000100010001000:1000100010
 -- test 64bit overflow
 SELECT txid_snapshot '1:9223372036854775807:3';
 SELECT txid_snapshot '1:9223372036854775808:3';
+
+-- test xid status functions
+BEGIN;
+SELECT txid_current() AS committed \gset
+COMMIT;
+
+BEGIN;
+SELECT txid_current() AS rolledback \gset
+ROLLBACK;
+
+BEGIN;
+SELECT txid_current() AS inprogress \gset
+
+-- We can reasonably assume we haven't hit the first xid
+-- wraparound here, so:
+SELECT txid_recent(:committed) = :'committed'::xid;
+SELECT txid_recent(:rolledback) = :'rolledback'::xid;
+SELECT txid_recent(:inprogress) = :'inprogress'::xid;
+SELECT txid_recent(0) = '0'::xid; -- InvalidTransactionId
+SELECT txid_recent(1) = '1'::xid; -- BootstrapTransactionId
+SELECT txid_recent(2) = '2'::xid; -- FrozenTransactionId
+
+COMMIT;
+
+BEGIN;
+CREATE FUNCTION test_future_xid(bigint)
+RETURNS void
+LANGUAGE plpgsql
+AS
+$$
+BEGIN
+  PERFORM txid_recent($1);
+  RAISE EXCEPTION 'didn''t ERROR at xid in the future as expected';
+EXCEPTION
+  WHEN invalid_parameter_value THEN
+    RAISE NOTICE 'Got expected error for xid in the future';
+END;
+$$;
+SELECT test_future_xid(:inprogress + 100);
+ROLLBACK;
-- 
2.5.5

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to