Revert "PG18 - Adapt columnar stripe metadata updates (#8030)"

This reverts commit 5d805eb10b.
heap_inplace_update was incorrectly replaced by
CatalogTupleUpdate in 5d805eb. In Citus, we assume a stripe
entry with some columns set to null means that a write
is in-progress, because otherwise we wouldn't see a such row.
But this breaks when we use CatalogTupleUpdate because it
inserts a new version of the row, which leaves the
in-progress version behind. Among other things, this was
causing various issues in PG18 - check-columnar test.
pull/8242/head^2
naisila 2025-10-14 12:45:17 +03:00 committed by Naisila Puka
parent aa0ac0af60
commit abd50a0bb8
1 changed files with 19 additions and 28 deletions

View File

@ -1405,17 +1405,7 @@ UpdateStripeMetadataRow(uint64 storageId, uint64 stripeId, uint64 fileOffset,
Oid columnarStripesOid = ColumnarStripeRelationId(); Oid columnarStripesOid = ColumnarStripeRelationId();
#if PG_VERSION_NUM >= 180000 Relation columnarStripes = table_open(columnarStripesOid, AccessShareLock);
/* CatalogTupleUpdate performs a normal heap UPDATE → RowExclusiveLock */
const LOCKMODE openLockMode = RowExclusiveLock;
#else
/* Inplace update never changed tuple length → AccessShareLock was enough */
const LOCKMODE openLockMode = AccessShareLock;
#endif
Relation columnarStripes = table_open(columnarStripesOid, openLockMode);
TupleDesc tupleDescriptor = RelationGetDescr(columnarStripes); TupleDesc tupleDescriptor = RelationGetDescr(columnarStripes);
Oid indexId = ColumnarStripePKeyIndexRelationId(); Oid indexId = ColumnarStripePKeyIndexRelationId();
@ -1439,6 +1429,17 @@ UpdateStripeMetadataRow(uint64 storageId, uint64 stripeId, uint64 fileOffset,
storageId, stripeId))); storageId, stripeId)));
} }
/*
* heap_modify_tuple + heap_inplace_update only exist on PG < 18;
* on PG18 the in-place helper was removed upstream, so we skip the whole block.
*/
#if PG_VERSION_NUM < PG_VERSION_18
/*
* heap_inplace_update already doesn't allow changing size of the original
* tuple, so we don't allow setting any Datum's to NULL values.
*/
Datum *newValues = (Datum *) palloc(tupleDescriptor->natts * sizeof(Datum)); Datum *newValues = (Datum *) palloc(tupleDescriptor->natts * sizeof(Datum));
bool *newNulls = (bool *) palloc0(tupleDescriptor->natts * sizeof(bool)); bool *newNulls = (bool *) palloc0(tupleDescriptor->natts * sizeof(bool));
bool *update = (bool *) palloc0(tupleDescriptor->natts * sizeof(bool)); bool *update = (bool *) palloc0(tupleDescriptor->natts * sizeof(bool));
@ -1459,37 +1460,27 @@ UpdateStripeMetadataRow(uint64 storageId, uint64 stripeId, uint64 fileOffset,
newNulls, newNulls,
update); update);
#if PG_VERSION_NUM < PG_VERSION_18
/*
* heap_inplace_update already doesn't allow changing size of the original
* tuple, so we don't allow setting any Datum's to NULL values.
*/
heap_inplace_update(columnarStripes, modifiedTuple); heap_inplace_update(columnarStripes, modifiedTuple);
#endif
/* /*
* Existing tuple now contains modifications, because we used * Existing tuple now contains modifications, because we used
* heap_inplace_update(). * heap_inplace_update().
*/ */
HeapTuple newTuple = oldTuple; HeapTuple newTuple = oldTuple;
#else
/* Regular catalog UPDATE keeps indexes in sync */
CatalogTupleUpdate(columnarStripes, &oldTuple->t_self, modifiedTuple);
HeapTuple newTuple = modifiedTuple;
#endif
CommandCounterIncrement();
/* /*
* Must not pass modifiedTuple, because BuildStripeMetadata expects a real * Must not pass modifiedTuple, because BuildStripeMetadata expects a real
* heap tuple with MVCC fields. * heap tuple with MVCC fields.
*/ */
StripeMetadata *modifiedStripeMetadata = StripeMetadata *modifiedStripeMetadata = BuildStripeMetadata(columnarStripes,
BuildStripeMetadata(columnarStripes, newTuple); newTuple);
CommandCounterIncrement();
systable_endscan(scanDescriptor); systable_endscan(scanDescriptor);
table_close(columnarStripes, openLockMode); table_close(columnarStripes, AccessShareLock);
pfree(newValues); pfree(newValues);
pfree(newNulls); pfree(newNulls);