Remove unnecessary PageIndexTupleOverwrite calls that caused UB (#438)

These places called PageIndexTupleOverwrite(), with the new tuple
pointing directly to the original page. The PageIndexTupleOverwrite()
call is unnecessary in these cases, as we have already modified the
tuple on the page directly. Moreover, PageIndexTupleOverWrite() will
call memcpy with same src and dst arguments, which is undefined
behavior.

It's OK to modify the pages on disk directly, and we don't need
critical sections, because we either use the generic xlog functions
which create a temporary copy of the page, or we are building a new
index so if we crash the whole index is invisible and will be dropped
anyway.
This commit is contained in:
Heikki Linnakangas
2024-01-29 08:52:56 +02:00
committed by GitHub
parent 86b31fdf96
commit 2d092016fc
2 changed files with 3 additions and 32 deletions

View File

@@ -357,7 +357,6 @@ HnswUpdateNeighborsOnDisk(Relation index, FmgrInfo *procinfo, Oid collation, Hns
GenericXLogState *state;
ItemId itemid;
HnswNeighborTuple ntup;
Size ntupSize;
int idx = -1;
int startIdx;
HnswElement neighborElement = HnswPtrAccess(base, hc->element);
@@ -398,7 +397,6 @@ HnswUpdateNeighborsOnDisk(Relation index, FmgrInfo *procinfo, Oid collation, Hns
/* Get tuple */
itemid = PageGetItemId(page, offno);
ntup = (HnswNeighborTuple) PageGetItem(page, itemid);
ntupSize = ItemIdGetLength(itemid);
/* Calculate index for update */
startIdx = (neighborElement->level - lc) * m;
@@ -427,13 +425,9 @@ HnswUpdateNeighborsOnDisk(Relation index, FmgrInfo *procinfo, Oid collation, Hns
{
ItemPointer indextid = &ntup->indextids[idx];
/* Update neighbor */
/* Update neighbor on the buffer */
ItemPointerSet(indextid, e->blkno, e->offno);
/* Overwrite tuple */
if (!PageIndexTupleOverwrite(page, offno, (Item) ntup, ntupSize))
elog(ERROR, "failed to add index item to \"%s\"", RelationGetRelationName(index));
/* Commit */
if (building)
MarkBufferDirty(buf);
@@ -459,7 +453,6 @@ AddDuplicateOnDisk(Relation index, HnswElement element, HnswElement dup, bool bu
GenericXLogState *state;
ItemId itemid;
HnswElementTuple etup;
Size etupSize;
int i;
/* Read page */
@@ -479,7 +472,6 @@ AddDuplicateOnDisk(Relation index, HnswElement element, HnswElement dup, bool bu
/* Find space */
itemid = PageGetItemId(page, dup->offno);
etup = (HnswElementTuple) PageGetItem(page, itemid);
etupSize = ItemIdGetLength(itemid);
for (i = 0; i < HNSW_HEAPTIDS; i++)
{
if (!ItemPointerIsValid(&etup->heaptids[i]))
@@ -495,13 +487,9 @@ AddDuplicateOnDisk(Relation index, HnswElement element, HnswElement dup, bool bu
return false;
}
/* Add heap TID */
/* Add heap TID, modifying the tuple on the page directly */
etup->heaptids[i] = element->heaptids[0];
/* Overwrite tuple */
if (!PageIndexTupleOverwrite(page, dup->offno, (Item) etup, etupSize))
elog(ERROR, "failed to add index item to \"%s\"", RelationGetRelationName(index));
/* Commit */
if (building)
MarkBufferDirty(buf);

View File

@@ -92,15 +92,10 @@ RemoveHeapTids(HnswVacuumState * vacuumstate)
if (itemUpdated)
{
Size etupSize = ItemIdGetLength(itemid);
/* Mark rest as invalid */
for (int i = idx; i < HNSW_HEAPTIDS; i++)
ItemPointerSetInvalid(&etup->heaptids[i]);
if (!PageIndexTupleOverwrite(page, offno, (Item) etup, etupSize))
elog(ERROR, "failed to add index item to \"%s\"", RelationGetRelationName(index));
updated = true;
}
}
@@ -482,8 +477,6 @@ MarkDeleted(HnswVacuumState * vacuumstate)
ItemId itemid = PageGetItemId(page, offno);
HnswElementTuple etup = (HnswElementTuple) PageGetItem(page, itemid);
HnswNeighborTuple ntup;
Size etupSize;
Size ntupSize;
Buffer nbuf;
Page npage;
BlockNumber neighborPage;
@@ -507,10 +500,6 @@ MarkDeleted(HnswVacuumState * vacuumstate)
if (ItemPointerIsValid(&etup->heaptids[0]))
continue;
/* Calculate sizes */
etupSize = ItemIdGetLength(itemid);
ntupSize = HNSW_NEIGHBOR_TUPLE_SIZE(etup->level, vacuumstate->m);
/* Get neighbor page */
neighborPage = ItemPointerGetBlockNumber(&etup->neighbortid);
neighborOffno = ItemPointerGetOffsetNumber(&etup->neighbortid);
@@ -537,13 +526,7 @@ MarkDeleted(HnswVacuumState * vacuumstate)
for (int i = 0; i < ntup->count; i++)
ItemPointerSetInvalid(&ntup->indextids[i]);
/* Overwrite element tuple */
if (!PageIndexTupleOverwrite(page, offno, (Item) etup, etupSize))
elog(ERROR, "failed to add index item to \"%s\"", RelationGetRelationName(index));
/* Overwrite neighbor tuple */
if (!PageIndexTupleOverwrite(npage, neighborOffno, (Item) ntup, ntupSize))
elog(ERROR, "failed to add index item to \"%s\"", RelationGetRelationName(index));
/* We modified the tuples in place, no need to call PageIndexTupleOverwrite */
/* Commit */
GenericXLogFinish(state);