On 24 August 2016 at 03:10, Robert Haas <robertmh...@gmail.com> wrote:
>
> On Tue, Aug 23, 2016 at 12:59 PM, Craig Ringer <cr...@2ndquadrant.com> wrote:
> > Also fine by me. You're right, keep it simple. It means the potential set of
> > values isn't discoverable the same way, but ... meh. Using it usefully means
> > reading the docs anyway.
> >
> > The remaining 2 patches of interest are attached - txid_status() and
> > txid_convert_if_recent(). Thanks for committing txid_current_if_assigned().
> >
> > Now I'd best stop pretending I'm in a sensible timezone.
>
> I reviewed this version some more and found some more problems.


Thanks. It took me a few days to prep a new patch as I found another
issue in the process. Updated patch attached.

The updated series starts (0001) with a change to slru.c to release
the control lock when throwing an exception so that we don't deadlock
with ourselves when re-entering slru.c; explanation below.

Then there's the txid_status (0002) patch with fixes, then
txid_convert_if_recent(0003).

I omitted txid_incinerate() ; I have an updated version that sets xact
status to aborted for burned xacts instead of leaving them at 0
(in-progress), but haven't had time to finish it so it doesn't try to
blindly set the status of xacts on pages where it didn't hold
XidGenLock when the page was added to the clog.

> +    uint32 xid_epoch = (uint32)(xid_with_epoch >>32);
> +    TransactionId xid = (TransactionId)(xid_with_epoch);
>
> I think this is not project style.  In particular, I think that the
> first one needs a space after the cast and another space before the
> 32; and I think the second one has an unnecessary set of parentheses
> and needs a space added.


OK, no problems. I didn't realise spacing around casts was specified.

>
> +/*
> + * Underlying implementation of txid_status, which is mapped to an enum in
> + * system_views.sql.
> + */
>
> Not any more...


That's why I shouldn't revise a patch at 1am ;)

