Hi,
As promised I'm currently updating the patch. Some questions arose
during that:
* Should HeapTupleHeaderXminFrozen also check for FrozenTransactionId?
It seems quite possible that people think they've delt with frozen
xmin entirely after checking, but they still might get
FrozenTransactionId back in a pg_upgraded cluster.
* Existing htup_details boolean checks contain an 'Is', but
HeapTupleHeaderXminCommitted, HeapTupleHeaderXminInvalid,
HeapTupleHeaderXminFrozen don't contain any verb. Not sure.
* EvalPlanQualFetch() uses priorXmax like logic to find updated versions
of tuples. I am not 100% sure there aren't cases where that's
problematic even with the current code, but I think it's better not to
use the raw xid there - priorXmax can never be FrozenTransactionId, so
comparing it to the GetXmin() seems better.
It also has the following wrong comment:
* .... (Should be safe to examine xmin without getting
* buffer's content lock, since xmin never changes in an existing
* tuple.)
But I don't see that causing any problems.
* ri_trigger.c did do a TransactionIdIsCurrentTransactionId() on a raw
xmin value, the consequences are minor though.
The rewrite to introduce HeapTupleHeaderGet[Raw]Xmin() indeed somewhat
increases the patchsize - but I think it's enough increase in
expressiveness to be well worth the cost. Indeed it led me to find at
least one issue (with minor consequences).
I think once we have this we should start opportunistically try to
freeze tuples during vacuum using OldestXmin instead of FreezeLimit if
the page is already dirty.
Patch 01 is a rebased version of Robert's patch without further changes,
02 contains my suggested modifications.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
>From ac6c9a15836414208c49393d05d2d7d29e8fae4a Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 21 Nov 2013 12:57:20 +0100
Subject: [PATCH 1/2] freeze-forensically-v1
Robert Haas
---
src/backend/access/heap/heapam.c | 50 +++++++++++++++--------------------
src/backend/access/heap/rewriteheap.c | 1 +
src/backend/catalog/index.c | 1 +
src/backend/commands/analyze.c | 1 +
src/backend/commands/cluster.c | 1 +
src/backend/commands/sequence.c | 4 +--
src/backend/commands/typecmds.c | 3 ++-
src/backend/commands/vacuumlazy.c | 9 +++++--
src/backend/optimizer/util/plancat.c | 1 +
src/backend/storage/lmgr/predicate.c | 7 ++++-
src/backend/utils/adt/ri_triggers.c | 6 +++--
src/backend/utils/cache/relcache.c | 8 ++++--
src/backend/utils/time/combocid.c | 8 +++---
src/backend/utils/time/tqual.c | 31 +++++++++++-----------
src/include/access/htup_details.h | 35 +++++++++++++++++++++++-
15 files changed, 108 insertions(+), 58 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index a21f31b..eb56230 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2221,13 +2221,9 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
tup->t_data->t_infomask2 &= ~(HEAP2_XACT_MASK);
tup->t_data->t_infomask |= HEAP_XMAX_INVALID;
+ HeapTupleHeaderSetXmin(tup->t_data, xid);
if (options & HEAP_INSERT_FROZEN)
- {
- tup->t_data->t_infomask |= HEAP_XMIN_COMMITTED;
- HeapTupleHeaderSetXmin(tup->t_data, FrozenTransactionId);
- }
- else
- HeapTupleHeaderSetXmin(tup->t_data, xid);
+ HeapTupleHeaderSetXminFrozen(tup->t_data);
HeapTupleHeaderSetCmin(tup->t_data, cid);
HeapTupleHeaderSetXmax(tup->t_data, 0); /* for cleanliness */
tup->t_tableOid = RelationGetRelid(relation);
@@ -5120,19 +5116,15 @@ heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
bool changed = false;
TransactionId xid;
- xid = HeapTupleHeaderGetXmin(tuple);
- if (TransactionIdIsNormal(xid) &&
- TransactionIdPrecedes(xid, cutoff_xid))
+ if (!HeapTupleHeaderXminFrozen(tuple))
{
- HeapTupleHeaderSetXmin(tuple, FrozenTransactionId);
-
- /*
- * Might as well fix the hint bits too; usually XMIN_COMMITTED will
- * already be set here, but there's a small chance not.
- */
- Assert(!(tuple->t_infomask & HEAP_XMIN_INVALID));
- tuple->t_infomask |= HEAP_XMIN_COMMITTED;
- changed = true;
+ xid = HeapTupleHeaderGetXmin(tuple);
+ if (TransactionIdIsNormal(xid) &&
+ TransactionIdPrecedes(xid, cutoff_xid))
+ {
+ HeapTupleHeaderSetXminFrozen(tuple);
+ changed = true;
+ }
}
/*
@@ -5185,8 +5177,7 @@ heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
* Might as well fix the hint bits too; usually XMIN_COMMITTED
* will already be set here, but there's a small chance not.
*/
- Assert(!(tuple->t_infomask & HEAP_XMIN_INVALID));
- tuple->t_infomask |= HEAP_XMIN_COMMITTED;
+ HeapTupleHeaderSetXminCommitted(tuple);
changed = true;
}
}
@@ -5477,10 +5468,13 @@ heap_tuple_needs_freeze(HeapTupleHeader tuple, TransactionId cutoff_xid,
{
TransactionId xid;
- xid = HeapTupleHeaderGetXmin(tuple);
- if (TransactionIdIsNormal(xid) &&
- TransactionIdPrecedes(xid, cutoff_xid))
- return true;
+ if (!HeapTupleHeaderXminFrozen(tuple))
+ {
+ xid = HeapTupleHeaderGetXmin(tuple);
+ if (TransactionIdIsNormal(xid) &&
+ TransactionIdPrecedes(xid, cutoff_xid))
+ return true;
+ }
if (!(tuple->t_infomask & HEAP_XMAX_INVALID))
{
@@ -5606,12 +5600,10 @@ HeapTupleHeaderAdvanceLatestRemovedXid(HeapTupleHeader tuple,
* This needs to work on both master and standby, where it is used to
* assess btree delete records.
*/
- if ((tuple->t_infomask & HEAP_XMIN_COMMITTED) ||
- (!(tuple->t_infomask & HEAP_XMIN_COMMITTED) &&
- !(tuple->t_infomask & HEAP_XMIN_INVALID) &&
- TransactionIdDidCommit(xmin)))
+ if (HeapTupleHeaderXminCommitted(tuple) ||
+ (!HeapTupleHeaderXminInvalid(tuple) && TransactionIdDidCommit(xmin)))
{
- if (xmax != xmin &&
+ if ((HeapTupleHeaderXminFrozen(tuple) || xmax != xmin) &&
TransactionIdFollows(xmax, *latestRemovedXid))
*latestRemovedXid = xmax;
}
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 951894c..f236a46 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -437,6 +437,7 @@ rewrite_heap_tuple(RewriteState state,
* RECENTLY_DEAD if and only if the xmin is not before OldestXmin.
*/
if ((new_tuple->t_data->t_infomask & HEAP_UPDATED) &&
+ !HeapTupleHeaderXminFrozen(new_tuple->t_data) &&
!TransactionIdPrecedes(HeapTupleHeaderGetXmin(new_tuple->t_data),
state->rs_oldest_xmin))
{
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 0275240..48e0c2a 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2277,6 +2277,7 @@ IndexBuildHeapScan(Relation heapRelation,
* before commit there. Give a warning if neither case
* applies.
*/
+ Assert(!HeapTupleHeaderXminFrozen(heapTuple->t_data));
xwait = HeapTupleHeaderGetXmin(heapTuple->t_data);
if (!TransactionIdIsCurrentTransactionId(xwait))
{
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 9845b0b..9568918 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1178,6 +1178,7 @@ acquire_sample_rows(Relation onerel, int elevel,
* has to adjust the numbers we send to the stats
* collector to make this come out right.)
*/
+ Assert(!HeapTupleHeaderXminFrozen(targtuple.t_data));
if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(targtuple.t_data)))
{
sample_it = true;
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index f6a5bfe..8b6e7e1 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -993,6 +993,7 @@ copy_heap_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
* there. Give a warning if neither case applies; but in any
* case we had better copy it.
*/
+ Assert(!HeapTupleHeaderXminFrozen(tuple->t_data));
if (!is_system_catalog &&
!TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple->t_data)))
elog(WARNING, "concurrent insert in progress within table \"%s\"",
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index 67b8a5d..24c7d07 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -360,10 +360,10 @@ fill_seq_with_data(Relation rel, HeapTuple tuple)
item = PageGetItem((Page) page, itemId);
HeapTupleHeaderSetXmin((HeapTupleHeader) item, FrozenTransactionId);
- ((HeapTupleHeader) item)->t_infomask |= HEAP_XMIN_COMMITTED;
+ HeapTupleHeaderSetXminFrozen((HeapTupleHeader) item);
HeapTupleHeaderSetXmin(tuple->t_data, FrozenTransactionId);
- tuple->t_data->t_infomask |= HEAP_XMIN_COMMITTED;
+ HeapTupleHeaderSetXminFrozen(tuple->t_data);
}
MarkBufferDirty(buf);
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index d4a14ca..4ad8386 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1202,7 +1202,8 @@ AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
* cases that could theoretically be safe; but fortunately pg_dump only
* needs the simplest case.
*/
- if (HeapTupleHeaderGetXmin(tup->t_data) == GetCurrentTransactionId() &&
+ if (!HeapTupleHeaderXminFrozen(tup->t_data) &&
+ HeapTupleHeaderGetXmin(tup->t_data) == GetCurrentTransactionId() &&
!(tup->t_data->t_infomask & HEAP_UPDATED))
/* safe to do inside transaction block */ ;
else
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 6778c7d..7ac1352 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -812,16 +812,19 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
{
TransactionId xmin;
- if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple.t_data))
{
all_visible = false;
break;
}
+
/*
* The inserter definitely committed. But is it old
* enough that everyone sees it as committed?
*/
+ if (HeapTupleHeaderXminFrozen(tuple.t_data))
+ break;
xmin = HeapTupleHeaderGetXmin(tuple.t_data);
if (!TransactionIdPrecedes(xmin, OldestXmin))
{
@@ -1736,7 +1739,7 @@ heap_page_is_all_visible(Relation rel, Buffer buf, TransactionId *visibility_cut
TransactionId xmin;
/* Check comments in lazy_scan_heap. */
- if (!(tuple.t_data->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple.t_data))
{
all_visible = false;
break;
@@ -1746,6 +1749,8 @@ heap_page_is_all_visible(Relation rel, Buffer buf, TransactionId *visibility_cut
* The inserter definitely committed. But is it old enough
* that everyone sees it as committed?
*/
+ if (HeapTupleHeaderXminFrozen(tuple.t_data))
+ break;
xmin = HeapTupleHeaderGetXmin(tuple.t_data);
if (!TransactionIdPrecedes(xmin, OldestXmin))
{
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 954666c..e22be0e 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -188,6 +188,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
* src/backend/access/heap/README.HOT for discussion.
*/
if (index->indcheckxmin &&
+ !HeapTupleHeaderXminFrozen(indexRelation->rd_indextuple->t_data) &&
!TransactionIdPrecedes(HeapTupleHeaderGetXmin(indexRelation->rd_indextuple->t_data),
TransactionXmin))
{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index a8a0e98..77d87c7 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -2475,7 +2475,10 @@ PredicateLockTuple(Relation relation, HeapTuple tuple, Snapshot snapshot)
{
TransactionId myxid;
- targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
+ if (HeapTupleHeaderXminFrozen(tuple->t_data))
+ targetxmin = FrozenTransactionId;
+ else
+ targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
myxid = GetTopTransactionIdIfAny();
if (TransactionIdIsValid(myxid))
@@ -3898,6 +3901,7 @@ CheckForSerializableConflictOut(bool visible, Relation relation,
case HEAPTUPLE_LIVE:
if (visible)
return;
+ Assert(!HeapTupleHeaderXminFrozen(tuple->t_data));
xid = HeapTupleHeaderGetXmin(tuple->t_data);
break;
case HEAPTUPLE_RECENTLY_DEAD:
@@ -3909,6 +3913,7 @@ CheckForSerializableConflictOut(bool visible, Relation relation,
xid = HeapTupleHeaderGetUpdateXid(tuple->t_data);
break;
case HEAPTUPLE_INSERT_IN_PROGRESS:
+ Assert(!HeapTupleHeaderXminFrozen(tuple->t_data));
xid = HeapTupleHeaderGetXmin(tuple->t_data);
break;
case HEAPTUPLE_DEAD:
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index 917130f..ed906ee 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2166,7 +2166,8 @@ RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel,
* UPDATE check. (We could skip this if we knew the INSERT
* trigger already fired, but there is no easy way to know that.)
*/
- if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
+ if (!HeapTupleHeaderXminFrozen(old_row->t_data) &&
+ TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
return true;
/* If all old and new key values are equal, no check is needed */
@@ -2204,7 +2205,8 @@ RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel,
* UPDATE check. (We could skip this if we knew the INSERT
* trigger already fired, but there is no easy way to know that.)
*/
- if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
+ if (!HeapTupleHeaderXminFrozen(old_row->t_data) &&
+ TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
return true;
/* If all old and new key values are equal, no check is needed */
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index d0acca8..4b92fbf 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1764,6 +1764,7 @@ RelationReloadIndexInfo(Relation relation)
{
HeapTuple tuple;
Form_pg_index index;
+ TransactionId xmin;
tuple = SearchSysCache1(INDEXRELID,
ObjectIdGetDatum(RelationGetRelid(relation)));
@@ -1789,8 +1790,11 @@ RelationReloadIndexInfo(Relation relation)
relation->rd_index->indislive = index->indislive;
/* Copy xmin too, as that is needed to make sense of indcheckxmin */
- HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data,
- HeapTupleHeaderGetXmin(tuple->t_data));
+ if (HeapTupleHeaderXminFrozen(relation->rd_indextuple->t_data))
+ xmin = FrozenTransactionId;
+ else
+ xmin = HeapTupleHeaderGetXmin(relation->rd_indextuple->t_data);
+ HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data, xmin);
ReleaseSysCache(tuple);
}
diff --git a/src/backend/utils/time/combocid.c b/src/backend/utils/time/combocid.c
index 923355d..baacd4a 100644
--- a/src/backend/utils/time/combocid.c
+++ b/src/backend/utils/time/combocid.c
@@ -148,10 +148,12 @@ HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
/*
* If we're marking a tuple deleted that was inserted by (any
* subtransaction of) our transaction, we need to use a combo command id.
- * Test for HEAP_XMIN_COMMITTED first, because it's cheaper than a
- * TransactionIdIsCurrentTransactionId call.
+ * It's cheaper to test HeapTupleHeaderXminCommitted before checking
+ * TransactionIdIsCurrentTransactionId, but it's also necessary for
+ * correctness: if HeapTupleHeaderXminCommitted returns true, the tuple
+ * might even be frozen.
*/
- if (!(tup->t_infomask & HEAP_XMIN_COMMITTED) &&
+ if (!HeapTupleHeaderXminCommitted(tup) &&
TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tup)))
{
CommandId cmin = HeapTupleHeaderGetCmin(tup);
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index ed66c49..bf8c9919 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -166,9 +166,9 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer)
Assert(ItemPointerIsValid(&htup->t_self));
Assert(htup->t_tableOid != InvalidOid);
- if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple))
{
- if (tuple->t_infomask & HEAP_XMIN_INVALID)
+ if (HeapTupleHeaderXminInvalid(tuple))
return false;
/* Used by pre-9.0 binary upgrades */
@@ -352,9 +352,9 @@ HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot,
Assert(ItemPointerIsValid(&htup->t_self));
Assert(htup->t_tableOid != InvalidOid);
- if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple))
{
- if (tuple->t_infomask & HEAP_XMIN_INVALID)
+ if (HeapTupleHeaderXminInvalid(tuple))
return false;
/* Used by pre-9.0 binary upgrades */
@@ -437,9 +437,9 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
Assert(ItemPointerIsValid(&htup->t_self));
Assert(htup->t_tableOid != InvalidOid);
- if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple))
{
- if (tuple->t_infomask & HEAP_XMIN_INVALID)
+ if (HeapTupleHeaderXminInvalid(tuple))
return HeapTupleInvisible;
/* Used by pre-9.0 binary upgrades */
@@ -665,9 +665,9 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
snapshot->xmin = snapshot->xmax = InvalidTransactionId;
- if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple))
{
- if (tuple->t_infomask & HEAP_XMIN_INVALID)
+ if (HeapTupleHeaderXminInvalid(tuple))
return false;
/* Used by pre-9.0 binary upgrades */
@@ -855,9 +855,9 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
Assert(ItemPointerIsValid(&htup->t_self));
Assert(htup->t_tableOid != InvalidOid);
- if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple))
{
- if (tuple->t_infomask & HEAP_XMIN_INVALID)
+ if (HeapTupleHeaderXminInvalid(tuple))
return false;
/* Used by pre-9.0 binary upgrades */
@@ -958,7 +958,8 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
* By here, the inserting transaction has committed - have to check
* when...
*/
- if (XidInMVCCSnapshot(HeapTupleHeaderGetXmin(tuple), snapshot))
+ if (!HeapTupleHeaderXminFrozen(tuple)
+ && XidInMVCCSnapshot(HeapTupleHeaderGetXmin(tuple), snapshot))
return false; /* treat as still in progress */
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid or aborted */
@@ -1058,9 +1059,9 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
* If the inserting transaction aborted, then the tuple was never visible
* to any other transaction, so we can delete it immediately.
*/
- if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
+ if (!HeapTupleHeaderXminCommitted(tuple))
{
- if (tuple->t_infomask & HEAP_XMIN_INVALID)
+ if (HeapTupleHeaderXminInvalid(tuple))
return HEAPTUPLE_DEAD;
/* Used by pre-9.0 binary upgrades */
else if (tuple->t_infomask & HEAP_MOVED_OFF)
@@ -1294,8 +1295,8 @@ HeapTupleIsSurelyDead(HeapTuple htup, TransactionId OldestXmin)
* invalid, then we assume it's still alive (since the presumption is that
* all relevant hint bits were just set moments ago).
*/
- if (!(tuple->t_infomask & HEAP_XMIN_COMMITTED))
- return (tuple->t_infomask & HEAP_XMIN_INVALID) != 0 ? true : false;
+ if (!HeapTupleHeaderXminCommitted(tuple))
+ return HeapTupleHeaderXminInvalid(tuple) ? true : false;
/*
* If the inserting transaction committed, but any deleting transaction
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index 0a832e9..f91dbc1 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -175,6 +175,7 @@ struct HeapTupleHeaderData
HEAP_XMAX_KEYSHR_LOCK)
#define HEAP_XMIN_COMMITTED 0x0100 /* t_xmin committed */
#define HEAP_XMIN_INVALID 0x0200 /* t_xmin invalid/aborted */
+#define HEAP_XMIN_FROZEN (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)
#define HEAP_XMAX_COMMITTED 0x0400 /* t_xmax committed */
#define HEAP_XMAX_INVALID 0x0800 /* t_xmax invalid/aborted */
#define HEAP_XMAX_IS_MULTI 0x1000 /* t_xmax is a MultiXactId */
@@ -254,6 +255,37 @@ struct HeapTupleHeaderData
(tup)->t_choice.t_heap.t_xmin = (xid) \
)
+#define HeapTupleHeaderXminCommitted(tup) \
+( \
+ ((tup)->t_infomask & HEAP_XMIN_COMMITTED) == HEAP_XMIN_COMMITTED \
+)
+#define HeapTupleHeaderXminInvalid(tup) \
+( \
+ ((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)) == \
+ HEAP_XMIN_INVALID \
+)
+#define HeapTupleHeaderXminFrozen(tup) \
+( \
+ ((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)) == \
+ HEAP_XMIN_FROZEN \
+)
+
+#define HeapTupleHeaderSetXminCommitted(tup) \
+( \
+ AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
+ ((tup)->t_infomask |= HEAP_XMIN_COMMITTED) \
+)
+#define HeapTupleHeaderSetXminInvalid(tup) \
+( \
+ AssertMacro(!HeapTupleHeaderXminCommitted(tup)), \
+ ((tup)->t_infomask |= HEAP_XMIN_INVALID) \
+)
+#define HeapTupleHeaderSetXminFrozen(tup) \
+( \
+ AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
+ ((tup)->t_infomask |= HEAP_XMIN_FROZEN) \
+)
+
/*
* HeapTupleHeaderGetRawXmax gets you the raw Xmax field. To find out the Xid
* that updated a tuple, you might need to resolve the MultiXactId if certain
@@ -374,7 +406,8 @@ do { \
#define HeapTupleHeaderIsHotUpdated(tup) \
( \
((tup)->t_infomask2 & HEAP_HOT_UPDATED) != 0 && \
- ((tup)->t_infomask & (HEAP_XMIN_INVALID | HEAP_XMAX_INVALID)) == 0 \
+ ((tup)->t_infomask & HEAP_XMAX_INVALID) == 0 && \
+ !HeapTupleHeaderXminInvalid(tup) \
)
#define HeapTupleHeaderSetHotUpdated(tup) \
--
1.8.5.rc2.dirty
>From 4ba3789b3f37bfe9676ec8af0aa5c3daa2489ca6 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 21 Nov 2013 14:21:39 +0100
Subject: [PATCH 2/2] Updates to the patch from Andres
---
contrib/pageinspect/heapfuncs.c | 2 +-
src/backend/access/common/heaptuple.c | 2 +-
src/backend/access/heap/heapam.c | 15 +++++++------
src/backend/access/heap/pruneheap.c | 4 ++--
src/backend/access/heap/rewriteheap.c | 1 -
src/backend/commands/typecmds.c | 3 +--
src/backend/commands/vacuumlazy.c | 5 -----
src/backend/optimizer/util/plancat.c | 1 -
src/backend/storage/lmgr/predicate.c | 5 +----
src/backend/utils/adt/ri_triggers.c | 6 ++---
src/backend/utils/cache/relcache.c | 8 ++-----
src/backend/utils/fmgr/fmgr.c | 4 ++--
src/backend/utils/time/combocid.c | 6 ++---
src/backend/utils/time/tqual.c | 42 +++++++++++++++++------------------
src/include/access/htup_details.h | 25 +++++++++++++++++----
src/pl/plperl/plperl.c | 4 ++--
src/pl/plpgsql/src/pl_comp.c | 4 ++--
src/pl/plpython/plpy_procedure.c | 6 ++---
src/pl/plpython/plpy_typeio.c | 4 ++--
src/pl/tcl/pltcl.c | 4 ++--
20 files changed, 75 insertions(+), 76 deletions(-)
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 6d8f6f1..a78cff3 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -162,7 +162,7 @@ heap_page_items(PG_FUNCTION_ARGS)
tuphdr = (HeapTupleHeader) PageGetItem(page, id);
- values[4] = UInt32GetDatum(HeapTupleHeaderGetXmin(tuphdr));
+ values[4] = UInt32GetDatum(HeapTupleHeaderGetRawXmin(tuphdr));
values[5] = UInt32GetDatum(HeapTupleHeaderGetRawXmax(tuphdr));
values[6] = UInt32GetDatum(HeapTupleHeaderGetRawCommandId(tuphdr)); /* shared with xvac */
values[7] = PointerGetDatum(&tuphdr->t_ctid);
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index e39b977..347d616 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -539,7 +539,7 @@ heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, bool *isnull)
result = ObjectIdGetDatum(HeapTupleGetOid(tup));
break;
case MinTransactionIdAttributeNumber:
- result = TransactionIdGetDatum(HeapTupleHeaderGetXmin(tup->t_data));
+ result = TransactionIdGetDatum(HeapTupleHeaderGetRawXmin(tup->t_data));
break;
case MaxTransactionIdAttributeNumber:
result = TransactionIdGetDatum(HeapTupleHeaderGetRawXmax(tup->t_data));
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index eb56230..fbc3043 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1733,7 +1733,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
*/
if (TransactionIdIsValid(prev_xmax) &&
!TransactionIdEquals(prev_xmax,
- HeapTupleHeaderGetXmin(heapTuple->t_data)))
+ HeapTupleHeaderGetRawXmin(heapTuple->t_data)))
break;
/*
@@ -2221,9 +2221,11 @@ heap_prepare_insert(Relation relation, HeapTuple tup, TransactionId xid,
tup->t_data->t_infomask &= ~(HEAP_XACT_MASK);
tup->t_data->t_infomask2 &= ~(HEAP2_XACT_MASK);
tup->t_data->t_infomask |= HEAP_XMAX_INVALID;
- HeapTupleHeaderSetXmin(tup->t_data, xid);
if (options & HEAP_INSERT_FROZEN)
HeapTupleHeaderSetXminFrozen(tup->t_data);
+ else
+ HeapTupleHeaderSetXmin(tup->t_data, xid);
+
HeapTupleHeaderSetCmin(tup->t_data, cid);
HeapTupleHeaderSetXmax(tup->t_data, 0); /* for cleanliness */
tup->t_tableOid = RelationGetRelid(relation);
@@ -5118,7 +5120,7 @@ heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
if (!HeapTupleHeaderXminFrozen(tuple))
{
- xid = HeapTupleHeaderGetXmin(tuple);
+ xid = HeapTupleHeaderGetRawXmin(tuple);
if (TransactionIdIsNormal(xid) &&
TransactionIdPrecedes(xid, cutoff_xid))
{
@@ -5470,7 +5472,7 @@ heap_tuple_needs_freeze(HeapTupleHeader tuple, TransactionId cutoff_xid,
if (!HeapTupleHeaderXminFrozen(tuple))
{
- xid = HeapTupleHeaderGetXmin(tuple);
+ xid = HeapTupleHeaderGetRawXmin(tuple);
if (TransactionIdIsNormal(xid) &&
TransactionIdPrecedes(xid, cutoff_xid))
return true;
@@ -5597,14 +5599,13 @@ HeapTupleHeaderAdvanceLatestRemovedXid(HeapTupleHeader tuple,
* updated/deleted by the inserting transaction.
*
* Look for a committed hint bit, or if no xmin bit is set, check clog.
- * This needs to work on both master and standby, where it is used to
+ * This needs to work on both master, and standby where it is used to
* assess btree delete records.
*/
if (HeapTupleHeaderXminCommitted(tuple) ||
(!HeapTupleHeaderXminInvalid(tuple) && TransactionIdDidCommit(xmin)))
{
- if ((HeapTupleHeaderXminFrozen(tuple) || xmax != xmin) &&
- TransactionIdFollows(xmax, *latestRemovedXid))
+ if (xmax != xmin && TransactionIdFollows(xmax, *latestRemovedXid))
*latestRemovedXid = xmax;
}
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 3ec10a0..d731e43 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -448,7 +448,7 @@ heap_prune_chain(Relation relation, Buffer buffer, OffsetNumber rootoffnum,
* Check the tuple XMIN against prior XMAX, if any
*/
if (TransactionIdIsValid(priorXmax) &&
- !TransactionIdEquals(HeapTupleHeaderGetXmin(htup), priorXmax))
+ !TransactionIdEquals(HeapTupleHeaderGetRawXmin(htup), priorXmax))
break;
/*
@@ -788,7 +788,7 @@ heap_get_root_tuples(Page page, OffsetNumber *root_offsets)
htup = (HeapTupleHeader) PageGetItem(page, lp);
if (TransactionIdIsValid(priorXmax) &&
- !TransactionIdEquals(priorXmax, HeapTupleHeaderGetXmin(htup)))
+ !TransactionIdEquals(priorXmax, HeapTupleHeaderGetRawXmin(htup)))
break;
/* Remember the root line pointer for this item */
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index f236a46..951894c 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -437,7 +437,6 @@ rewrite_heap_tuple(RewriteState state,
* RECENTLY_DEAD if and only if the xmin is not before OldestXmin.
*/
if ((new_tuple->t_data->t_infomask & HEAP_UPDATED) &&
- !HeapTupleHeaderXminFrozen(new_tuple->t_data) &&
!TransactionIdPrecedes(HeapTupleHeaderGetXmin(new_tuple->t_data),
state->rs_oldest_xmin))
{
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 4ad8386..d4a14ca 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1202,8 +1202,7 @@ AlterEnum(AlterEnumStmt *stmt, bool isTopLevel)
* cases that could theoretically be safe; but fortunately pg_dump only
* needs the simplest case.
*/
- if (!HeapTupleHeaderXminFrozen(tup->t_data) &&
- HeapTupleHeaderGetXmin(tup->t_data) == GetCurrentTransactionId() &&
+ if (HeapTupleHeaderGetXmin(tup->t_data) == GetCurrentTransactionId() &&
!(tup->t_data->t_infomask & HEAP_UPDATED))
/* safe to do inside transaction block */ ;
else
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 7ac1352..cc8aaf7 100644
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -818,13 +818,10 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
break;
}
-
/*
* The inserter definitely committed. But is it old
* enough that everyone sees it as committed?
*/
- if (HeapTupleHeaderXminFrozen(tuple.t_data))
- break;
xmin = HeapTupleHeaderGetXmin(tuple.t_data);
if (!TransactionIdPrecedes(xmin, OldestXmin))
{
@@ -1749,8 +1746,6 @@ heap_page_is_all_visible(Relation rel, Buffer buf, TransactionId *visibility_cut
* The inserter definitely committed. But is it old enough
* that everyone sees it as committed?
*/
- if (HeapTupleHeaderXminFrozen(tuple.t_data))
- break;
xmin = HeapTupleHeaderGetXmin(tuple.t_data);
if (!TransactionIdPrecedes(xmin, OldestXmin))
{
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index e22be0e..954666c 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -188,7 +188,6 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
* src/backend/access/heap/README.HOT for discussion.
*/
if (index->indcheckxmin &&
- !HeapTupleHeaderXminFrozen(indexRelation->rd_indextuple->t_data) &&
!TransactionIdPrecedes(HeapTupleHeaderGetXmin(indexRelation->rd_indextuple->t_data),
TransactionXmin))
{
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 77d87c7..8143ed0 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -2475,10 +2475,7 @@ PredicateLockTuple(Relation relation, HeapTuple tuple, Snapshot snapshot)
{
TransactionId myxid;
- if (HeapTupleHeaderXminFrozen(tuple->t_data))
- targetxmin = FrozenTransactionId;
- else
- targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
+ targetxmin = HeapTupleHeaderGetXmin(tuple->t_data);
myxid = GetTopTransactionIdIfAny();
if (TransactionIdIsValid(myxid))
diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c
index ed906ee..917130f 100644
--- a/src/backend/utils/adt/ri_triggers.c
+++ b/src/backend/utils/adt/ri_triggers.c
@@ -2166,8 +2166,7 @@ RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel,
* UPDATE check. (We could skip this if we knew the INSERT
* trigger already fired, but there is no easy way to know that.)
*/
- if (!HeapTupleHeaderXminFrozen(old_row->t_data) &&
- TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
+ if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
return true;
/* If all old and new key values are equal, no check is needed */
@@ -2205,8 +2204,7 @@ RI_FKey_fk_upd_check_required(Trigger *trigger, Relation fk_rel,
* UPDATE check. (We could skip this if we knew the INSERT
* trigger already fired, but there is no easy way to know that.)
*/
- if (!HeapTupleHeaderXminFrozen(old_row->t_data) &&
- TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
+ if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(old_row->t_data)))
return true;
/* If all old and new key values are equal, no check is needed */
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 4b92fbf..d0acca8 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -1764,7 +1764,6 @@ RelationReloadIndexInfo(Relation relation)
{
HeapTuple tuple;
Form_pg_index index;
- TransactionId xmin;
tuple = SearchSysCache1(INDEXRELID,
ObjectIdGetDatum(RelationGetRelid(relation)));
@@ -1790,11 +1789,8 @@ RelationReloadIndexInfo(Relation relation)
relation->rd_index->indislive = index->indislive;
/* Copy xmin too, as that is needed to make sense of indcheckxmin */
- if (HeapTupleHeaderXminFrozen(relation->rd_indextuple->t_data))
- xmin = FrozenTransactionId;
- else
- xmin = HeapTupleHeaderGetXmin(relation->rd_indextuple->t_data);
- HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data, xmin);
+ HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data,
+ HeapTupleHeaderGetXmin(tuple->t_data));
ReleaseSysCache(tuple);
}
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9246a00..cae1116 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -514,7 +514,7 @@ lookup_C_func(HeapTuple procedureTuple)
NULL);
if (entry == NULL)
return NULL; /* no such entry */
- if (entry->fn_xmin == HeapTupleHeaderGetXmin(procedureTuple->t_data) &&
+ if (entry->fn_xmin == HeapTupleHeaderGetRawXmin(procedureTuple->t_data) &&
ItemPointerEquals(&entry->fn_tid, &procedureTuple->t_self))
return entry; /* OK */
return NULL; /* entry is out of date */
@@ -552,7 +552,7 @@ record_C_func(HeapTuple procedureTuple,
HASH_ENTER,
&found);
/* OID is already filled in */
- entry->fn_xmin = HeapTupleHeaderGetXmin(procedureTuple->t_data);
+ entry->fn_xmin = HeapTupleHeaderGetRawXmin(procedureTuple->t_data);
entry->fn_tid = procedureTuple->t_self;
entry->user_fn = user_fn;
entry->inforec = inforec;
diff --git a/src/backend/utils/time/combocid.c b/src/backend/utils/time/combocid.c
index baacd4a..08955f3 100644
--- a/src/backend/utils/time/combocid.c
+++ b/src/backend/utils/time/combocid.c
@@ -148,10 +148,8 @@ HeapTupleHeaderAdjustCmax(HeapTupleHeader tup,
/*
* If we're marking a tuple deleted that was inserted by (any
* subtransaction of) our transaction, we need to use a combo command id.
- * It's cheaper to test HeapTupleHeaderXminCommitted before checking
- * TransactionIdIsCurrentTransactionId, but it's also necessary for
- * correctness: if HeapTupleHeaderXminCommitted returns true, the tuple
- * might even be frozen.
+ * Test for HeapTupleHeaderXminCommitted() first, because it's cheaper than a
+ * TransactionIdIsCurrentTransactionId call.
*/
if (!HeapTupleHeaderXminCommitted(tup) &&
TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tup)))
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index bf8c9919..341cc4c 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -210,7 +210,7 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer)
}
}
}
- else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmin(tuple)))
{
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */
return true;
@@ -243,11 +243,11 @@ HeapTupleSatisfiesSelf(HeapTuple htup, Snapshot snapshot, Buffer buffer)
return false;
}
- else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
return false;
- else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdDidCommit(HeapTupleHeaderGetRawXmin(tuple)))
SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- HeapTupleHeaderGetXmin(tuple));
+ HeapTupleHeaderGetRawXmin(tuple));
else
{
/* it must have aborted or crashed */
@@ -481,7 +481,7 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
}
}
}
- else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmin(tuple)))
{
if (HeapTupleHeaderGetCmin(tuple) >= curcid)
return HeapTupleInvisible; /* inserted after scan started */
@@ -527,11 +527,11 @@ HeapTupleSatisfiesUpdate(HeapTuple htup, CommandId curcid,
else
return HeapTupleInvisible; /* updated before scan started */
}
- else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
return HeapTupleInvisible;
- else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdDidCommit(HeapTupleHeaderGetRawXmin(tuple)))
SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- HeapTupleHeaderGetXmin(tuple));
+ HeapTupleHeaderGetRawXmin(tuple));
else
{
/* it must have aborted or crashed */
@@ -709,7 +709,7 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
}
}
}
- else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmin(tuple)))
{
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */
return true;
@@ -742,15 +742,15 @@ HeapTupleSatisfiesDirty(HeapTuple htup, Snapshot snapshot,
return false;
}
- else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
{
- snapshot->xmin = HeapTupleHeaderGetXmin(tuple);
+ snapshot->xmin = HeapTupleHeaderGetRawXmin(tuple);
/* XXX shouldn't we fall through to look at xmax? */
return true; /* in insertion by other */
}
- else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdDidCommit(HeapTupleHeaderGetRawXmin(tuple)))
SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- HeapTupleHeaderGetXmin(tuple));
+ HeapTupleHeaderGetRawXmin(tuple));
else
{
/* it must have aborted or crashed */
@@ -899,7 +899,7 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
}
}
}
- else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsCurrentTransactionId(HeapTupleHeaderGetRawXmin(tuple)))
{
if (HeapTupleHeaderGetCmin(tuple) >= snapshot->curcid)
return false; /* inserted after scan started */
@@ -940,11 +940,11 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
else
return false; /* deleted before scan started */
}
- else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
return false;
- else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdDidCommit(HeapTupleHeaderGetRawXmin(tuple)))
SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- HeapTupleHeaderGetXmin(tuple));
+ HeapTupleHeaderGetRawXmin(tuple));
else
{
/* it must have aborted or crashed */
@@ -959,7 +959,7 @@ HeapTupleSatisfiesMVCC(HeapTuple htup, Snapshot snapshot,
* when...
*/
if (!HeapTupleHeaderXminFrozen(tuple)
- && XidInMVCCSnapshot(HeapTupleHeaderGetXmin(tuple), snapshot))
+ && XidInMVCCSnapshot(HeapTupleHeaderGetRawXmin(tuple), snapshot))
return false; /* treat as still in progress */
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid or aborted */
@@ -1100,7 +1100,7 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
return HEAPTUPLE_DEAD;
}
}
- else if (TransactionIdIsInProgress(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdIsInProgress(HeapTupleHeaderGetRawXmin(tuple)))
{
if (tuple->t_infomask & HEAP_XMAX_INVALID) /* xid invalid */
return HEAPTUPLE_INSERT_IN_PROGRESS;
@@ -1111,9 +1111,9 @@ HeapTupleSatisfiesVacuum(HeapTuple htup, TransactionId OldestXmin,
/* inserted and then deleted by same xact */
return HEAPTUPLE_DELETE_IN_PROGRESS;
}
- else if (TransactionIdDidCommit(HeapTupleHeaderGetXmin(tuple)))
+ else if (TransactionIdDidCommit(HeapTupleHeaderGetRawXmin(tuple)))
SetHintBits(tuple, buffer, HEAP_XMIN_COMMITTED,
- HeapTupleHeaderGetXmin(tuple));
+ HeapTupleHeaderGetRawXmin(tuple));
else
{
/*
diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h
index f91dbc1..b3b8894 100644
--- a/src/include/access/htup_details.h
+++ b/src/include/access/htup_details.h
@@ -17,6 +17,7 @@
#include "access/htup.h"
#include "access/tupdesc.h"
#include "access/tupmacs.h"
+#include "access/transam.h"
#include "storage/bufpage.h"
/*
@@ -245,11 +246,24 @@ struct HeapTupleHeaderData
* macros evaluate their other argument only once.
*/
-#define HeapTupleHeaderGetXmin(tup) \
+/*
+ * HeapTupleHeaderGetRawXmin returns the "raw" xmin field - it might actually
+ * be frozen in which case it may never be used for anything but diagnostics
+ * purposes. Use HeapTupleHeaderGetXmin if not interested in such values or
+ * check for frozen xids with HeapTupleHeaderXminFrozen(). Not that even using
+ * *RawXmin might return FrozenTransactionId in pg_upgrade'd clusters.
+ */
+#define HeapTupleHeaderGetRawXmin(tup) \
( \
(tup)->t_choice.t_heap.t_xmin \
)
+#define HeapTupleHeaderGetXmin(tup) \
+( \
+ HeapTupleHeaderXminFrozen(tup) ? \
+ FrozenTransactionId : HeapTupleHeaderGetRawXmin(tup) \
+)
+
#define HeapTupleHeaderSetXmin(tup, xid) \
( \
(tup)->t_choice.t_heap.t_xmin = (xid) \
@@ -257,17 +271,18 @@ struct HeapTupleHeaderData
#define HeapTupleHeaderXminCommitted(tup) \
( \
- ((tup)->t_infomask & HEAP_XMIN_COMMITTED) == HEAP_XMIN_COMMITTED \
+ (tup)->t_infomask & HEAP_XMIN_COMMITTED \
)
+
#define HeapTupleHeaderXminInvalid(tup) \
( \
((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)) == \
HEAP_XMIN_INVALID \
)
+
#define HeapTupleHeaderXminFrozen(tup) \
( \
- ((tup)->t_infomask & (HEAP_XMIN_COMMITTED|HEAP_XMIN_INVALID)) == \
- HEAP_XMIN_FROZEN \
+ ((tup)->t_infomask & (HEAP_XMIN_FROZEN)) == HEAP_XMIN_FROZEN \
)
#define HeapTupleHeaderSetXminCommitted(tup) \
@@ -275,11 +290,13 @@ struct HeapTupleHeaderData
AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
((tup)->t_infomask |= HEAP_XMIN_COMMITTED) \
)
+
#define HeapTupleHeaderSetXminInvalid(tup) \
( \
AssertMacro(!HeapTupleHeaderXminCommitted(tup)), \
((tup)->t_infomask |= HEAP_XMIN_INVALID) \
)
+
#define HeapTupleHeaderSetXminFrozen(tup) \
( \
AssertMacro(!HeapTupleHeaderXminInvalid(tup)), \
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index de8cb0e..5d418c1 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -2399,7 +2399,7 @@ validate_plperl_function(plperl_proc_ptr *proc_ptr, HeapTuple procTup)
* This is needed because CREATE OR REPLACE FUNCTION can modify the
* function's pg_proc entry without changing its OID.
************************************************************/
- uptodate = (prodesc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
+ uptodate = (prodesc->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
ItemPointerEquals(&prodesc->fn_tid, &procTup->t_self));
if (uptodate)
@@ -2517,7 +2517,7 @@ compile_plperl_function(Oid fn_oid, bool is_trigger)
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
}
- prodesc->fn_xmin = HeapTupleHeaderGetXmin(procTup->t_data);
+ prodesc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
prodesc->fn_tid = procTup->t_self;
/* Remember if function is STABLE/IMMUTABLE */
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 426aeb5..9a16b0d 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -167,7 +167,7 @@ recheck:
if (function)
{
/* We have a compiled function, but is it still valid? */
- if (function->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
+ if (function->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
ItemPointerEquals(&function->fn_tid, &procTup->t_self))
function_valid = true;
else
@@ -345,7 +345,7 @@ do_compile(FunctionCallInfo fcinfo,
function->fn_signature = format_procedure(fcinfo->flinfo->fn_oid);
function->fn_oid = fcinfo->flinfo->fn_oid;
- function->fn_xmin = HeapTupleHeaderGetXmin(procTup->t_data);
+ function->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
function->fn_tid = procTup->t_self;
function->fn_input_collation = fcinfo->fncollation;
function->fn_cxt = func_cxt;
diff --git a/src/pl/plpython/plpy_procedure.c b/src/pl/plpython/plpy_procedure.c
index d278d6e..fad80b2 100644
--- a/src/pl/plpython/plpy_procedure.c
+++ b/src/pl/plpython/plpy_procedure.c
@@ -157,7 +157,7 @@ PLy_procedure_create(HeapTuple procTup, Oid fn_oid, bool is_trigger)
proc = PLy_malloc(sizeof(PLyProcedure));
proc->proname = PLy_strdup(NameStr(procStruct->proname));
proc->pyname = PLy_strdup(procName);
- proc->fn_xmin = HeapTupleHeaderGetXmin(procTup->t_data);
+ proc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
proc->fn_tid = procTup->t_self;
/* Remember if function is STABLE/IMMUTABLE */
proc->fn_readonly =
@@ -446,7 +446,7 @@ PLy_procedure_argument_valid(PLyTypeInfo *arg)
elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
/* If it has changed, the cached data is not valid */
- valid = (arg->typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
+ valid = (arg->typrel_xmin == HeapTupleHeaderGetRawXmin(relTup->t_data) &&
ItemPointerEquals(&arg->typrel_tid, &relTup->t_self));
ReleaseSysCache(relTup);
@@ -466,7 +466,7 @@ PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
Assert(proc != NULL);
/* If the pg_proc tuple has changed, it's not valid */
- if (!(proc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
+ if (!(proc->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
return false;
diff --git a/src/pl/plpython/plpy_typeio.c b/src/pl/plpython/plpy_typeio.c
index caccbf9..40c67ca 100644
--- a/src/pl/plpython/plpy_typeio.c
+++ b/src/pl/plpython/plpy_typeio.c
@@ -157,7 +157,7 @@ PLy_input_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc)
elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
/* Remember XMIN and TID for later validation if cache is still OK */
- arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
+ arg->typrel_xmin = HeapTupleHeaderGetRawXmin(relTup->t_data);
arg->typrel_tid = relTup->t_self;
ReleaseSysCache(relTup);
@@ -221,7 +221,7 @@ PLy_output_tuple_funcs(PLyTypeInfo *arg, TupleDesc desc)
elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
/* Remember XMIN and TID for later validation if cache is still OK */
- arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
+ arg->typrel_xmin = HeapTupleHeaderGetRawXmin(relTup->t_data);
arg->typrel_tid = relTup->t_self;
ReleaseSysCache(relTup);
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index c94d0d8..b703bda 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -1205,7 +1205,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, bool pltrusted)
{
bool uptodate;
- uptodate = (prodesc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
+ uptodate = (prodesc->fn_xmin == HeapTupleHeaderGetRawXmin(procTup->t_data) &&
ItemPointerEquals(&prodesc->fn_tid, &procTup->t_self));
if (!uptodate)
@@ -1267,7 +1267,7 @@ compile_pltcl_function(Oid fn_oid, Oid tgreloid, bool pltrusted)
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
- prodesc->fn_xmin = HeapTupleHeaderGetXmin(procTup->t_data);
+ prodesc->fn_xmin = HeapTupleHeaderGetRawXmin(procTup->t_data);
prodesc->fn_tid = procTup->t_self;
/* Remember if function is STABLE/IMMUTABLE */
--
1.8.5.rc2.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
