[prev in list] [next in list] [prev in thread] [next in thread] 

List:       postgresql-general
Subject:    [HACKERS] [PATCH] HOT on tables with oid indexes broken
From:       Andres Freund <andres () 2ndquadrant ! com>
Date:       2013-01-31 12:37:35
Message-ID: 20130131123735.GA13337 () awork2 ! anarazel ! de
[Download RAW message or body]

Hi,

The fklocks patch moved HeapSatisfiesHOTandKeyUpdate (or rather
HeapSatisfiesHOTUpdate back then) to be called way earlier in
heap_update as its needed to know which lock level is
required. Unfortunately the oid of the new tuple isn't yet setup at that
point.

Due to this everytime there's an index on an oid - like in most catalog
tables - no HOT updates will be performed as the old tuples oid will be
compared to the new ones which isn't setup yet (so either InvalidOid or
uninitialized).

There's also a related bug which seems to go further back but has far
fewer implications, namely that the tableOid of the old tuple isn't
necessarily setup before determining HOTability. Now that only matters
if there's an index on 'tableoid' which doesn't seem to be an all that
frequent thing to do.

Greetings,

Andres Freund

-- 
 Andres Freund	                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

["0001-Copy-a-tuple-s-oid-early-enough-in-heap_update-for-H.patch" (text/x-patch)]

From 295a5b40779c9f1e86dd80d532e78af2ea84b3ff Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Thu, 31 Jan 2013 13:15:17 +0100
Subject: [PATCH] Copy a tuple's oid early enough in heap_update for HOT to
 work on table with oid indexes

This was broken in 0ac5ad5134 which moved HeapSatisfiesHOTandKeyUpdate (called
HeapSatisfiesHOTUpdate back then) to be called earlier in heap_update but
didn't move enough of the tuple setup to be done early enough.
---
 src/backend/access/heap/heapam.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 57d47e8..5dd14a6 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2978,10 +2978,30 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	lp = PageGetItemId(page, ItemPointerGetOffsetNumber(otid));
 	Assert(ItemIdIsNormal(lp));
 
+	/* setup enough data for HeapSatisfiesHOTandKeyUpdate to work properly */
+	oldtup.t_tableOid = RelationGetRelid(relation);
 	oldtup.t_data = (HeapTupleHeader) PageGetItem(page, lp);
 	oldtup.t_len = ItemIdGetLength(lp);
 	oldtup.t_self = *otid;
 
+	newtup->t_tableOid = RelationGetRelid(relation);
+
+	/* Fill in OID for newtup */
+	if (relation->rd_rel->relhasoids)
+	{
+#ifdef NOT_USED
+		/* this is redundant with an Assert in HeapTupleSetOid */
+		Assert(newtup->t_data->t_infomask & HEAP_HASOID);
+#endif
+		HeapTupleSetOid(newtup, HeapTupleGetOid(&oldtup));
+	}
+	else
+	{
+		/* check there is not space for an OID */
+		Assert(!(newtup->t_data->t_infomask & HEAP_HASOID));
+	}
+
+
 	/*
 	 * If we're not updating any "key" column, we can grab a weaker lock type.
 	 * This allows for more concurrency when we are running simultaneously with
@@ -3243,20 +3263,7 @@ l2:
 	 */
 	CheckForSerializableConflictIn(relation, &oldtup, buffer);
 
-	/* Fill in OID and transaction status data for newtup */
-	if (relation->rd_rel->relhasoids)
-	{
-#ifdef NOT_USED
-		/* this is redundant with an Assert in HeapTupleSetOid */
-		Assert(newtup->t_data->t_infomask & HEAP_HASOID);
-#endif
-		HeapTupleSetOid(newtup, HeapTupleGetOid(&oldtup));
-	}
-	else
-	{
-		/* check there is not space for an OID */
-		Assert(!(newtup->t_data->t_infomask & HEAP_HASOID));
-	}
+	/* Fill in transaction status data */
 
 	/*
 	 * If the tuple we're updating is locked, we need to preserve the locking
@@ -3306,7 +3313,6 @@ l2:
 	newtup->t_data->t_infomask |= HEAP_UPDATED | infomask_new_tuple;
 	newtup->t_data->t_infomask2 |= infomask2_new_tuple;
 	HeapTupleHeaderSetXmax(newtup->t_data, xmax_new_tuple);
-	newtup->t_tableOid = RelationGetRelid(relation);
 
 	/*
 	 * Replace cid with a combo cid if necessary.  Note that we already put
-- 
1.7.12.289.g0ce9864.dirty



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


[prev in list] [next in list] [prev in thread] [next in thread]