>
> +    if (TransactionIdIsCurrentTransactionId(xid) ||
> TransactionIdIsInProgress(xid))
> +        status = gettext_noop("in progress");
> +    else if (TransactionIdDidCommit(xid))
> +        status = gettext_noop("committed");
> +    else if (TransactionIdDidAbort(xid))
> +        status = gettext_noop("aborted");
> +    else
> +        /* shouldn't happen */
> +        ereport(ERROR,
> +            (errmsg_internal("unable to determine commit status of
> xid "UINT64_FORMAT, xid)));
>
> Maybe I'm all wet here, but it seems like there might be a problem
> here if the XID is older than the CLOG truncation point but less than
> one epoch old. get_xid_in_recent_past only guarantees that the
> transaction is less than one epoch old, not that we still have CLOG
> data for it.


Good point. The call would then fail with something like

  ERROR:  could not access status of transaction 778793573
  DETAIL:  could not open file "pg_clog/02E6": No such file or directory

This probably didn't come up in my wraparound testing because I'm
aggressively forcing wraparound by writing a lot of clog very quickly
under XidGenLock, and because I'm mostly looking at xacts that are
either recent or past the xid boundary. I've added better add coverage
for xacts around 2^30 behind the nextXid to the wraparound tests;
can't add them to txid.sql since the xid never gets that far in normal
regression testing.

What I'd really like is to be able to ask transam.c to handle the
xid_in_recent_past logic, treating an attempt to read an xid from
beyond the clog truncation threshold as a soft error indicating
unknown xact state. But that involves delving into slru.c, and I
really, really don't want to touch that for what should be a simple
and pretty noninvasive utility function.

A PG_TRY to trap ERRCODE_UNDEFINED_FILE seems like it'd be sufficient,
except for two issues:

* I see no accepted way to access the errcode etc from within the
PG_CATCH block, though. PG_RE_THROW() can do it because pg_re_throw()
is in elog.c. I couldn't find any existing code that seems to check
details about an error thrown in a PG_TRY block, only SPI calls. I
don't want to ignore all types of errors and potentially hide
problems, so I just used geterrcode() - which is meant for errcontext
callbacks - and changed the comment to say it can be used in PG_CATCH
too. I don't see why it shouldn't be.
We should probably have some sort of PG_CATCH_INFO(varname) that
exposes the top ErrorData, but that's not needed for this patch so I
left it alone.

* TransactionIdGetStatus() releases the CLogControlLock taken by
SimpleLruReadPage_ReadOnly() on normal exit but not after an exception
thrown from SlruReportIOError(). It seems appropriate for
SimpleLruReadPage() to release the LWLock before calling
SlruReportIOError(), so I've done that as a separate patch (now 0001).


I also removed the TransactionIdInProgress check in txid_status and
just assume it's in progress if it isn't our xid, committed or
aborted. TransactionIdInProgress looks like it's potentially more
expensive, and most of the time we'll be looking at committed or
aborted xacts anyway. I can't sanity-check TransactionIdInProgress
after commited/aborted, because there's then a race where the xact can
commit or abort after we decide it's not committed/aborted so it's not
in progress when we go to check that.



> And there's nothing to keep NextXID from advancing under
> us, so if somebody asks about a transaction that's just under 2^32
> transactions old, then get_xid_in_recent_past could say it's valid,
> then NextXID advances and we look up the XID extracted from the txid
> and get the status of the new transaction instead of the old one!

Hm, yeah. Though due to the clog truncation issue you noticed it
probably won't happen.

We could require that XidGenLock be held at least as LW_SHARED when
entering get_xid_in_recent_past(), but I'd rather not since that'd be
an otherwise-unnecessary lwlock for txid_convert_ifrecent().

Instead, I think I'll rename the wraparound flag to too_old and set it
if the xact is more than say 2^30 from the epoch struct's last_xid,
leaving a race window so ridiculously improbable that the nearly
impossible chance of failing with a clog access error is not a worry.
If the server's managing to have a proc stuck that long then it's
already on fire. We're only interested in reasonably recent xacts
since we can only work with xacts before wraparound / clog truncation.
This just moves the threshold for "reasonably recent" a bit closer.

All this certainly reinforces my view that users handling 'xid'
directly or trying to extract it from a bigint epoch-extended xid is a
bad idea that needs to go away soon.

-- 
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From 0b928fb91b5b3aa527372495e9fa4778c5b5bfab Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Thu, 25 Aug 2016 10:59:57 +0800
Subject: [PATCH 1/4] Release SLRU control lock before reporting I/O error

To allow callers to trap SLRU I/O errors with a PG_TRY() / PG_CATCH() block,
slru.c functions that acquire the SLRU control lock now also release the SLRU
control LWLock before reporting I/O errors.

Many SLRU functions previously took the LWLock and threw exceptions with it
still held.
---
 src/backend/access/transam/slru.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index bbae584..f020522 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -366,7 +366,8 @@ SimpleLruWaitIO(SlruCtl ctl, int slotno)
  * Return value is the shared-buffer slot number now holding the page.
  * The buffer's LRU access info is updated.
  *
- * Control lock must be held at entry, and will be held at exit.
+ * Control lock must be held at entry, and will be held at normal exit.
+ * The control lock is released if an exception is thrown.
  */
 int
 SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok,
@@ -439,7 +440,10 @@ SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok,
 
 		/* Now it's okay to ereport if we failed */
 		if (!ok)
+		{
+			LWLockRelease(shared->ControlLock);
 			SlruReportIOError(ctl, pageno, xid);
+		}
 
 		SlruRecentlyUsed(shared, slotno);
 		return slotno;
@@ -457,8 +461,9 @@ SimpleLruReadPage(SlruCtl ctl, int pageno, bool write_ok,
  * Return value is the shared-buffer slot number now holding the page.
  * The buffer's LRU access info is updated.
  *
- * Control lock must NOT be held at entry, but will be held at exit.
- * It is unspecified whether the lock will be shared or exclusive.
+ * Control lock must NOT be held at entry, but will be held at exit
+ * unless an exception is thrown. It is unspecified whether the lock
+ * will be shared or exclusive.
  */
 int
 SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid)
@@ -498,7 +503,8 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid)
  * the write).  However, we *do* attempt a fresh write even if the page
  * is already being written; this is for checkpoints.
  *
- * Control lock must be held at entry, and will be held at exit.
+ * Control lock must be held at entry, and will be held at normal exit.
+ * The lock is released if an exception is thrown.
  */
 static void
 SlruInternalWritePage(SlruCtl ctl, int slotno, SlruFlush fdata)
@@ -564,7 +570,10 @@ SlruInternalWritePage(SlruCtl ctl, int slotno, SlruFlush fdata)
 
 	/* Now it's okay to ereport if we failed */
 	if (!ok)
+	{
+		LWLockRelease(shared->ControlLock);
 		SlruReportIOError(ctl, pageno, InvalidTransactionId);
+	}
 }
 
 /*
-- 
2.5.5

From 37e3f6195a7ba58de0706f00bfec630c85d3d530 Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Fri, 19 Aug 2016 14:44:15 +0800
Subject: [PATCH 2/4] Introduce txid_status(bigint) to get status of an xact

If an appliation is disconnected while a COMMIT request is in flight,
the backend crashes mid-commit, etc, then an application may not be
sure whether or not a commit completed successfully or was rolled
back. While two-phase commit solves this it does so at a considerable
overhead, so introduce a lighter alternative.

txid_status(bigint) lets an application determine the status of a
a commit based on an xid-with-epoch as returned by txid_current()
or similar. Status may be committed, aborted, in-progress (including
prepared xacts) or null if the xact is too old for its commit status
to still be retained because it has passed the wrap-around epoch
boundary.

Applications must call txid_current() in their transactions to make
much use of this since PostgreSQL does not automatically report an xid
to the client when one is assigned.

To handle failure to access the clog when the xid specified is newer
than the wraparound threshold but older than the clog truncation
threshold we need to catch exceptions and check to see if the failure
was an I/O error. The only documented way to get the error code is for
use in errcontext callbacks, the errcode() call. There's no good
reason it can't also be used in PG_CATCH() blocks so amend the
comment in elog.c.
---
 doc/src/sgml/func.sgml             |  26 ++++++++
 src/backend/access/transam/clog.c  |  23 -------
 src/backend/utils/adt/txid.c       | 132 +++++++++++++++++++++++++++++++++++++
 src/backend/utils/error/elog.c     |   5 +-
 src/include/access/clog.h          |  23 +++++++
 src/include/catalog/pg_proc.h      |   2 +
 src/include/utils/builtins.h       |   1 +
 src/test/regress/expected/txid.out |  68 +++++++++++++++++++
 src/test/regress/sql/txid.sql      |  38 +++++++++++
 9 files changed, 293 insertions(+), 25 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index 5148095..db11f29 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17143,6 +17143,10 @@ SELECT collation for ('foo' COLLATE "de_DE");
     <primary>txid_visible_in_snapshot</primary>
    </indexterm>
 
+   <indexterm>
+    <primary>txid_status</primary>
+   </indexterm>
+
    <para>
     The functions shown in <xref linkend="functions-txid-snapshot">
     provide server transaction information in an exportable form.  The main
@@ -17193,6 +17197,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_status(<parameter>bigint</parameter>)</function></literal></entry>
+       <entry><type>txid_status</type></entry>
+       <entry>report the status of the given xact - <literal>committed</literal>, <literal>aborted</literal>, <literal>in progress</literal>, or NULL if the xid is too old</entry>
+      </row>
      </tbody>
     </tgroup>
    </table>
@@ -17263,6 +17272,23 @@ SELECT collation for ('foo' COLLATE "de_DE");
    </para>
 
    <para>
+    <function>txid_status(bigint)</> reports the commit status of a recent
+    transaction. Any recent transaction can be identified as one of
+    <itemizedlist>
+     <listitem><para>in progress</></>
+     <listitem><para>committed</></>
+     <listitem><para>aborted</></>
+    </itemizedlist>
+    Prepared transactions are identified as <literal>in progress</>.
+    The commit status of transactions older than the transaction ID wrap-around
+    threshold is no longer known by the system, so <function>txid_status</>
+    returns <literal>NULL</> for such transactions. Applications may use
+    <function>txid_status</> to determine whether a transaction committed
+    or aborted when the application and/or database server crashed or lost
+    connection while a <literal>COMMIT</literal> command was in progress.
+   </para>
+
+   <para>
     The functions shown in <xref linkend="functions-commit-timestamp">
     provide information about transactions that have been already committed.
     These functions mainly provide information about when the transactions
diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 2634476..1a6e26d 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -41,29 +41,6 @@
 #include "miscadmin.h"
 #include "pg_trace.h"
 
-/*
- * Defines for CLOG page sizes.  A page is the same BLCKSZ as is used
- * everywhere else in Postgres.
- *
- * Note: because TransactionIds are 32 bits and wrap around at 0xFFFFFFFF,
- * CLOG page numbering also wraps around at 0xFFFFFFFF/CLOG_XACTS_PER_PAGE,
- * and CLOG segment numbering at
- * 0xFFFFFFFF/CLOG_XACTS_PER_PAGE/SLRU_PAGES_PER_SEGMENT.  We need take no
- * explicit notice of that fact in this module, except when comparing segment
- * and page numbers in TruncateCLOG (see CLOGPagePrecedes).
- */
-
-/* We need two bits per xact, so four xacts fit in a byte */
-#define CLOG_BITS_PER_XACT	2
-#define CLOG_XACTS_PER_BYTE 4
-#define CLOG_XACTS_PER_PAGE (BLCKSZ * CLOG_XACTS_PER_BYTE)
-#define CLOG_XACT_BITMASK	((1 << CLOG_BITS_PER_XACT) - 1)
-
-#define TransactionIdToPage(xid)	((xid) / (TransactionId) CLOG_XACTS_PER_PAGE)
-#define TransactionIdToPgIndex(xid) ((xid) % (TransactionId) CLOG_XACTS_PER_PAGE)
-#define TransactionIdToByte(xid)	(TransactionIdToPgIndex(xid) / CLOG_XACTS_PER_BYTE)
-#define TransactionIdToBIndex(xid)	((xid) % (TransactionId) CLOG_XACTS_PER_BYTE)
-
 /* We store the latest async LSN for each group of transactions */
 #define CLOG_XACTS_PER_LSN_GROUP	32	/* keep this a power of 2 */
 #define CLOG_LSNS_PER_PAGE	(CLOG_XACTS_PER_PAGE / CLOG_XACTS_PER_LSN_GROUP)
