Here's a version with fixed comments.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index ea579a0..19edbdf 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -95,11 +95,8 @@ static XLogRecPtr log_heap_update(Relation reln, Buffer
oldbuf,
Buffer newbuf, HeapTuple oldtup,
HeapTuple newtup, HeapTuple old_key_tup,
bool all_visible_cleared, bool
new_all_visible_cleared);
-static void HeapSatisfiesHOTandKeyUpdate(Relation relation,
- Bitmapset *hot_attrs,
- Bitmapset *key_attrs,
Bitmapset *id_attrs,
- bool *satisfies_hot,
bool *satisfies_key,
- bool *satisfies_id,
+static Bitmapset *HeapDetermineModifiedColumns(Relation relation,
+ Bitmapset
*interesting_cols,
HeapTuple oldtup,
HeapTuple newtup);
static bool heap_acquire_tuplock(Relation relation, ItemPointer tid,
LockTupleMode mode, LockWaitPolicy
wait_policy,
@@ -3443,6 +3440,8 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
Bitmapset *hot_attrs;
Bitmapset *key_attrs;
Bitmapset *id_attrs;
+ Bitmapset *interesting_attrs;
+ Bitmapset *modified_attrs;
ItemId lp;
HeapTupleData oldtup;
HeapTuple heaptup;
@@ -3460,9 +3459,6 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
pagefree;
bool have_tuple_lock = false;
bool iscombo;
- bool satisfies_hot;
- bool satisfies_key;
- bool satisfies_id;
bool use_hot_update = false;
bool key_intact;
bool all_visible_cleared = false;
@@ -3489,21 +3485,30 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
errmsg("cannot update tuples during a parallel
operation")));
/*
- * Fetch the list of attributes to be checked for HOT update. This is
- * wasted effort if we fail to update or have to put the new tuple on a
- * different page. But we must compute the list before obtaining buffer
- * lock --- in the worst case, if we are doing an update on one of the
- * relevant system catalogs, we could deadlock if we try to fetch the
list
- * later. In any case, the relcache caches the data so this is usually
- * pretty cheap.
+ * Fetch the list of attributes to be checked for various operations.
*
- * Note that we get a copy here, so we need not worry about relcache
flush
- * happening midway through.
+ * For HOT considerations, this is wasted effort if we fail to update or
+ * have to put the new tuple on a different page. But we must compute
the
+ * list before obtaining buffer lock --- in the worst case, if we are
doing
+ * an update on one of the relevant system catalogs, we could deadlock
if
+ * we try to fetch the list later. In any case, the relcache caches the
+ * data so this is usually pretty cheap.
+ *
+ * We also need columns used by the replica identity, the columns that
+ * are considered the "key" of rows in the table, and columns that are
+ * part of indirect indexes.
+ *
+ * Note that we get copies of each bitmap, so we need not worry about
+ * relcache flush happening midway through.
*/
hot_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_ALL);
key_attrs = RelationGetIndexAttrBitmap(relation, INDEX_ATTR_BITMAP_KEY);
id_attrs = RelationGetIndexAttrBitmap(relation,
INDEX_ATTR_BITMAP_IDENTITY_KEY);
+ interesting_attrs = bms_add_members(NULL, hot_attrs);
+ interesting_attrs = bms_add_members(interesting_attrs, key_attrs);
+ interesting_attrs = bms_add_members(interesting_attrs, id_attrs);
+
block = ItemPointerGetBlockNumber(otid);
buffer = ReadBuffer(relation, block);
@@ -3524,7 +3529,7 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
Assert(ItemIdIsNormal(lp));
/*
- * Fill in enough data in oldtup for HeapSatisfiesHOTandKeyUpdate to
work
+ * Fill in enough data in oldtup for HeapDetermineModifiedColumns to
work
* properly.
*/
oldtup.t_tableOid = RelationGetRelid(relation);
@@ -3550,6 +3555,10 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
Assert(!(newtup->t_data->t_infomask & HEAP_HASOID));
}
+ /* Determine columns modified by the update. */
+ modified_attrs = HeapDetermineModifiedColumns(relation,
interesting_attrs,
+
&oldtup, newtup);
+
/*
* 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
@@ -3561,10 +3570,7 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
* is updates that don't manipulate key columns, not those that
* serendipitiously arrive at the same key values.
*/
- HeapSatisfiesHOTandKeyUpdate(relation, hot_attrs, key_attrs, id_attrs,
-
&satisfies_hot, &satisfies_key,
- &satisfies_id,
&oldtup, newtup);
- if (satisfies_key)
+ if (!bms_overlap(modified_attrs, key_attrs))
{
*lockmode = LockTupleNoKeyExclusive;
mxact_status = MultiXactStatusNoKeyUpdate;
@@ -3803,6 +3809,8 @@ l2:
bms_free(hot_attrs);
bms_free(key_attrs);
bms_free(id_attrs);
+ bms_free(modified_attrs);
+ bms_free(interesting_attrs);
return result;
}
@@ -4107,7 +4115,7 @@ l2:
* to do a HOT update. Check if any of the index columns have
been
* changed. If not, then HOT update is possible.
*/
- if (satisfies_hot)
+ if (!bms_overlap(modified_attrs, hot_attrs))
use_hot_update = true;
}
else
@@ -4122,7 +4130,9 @@ l2:
* ExtractReplicaIdentity() will return NULL if nothing needs to be
* logged.
*/
- old_key_tuple = ExtractReplicaIdentity(relation, &oldtup,
!satisfies_id, &old_key_copied);
+ old_key_tuple = ExtractReplicaIdentity(relation, &oldtup,
+
bms_overlap(modified_attrs, id_attrs),
+
&old_key_copied);
/* NO EREPORT(ERROR) from here till changes are logged */
START_CRIT_SECTION();
@@ -4270,13 +4280,15 @@ l2:
bms_free(hot_attrs);
bms_free(key_attrs);
bms_free(id_attrs);
+ bms_free(modified_attrs);
+ bms_free(interesting_attrs);
return HeapTupleMayBeUpdated;
}
/*
* Check if the specified attribute's value is same in both given tuples.
- * Subroutine for HeapSatisfiesHOTandKeyUpdate.
+ * Subroutine for HeapDetermineModifiedColumns.
*/
static bool
heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
@@ -4310,7 +4322,7 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
/*
* Extract the corresponding values. XXX this is pretty inefficient if
- * there are many indexed columns. Should HeapSatisfiesHOTandKeyUpdate
do
+ * there are many indexed columns. Should HeapDetermineModifiedColumns
do
* a single heap_deform_tuple call on each tuple, instead? But that
* doesn't work for system columns ...
*/
@@ -4355,114 +4367,30 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
/*
* Check which columns are being updated.
*
- * This simultaneously checks conditions for HOT updates, for FOR KEY
- * SHARE updates, and REPLICA IDENTITY concerns. Since much of the time they
- * will be checking very similar sets of columns, and doing the same tests on
- * them, it makes sense to optimize and do them together.
+ * Given an updated tuple, determine (and return into the output bitmapset),
+ * from those listed as interesting, the set of columns that changed.
*
- * We receive three bitmapsets comprising the three sets of columns we're
- * interested in. Note these are destructively modified; that is OK since
- * this is invoked at most once in heap_update.
- *
- * hot_result is set to TRUE if it's okay to do a HOT update (i.e. it does not
- * modified indexed columns); key_result is set to TRUE if the update does not
- * modify columns used in the key; id_result is set to TRUE if the update does
- * not modify columns in any index marked as the REPLICA IDENTITY.
+ * The input bitmapset is destructively modified; that is OK since this is
+ * invoked at most once in heap_update.
*/
-static void
-HeapSatisfiesHOTandKeyUpdate(Relation relation, Bitmapset *hot_attrs,
- Bitmapset *key_attrs,
Bitmapset *id_attrs,
- bool *satisfies_hot,
bool *satisfies_key,
- bool *satisfies_id,
+static Bitmapset *
+HeapDetermineModifiedColumns(Relation relation, Bitmapset *interesting_cols,
HeapTuple oldtup,
HeapTuple newtup)
{
- int next_hot_attnum;
- int next_key_attnum;
- int next_id_attnum;
- bool hot_result = true;
- bool key_result = true;
- bool id_result = true;
+ int attnum;
+ Bitmapset *modified = NULL;
- /* If REPLICA IDENTITY is set to FULL, id_attrs will be empty. */
- Assert(bms_is_subset(id_attrs, key_attrs));
- Assert(bms_is_subset(key_attrs, hot_attrs));
-
- /*
- * If one of these sets contains no remaining bits, bms_first_member
will
- * return -1, and after adding FirstLowInvalidHeapAttributeNumber (which
- * is negative!) we'll get an attribute number that can't possibly be
- * real, and thus won't match any actual attribute number.
- */
- next_hot_attnum = bms_first_member(hot_attrs);
- next_hot_attnum += FirstLowInvalidHeapAttributeNumber;
- next_key_attnum = bms_first_member(key_attrs);
- next_key_attnum += FirstLowInvalidHeapAttributeNumber;
- next_id_attnum = bms_first_member(id_attrs);
- next_id_attnum += FirstLowInvalidHeapAttributeNumber;
-
- for (;;)
+ while ((attnum = bms_first_member(interesting_cols)) >= 0)
{
- bool changed;
- int check_now;
+ attnum += FirstLowInvalidHeapAttributeNumber;
- /*
- * Since the HOT attributes are a superset of the key
attributes and
- * the key attributes are a superset of the id attributes, this
logic
- * is guaranteed to identify the next column that needs to be
checked.
- */
- if (hot_result && next_hot_attnum >
FirstLowInvalidHeapAttributeNumber)
- check_now = next_hot_attnum;
- else if (key_result && next_key_attnum >
FirstLowInvalidHeapAttributeNumber)
- check_now = next_key_attnum;
- else if (id_result && next_id_attnum >
FirstLowInvalidHeapAttributeNumber)
- check_now = next_id_attnum;
- else
- break;
-
- /* See whether it changed. */
- changed = !heap_tuple_attr_equals(RelationGetDescr(relation),
-
check_now, oldtup, newtup);
- if (changed)
- {
- if (check_now == next_hot_attnum)
- hot_result = false;
- if (check_now == next_key_attnum)
- key_result = false;
- if (check_now == next_id_attnum)
- id_result = false;
-
- /* if all are false now, we can stop checking */
- if (!hot_result && !key_result && !id_result)
- break;
- }
-
- /*
- * Advance the next attribute numbers for the sets that contain
the
- * attribute we just checked. As we work our way through the
columns,
- * the next_attnum values will rise; but when each set becomes
empty,
- * bms_first_member() will return -1 and the attribute number
will end
- * up with a value less than FirstLowInvalidHeapAttributeNumber.
- */
- if (hot_result && check_now == next_hot_attnum)
- {
- next_hot_attnum = bms_first_member(hot_attrs);
- next_hot_attnum += FirstLowInvalidHeapAttributeNumber;
- }
- if (key_result && check_now == next_key_attnum)
- {
- next_key_attnum = bms_first_member(key_attrs);
- next_key_attnum += FirstLowInvalidHeapAttributeNumber;
- }
- if (id_result && check_now == next_id_attnum)
- {
- next_id_attnum = bms_first_member(id_attrs);
- next_id_attnum += FirstLowInvalidHeapAttributeNumber;
- }
+ if (!heap_tuple_attr_equals(RelationGetDescr(relation),
+ attnum,
oldtup, newtup))
+ modified = bms_add_member(modified,
+
attnum - FirstLowInvalidHeapAttributeNumber);
}
- *satisfies_hot = hot_result;
- *satisfies_key = key_result;
- *satisfies_id = id_result;
+ return modified;
}
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
