Pursuant to my comments at
https://www.postgresql.org/message-id/20161223192245.hx4rbrxbrwtgwj6i@alvherre.pgsql
and because of a stupid bug I found in my indirect indexes patch, I
rewrote (read: removed) HeapSatisfiesHOTAndKey. The replacement
function HeapDetermineModifiedColumns returns a bitmapset with a bit set
for each modified column, for those columns that are listed as
"interesting" -- currently that set is the ID columns, the "key"
columns, and the indexed columns. The new code is much simpler, at the
expense of a few bytes of additional memory used during heap_update().
All tests pass.
Both WARM and indirect indexes should be able to use this infrastructure
in a way that better serves them than the current HeapSatisfiesHOTAndKey
(both patches modify that function in a rather ugly way). I intend to
get this pushed shortly unless objections are raised.
The new coding prevents stopping the check early as soon as the three
result booleans could be determined. However, both patches were adding
reasons to avoid stopping early anyway, so I don't think this is a
significant loss.
Pavan, please rebase your WARM patch on top of this and let me know how
you like it. I'll post a new version of indirect indexes later this
week.
Comments welcome.
--
Á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..1dbefda 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;
@@ -3504,6 +3500,10 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
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 +3524,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);
@@ -3551,6 +3551,13 @@ heap_update(Relation relation, ItemPointer otid,
HeapTuple newtup,
}
/*
+ * Determine which columns are being 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
* with foreign key checks.
@@ -3561,10 +3568,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 +3807,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 +4113,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 +4128,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,6 +4278,8 @@ l2:
bms_free(hot_attrs);
bms_free(key_attrs);
bms_free(id_attrs);
+ bms_free(modified_attrs);
+ bms_free(interesting_attrs);
return HeapTupleMayBeUpdated;
}
@@ -4369,100 +4379,24 @@ heap_tuple_attr_equals(TupleDesc tupdesc, int attrnum,
* 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.
*/
-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