diff --git a/src/backend/utils/adt/txid.c b/src/backend/utils/adt/txid.c
index 276075e..a0214a5 100644
--- a/src/backend/utils/adt/txid.c
+++ b/src/backend/utils/adt/txid.c
@@ -21,6 +21,7 @@
 
 #include "postgres.h"
 
+#include "access/clog.h"
 #include "access/transam.h"
 #include "access/xact.h"
 #include "access/xlog.h"
@@ -117,6 +118,64 @@ 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
+ * too_old param to true if wraparound for this xid is close,
+ * otherwise false.
+ *
+ * To guard against wraparound occurring just after we check the xid,
+ * such that the xid is not wrapped when we return it but wraps
+ * shortly afterwards, treat xids more than 2^30 from the current
+ * nextXid as too old.  That way there's a large margin of nextXid
+ * growth before TransactionIdPrecedes would start reporting the
+ * queried xid as in the future relative to new xacts as it crosses
+ * the 2^31 difference boundary.
+ *
+ * In practice the clog will usually be truncated away for such xids
+ * already so they're of limited utility.
+ */
+static TransactionId
+get_xid_in_recent_past(txid xid_with_epoch, bool *too_old)
+{
+	uint32			xid_epoch = (uint32) (xid_with_epoch >> 32);
+	TransactionId	xid = (TransactionId) xid_with_epoch;
+	TxidEpoch		now_epoch;
+	const int		wraparound_safety_margin = 1 << 30;
+
+	load_xid_epoch(&now_epoch);
+
+	*too_old = 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
+				 && (now_epoch.last_xid - xid) > wraparound_safety_margin))
+	{
+		/* xid too far in the past */
+		*too_old = true;
+	}
+	else
+	{
+		Assert(TransactionIdPrecedesOrEquals(xid, now_epoch.last_xid));
+	}
+
+	return xid;
+}
+
+/*
  * txid comparator for qsort/bsearch
  */
 static int
@@ -354,6 +413,9 @@ bad_format:
  *
  *	Return the current toplevel transaction ID as TXID
  *	If the current transaction does not have one, one is assigned.
+ *
+ *	This value has the epoch as the high 32 bits and the 32-bit xid
+ *	as the low 32 bits.
  */
 Datum
 txid_current(PG_FUNCTION_ARGS)
@@ -658,3 +720,73 @@ txid_snapshot_xip(PG_FUNCTION_ARGS)
 		SRF_RETURN_DONE(fctx);
 	}
 }
+
+/*
+ * Report the status of a recent transaction ID, or null for wrapped,
+ * truncated away or otherwise too old XIDs.
+ */
+Datum
+txid_status(PG_FUNCTION_ARGS)
+{
+	const char	   *status;
+	bool			too_old;
+	uint64			xid_with_epoch = PG_GETARG_INT64(0);
+	TransactionId	xid = get_xid_in_recent_past(xid_with_epoch, &too_old);
+
+	if (!TransactionIdIsValid(xid))
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("transaction ID "UINT64_FORMAT" is an invalid xid",
+						xid_with_epoch)));
+	}
+
+	if (too_old)
+		status = NULL;
+	else if (TransactionIdIsCurrentTransactionId(xid))
+		status = gettext_noop("in progress");
+	else
+	{
+		/*
+		 * clog access might fail here if the requested xid is past the
+		 * clog truncation threshold but not wrapped around. We want to
+		 * return null in such cases rather than raise an error.
+		 */
+		PG_TRY();
+		{
+			if (TransactionIdDidCommit(xid))
+				status = gettext_noop("committed");
+			else if (TransactionIdDidAbort(xid))
+				status = gettext_noop("aborted");
+			else
+				/*
+				 * can't test TransactionIdIsInProgress here or we race
+				 * with concurrent commit/abort. There's no point anyway,
+				 * since it might then commit/abort just after we check.
+				 */
+				status = gettext_noop("in progress");
+		}
+		PG_CATCH();
+		{
+			if (geterrcode() == ERRCODE_UNDEFINED_FILE)
+			{
+				/*
+				 * The clog SLRU reports that it can't find the segment for
+				 * the xid of interest. Presumably it has been truncated
+				 * away, so the xact is too old to report status for.
+				 */
+				elog(INFO, "clog access failed with access error");
+				status = NULL;
+			}
+			else
+				PG_RE_THROW();
+
+		}
+		PG_END_TRY();
+	}
+
+	if (status == NULL)
+		PG_RETURN_NULL();
+	else
+		PG_RETURN_TEXT_P(cstring_to_text(status));
+}
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 224ee78..10bca81 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -1233,8 +1233,9 @@ set_errdata_field(MemoryContextData *cxt, char **ptr, const char *str)
 /*
  * geterrcode --- return the currently set SQLSTATE error code
  *
- * This is only intended for use in error callback subroutines, since there
- * is no other place outside elog.c where the concept is meaningful.
+ * This is only intended for use in error callback subroutines and PG_CATCH
+ * blocks, since there is no other place outside elog.c where the concept is
+ * meaningful.
  */
 int
 geterrcode(void)
diff --git a/src/include/access/clog.h b/src/include/access/clog.h
index 06c069a..a763dfb 100644
--- a/src/include/access/clog.h
+++ b/src/include/access/clog.h
@@ -28,6 +28,29 @@ typedef int XidStatus;
 #define TRANSACTION_STATUS_ABORTED			0x02
 #define TRANSACTION_STATUS_SUB_COMMITTED	0x03
 
+/*
+ * Defines for CLOG page sizes.  A page is the same BLCKSZ as is used
+ * everywhere else in Postgres.
+ *
+ * Note: because TransactionIds are 32 bits and wrap around at 0xFFFFFFFF,
+ * CLOG page numbering also wraps around at 0xFFFFFFFF/CLOG_XACTS_PER_PAGE,
+ * and CLOG segment numbering at
+ * 0xFFFFFFFF/CLOG_XACTS_PER_PAGE/SLRU_PAGES_PER_SEGMENT.  We need take no
+ * explicit notice of that fact in this module, except when comparing segment
+ * and page numbers in TruncateCLOG (see CLOGPagePrecedes).
+ */
+
+/* We need two bits per xact, so four xacts fit in a byte */
+#define CLOG_BITS_PER_XACT	2
+#define CLOG_XACTS_PER_BYTE 4
+#define CLOG_XACTS_PER_PAGE (BLCKSZ * CLOG_XACTS_PER_BYTE)
+#define CLOG_XACT_BITMASK	((1 << CLOG_BITS_PER_XACT) - 1)
+
+#define TransactionIdToPage(xid)	((xid) / (TransactionId) CLOG_XACTS_PER_PAGE)
+#define TransactionIdToPgIndex(xid) ((xid) % (TransactionId) CLOG_XACTS_PER_PAGE)
+#define TransactionIdToByte(xid)	(TransactionIdToPgIndex(xid) / CLOG_XACTS_PER_BYTE)
+#define TransactionIdToBIndex(xid)	((xid) % (TransactionId) CLOG_XACTS_PER_BYTE)
+
 
 extern void TransactionIdSetTreeStatus(TransactionId xid, int nsubxids,
 				   TransactionId *subxids, XidStatus status, XLogRecPtr lsn);
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index e2d08ba..0ad870c 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4928,6 +4928,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 = 3346 (  txid_status				PGNSP PGUID 12 1  0 0 0 f f f f t f v s 1 0 25 "20" _null_ _null_ _null_ _null_ _null_ txid_status _null_ _null_ _null_ ));
+DESCR("commit status of transaction");
 
 /* 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/include/utils/builtins.h b/src/include/utils/builtins.h
index 2ae212a..baffa38 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -1227,6 +1227,7 @@ extern Datum txid_snapshot_xmin(PG_FUNCTION_ARGS);
 extern Datum txid_snapshot_xmax(PG_FUNCTION_ARGS);
 extern Datum txid_snapshot_xip(PG_FUNCTION_ARGS);
 extern Datum txid_visible_in_snapshot(PG_FUNCTION_ARGS);
+extern Datum txid_status(PG_FUNCTION_ARGS);
 
 /* uuid.c */
 extern Datum uuid_in(PG_FUNCTION_ARGS);
diff --git a/src/test/regress/expected/txid.out b/src/test/regress/expected/txid.out
index 802ccb9..1a0b84c 100644
--- a/src/test/regress/expected/txid.out
+++ b/src/test/regress/expected/txid.out
@@ -254,3 +254,71 @@ SELECT txid_current_if_assigned() IS NOT DISTINCT FROM BIGINT :'txid_current';
 (1 row)
 
 COMMIT;
+-- 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
+SELECT txid_status(:committed) AS committed;
+ committed 
+-----------
+ committed
+(1 row)
+
+SELECT txid_status(:rolledback) AS rolledback;
+ rolledback 
+------------
+ aborted
+(1 row)
+
+SELECT txid_status(:inprogress) AS inprogress;
+ inprogress  
+-------------
+ in progress
+(1 row)
+
+SELECT txid_status(1); -- BootstrapTransactionId is always committed
+ txid_status 
+-------------
+ committed
+(1 row)
+
+SELECT txid_status(2); -- FrozenTransactionId is always committed
+ txid_status 
+-------------
+ committed
+(1 row)
+
+SELECT txid_status(3); -- FirstNormalTransactionId is always committed for the first epoch because of initdb work
+ txid_status 
+-------------
+ committed
+(1 row)
+
+COMMIT;
+BEGIN;
+CREATE FUNCTION test_future_xid_status(bigint)
+RETURNS void
+LANGUAGE plpgsql
+AS
+$$
+BEGIN
+  PERFORM txid_status($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_status(:inprogress + 10000);
+NOTICE:  Got expected error for xid in the future
+ test_future_xid_status 
+------------------------
+ 
+(1 row)
+
+ROLLBACK;
diff --git a/src/test/regress/sql/txid.sql b/src/test/regress/sql/txid.sql
index 4aefd9e..1c56ec1 100644
--- a/src/test/regress/sql/txid.sql
+++ b/src/test/regress/sql/txid.sql
@@ -59,3 +59,41 @@ SELECT txid_current_if_assigned() IS NULL;
 SELECT txid_current() \gset
 SELECT txid_current_if_assigned() IS NOT DISTINCT FROM BIGINT :'txid_current';
 COMMIT;
+
+-- 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
+
+SELECT txid_status(:committed) AS committed;
+SELECT txid_status(:rolledback) AS rolledback;
+SELECT txid_status(:inprogress) AS inprogress;
+SELECT txid_status(1); -- BootstrapTransactionId is always committed
+SELECT txid_status(2); -- FrozenTransactionId is always committed
+SELECT txid_status(3); -- FirstNormalTransactionId is always committed for the first epoch because of initdb work
+
+COMMIT;
+
+BEGIN;
+CREATE FUNCTION test_future_xid_status(bigint)
+RETURNS void
+LANGUAGE plpgsql
+AS
+$$
+BEGIN
+  PERFORM txid_status($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_status(:inprogress + 10000);
+ROLLBACK;
-- 
2.5.5

From 65faf9c9e930a1e1aa3267dd03606f30fbb740bf Mon Sep 17 00:00:00 2001
From: Craig Ringer <cr...@2ndquadrant.com>
Date: Fri, 19 Aug 2016 14:49:52 +0800
Subject: [PATCH 3/4] Add txid_convert_if_recent() to get the 32-bit xid from a
 bigint xid

txid_current() returns an epoch-extended 64-bit xid as a bigint, but
many PostgreSQL functions take and many views report the narrow 32-bit
'xid' type that's subject to wrap-around. To compare these apps must
currently bit-shift the 64-bit xid down and they have no way to check
the epoch.

Add a function that returns the downshifted xid if it's in the current
epoch, or null if the xid is too far in the past and cannot be
compared with any 'xid' value in the current server epoch.
---
 doc/src/sgml/func.sgml                    |  17 ++++-
 src/backend/utils/adt/txid.c              |  12 ++++
 src/include/catalog/pg_proc.h             |   4 +-
 src/test/regress/expected/alter_table.out |   4 +-
 src/test/regress/expected/txid.out        | 100 ++++++++++++++++++++++++++++++
 src/test/regress/sql/alter_table.sql      |   4 +-
 src/test/regress/sql/txid.sql             |  51 +++++++++++++++
 7 files changed, 184 insertions(+), 8 deletions(-)

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index db11f29..1c6f830 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -17198,6 +17198,11 @@ SELECT collation for ('foo' COLLATE "de_DE");
        <entry>is transaction ID visible in snapshot? (do not use with subtransaction ids)</entry>
       </row>
       <row>
+       <entry><literal><function>txid_convert_if_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>
+      <row>
        <entry><literal><function>txid_status(<parameter>bigint</parameter>)</function></literal></entry>
        <entry><type>txid_status</type></entry>
        <entry>report the status of the given xact - <literal>committed</literal>, <literal>aborted</literal>, <literal>in progress</literal>, or NULL if the xid is too old</entry>
@@ -17210,9 +17215,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_convert_if_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 a0214a5..6302a70 100644
--- a/src/backend/utils/adt/txid.c
+++ b/src/backend/utils/adt/txid.c
@@ -721,6 +721,18 @@ txid_snapshot_xip(PG_FUNCTION_ARGS)
 	}
 }
 
+Datum
+txid_convert_if_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);
+}
+
 /*
  * Report the status of a recent transaction ID, or null for wrapped,
  * truncated away or otherwise too old XIDs.
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 0ad870c..59fa907 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -4928,8 +4928,10 @@ 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 = 3346 (  txid_status				PGNSP PGUID 12 1  0 0 0 f f f f t f v s 1 0 25 "20" _null_ _null_ _null_ _null_ _null_ txid_status _null_ _null_ _null_ ));
+DATA(insert OID = 3347 (  txid_status				PGNSP PGUID 12 1  0 0 0 f f f f t f v s 1 0 25 "20" _null_ _null_ _null_ _null_ _null_ txid_status _null_ _null_ _null_ ));
 DESCR("commit status of transaction");
+DATA(insert OID = 3344 (  txid_convert_if_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_convert_if_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/alter_table.out b/src/test/regress/expected/alter_table.out
index 3232cda..3bdbb87 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2029,7 +2029,7 @@ from pg_locks l join pg_class c on l.relation = c.oid
 where virtualtransaction = (
         select virtualtransaction
         from pg_locks
-        where transactionid = txid_current()::integer)
+        where transactionid is not distinct from txid_convert_if_recent(txid_current()) )
 and locktype = 'relation'
 and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
 and c.relname != 'my_locks'
@@ -2192,7 +2192,7 @@ from pg_locks l join pg_class c on l.relation = c.oid
 where virtualtransaction = (
         select virtualtransaction
         from pg_locks
-        where transactionid = txid_current()::integer)
+        where transactionid is not distinct from txid_convert_if_recent(txid_current()) )
 and locktype = 'relation'
 and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
 and c.relname = 'my_locks'
diff --git a/src/test/regress/expected/txid.out b/src/test/regress/expected/txid.out
index 1a0b84c..01c5673 100644
--- a/src/test/regress/expected/txid.out
+++ b/src/test/regress/expected/txid.out
@@ -263,6 +263,63 @@ 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_convert_if_recent(:committed) = :'committed'::xid;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_convert_if_recent(:rolledback) = :'rolledback'::xid;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_convert_if_recent(:inprogress) = :'inprogress'::xid;
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_convert_if_recent(0) = '0'::xid; -- InvalidTransactionId
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_convert_if_recent(1) = '1'::xid; -- BootstrapTransactionId
+ ?column? 
+----------
+ t
+(1 row)
+
+SELECT txid_convert_if_recent(2) = '2'::xid; -- FrozenTransactionId
+ ?column? 
+----------
+ t
+(1 row)
+
+-- we ignore epoch for the fixed xids
+SELECT txid_convert_if_recent(BIGINT '1' << 32);
+ txid_convert_if_recent 
+------------------------
+                      0
+(1 row)
+
+SELECT txid_convert_if_recent((BIGINT '1' << 32) + 1);
+ txid_convert_if_recent 
+------------------------
+                      1
+(1 row)
+
+SELECT txid_convert_if_recent((BIGINT '1' << 32) + 2);
+ txid_convert_if_recent 
+------------------------
+                      2
+(1 row)
+
 SELECT txid_status(:committed) AS committed;
  committed 
 -----------
@@ -300,6 +357,49 @@ SELECT txid_status(3); -- FirstNormalTransactionId is always committed for the f
 (1 row)
 
 COMMIT;
+-- Check xids in the future
+DO
+$$
+BEGIN
+  PERFORM txid_convert_if_recent(txid_current() + (BIGINT '1' << 32));
+EXCEPTION
+  WHEN invalid_parameter_value THEN
+    RAISE NOTICE 'got expected xid out of range error';
+END;
+$$;
+NOTICE:  got expected xid out of range error
+DO
+$$
+BEGIN
+  PERFORM txid_convert_if_recent((BIGINT '1' << 32) - 1);
+EXCEPTION
+  WHEN invalid_parameter_value THEN
+    RAISE NOTICE 'got expected xid out of range error';
+END;
+$$;
+NOTICE:  got expected xid out of range error
+BEGIN;
+CREATE FUNCTION test_future_xid(bigint)
+RETURNS void
+LANGUAGE plpgsql
+AS
+$$
+BEGIN
+  PERFORM txid_convert_if_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 + 100000);
+NOTICE:  Got expected error for xid in the future
+ test_future_xid 
+-----------------
+ 
+(1 row)
+
+ROLLBACK;
 BEGIN;
 CREATE FUNCTION test_future_xid_status(bigint)
 RETURNS void
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 72e65d4..124d71f 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1335,7 +1335,7 @@ from pg_locks l join pg_class c on l.relation = c.oid
 where virtualtransaction = (
         select virtualtransaction
         from pg_locks
-        where transactionid = txid_current()::integer)
+        where transactionid is not distinct from txid_convert_if_recent(txid_current()) )
 and locktype = 'relation'
 and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
 and c.relname != 'my_locks'
@@ -1422,7 +1422,7 @@ from pg_locks l join pg_class c on l.relation = c.oid
 where virtualtransaction = (
         select virtualtransaction
         from pg_locks
-        where transactionid = txid_current()::integer)
+        where transactionid is not distinct from txid_convert_if_recent(txid_current()) )
 and locktype = 'relation'
 and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
 and c.relname = 'my_locks'
diff --git a/src/test/regress/sql/txid.sql b/src/test/regress/sql/txid.sql
index 1c56ec1..454aba4 100644
--- a/src/test/regress/sql/txid.sql
+++ b/src/test/regress/sql/txid.sql
@@ -72,6 +72,19 @@ ROLLBACK;
 BEGIN;
 SELECT txid_current() AS inprogress \gset
 
+-- We can reasonably assume we haven't hit the first xid
+-- wraparound here, so:
+SELECT txid_convert_if_recent(:committed) = :'committed'::xid;
+SELECT txid_convert_if_recent(:rolledback) = :'rolledback'::xid;
+SELECT txid_convert_if_recent(:inprogress) = :'inprogress'::xid;
+SELECT txid_convert_if_recent(0) = '0'::xid; -- InvalidTransactionId
+SELECT txid_convert_if_recent(1) = '1'::xid; -- BootstrapTransactionId
+SELECT txid_convert_if_recent(2) = '2'::xid; -- FrozenTransactionId
+-- we ignore epoch for the fixed xids
+SELECT txid_convert_if_recent(BIGINT '1' << 32);
+SELECT txid_convert_if_recent((BIGINT '1' << 32) + 1);
+SELECT txid_convert_if_recent((BIGINT '1' << 32) + 2);
+
 SELECT txid_status(:committed) AS committed;
 SELECT txid_status(:rolledback) AS rolledback;
 SELECT txid_status(:inprogress) AS inprogress;
@@ -81,6 +94,44 @@ SELECT txid_status(3); -- FirstNormalTransactionId is always committed for the f
 
 COMMIT;
 
+-- Check xids in the future
+DO
+$$
+BEGIN
+  PERFORM txid_convert_if_recent(txid_current() + (BIGINT '1' << 32));
+EXCEPTION
+  WHEN invalid_parameter_value THEN
+    RAISE NOTICE 'got expected xid out of range error';
+END;
+$$;
+
+DO
+$$
+BEGIN
+  PERFORM txid_convert_if_recent((BIGINT '1' << 32) - 1);
+EXCEPTION
+  WHEN invalid_parameter_value THEN
+    RAISE NOTICE 'got expected xid out of range error';
+END;
+$$;
+
+BEGIN;
+CREATE FUNCTION test_future_xid(bigint)
+RETURNS void
+LANGUAGE plpgsql
+AS
+$$
+BEGIN
+  PERFORM txid_convert_if_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 + 100000);
+ROLLBACK;
+
 BEGIN;
 CREATE FUNCTION test_future_xid_status(bigint)
 RETURNS void
-- 
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