Hi, > Andres Freund <[email protected]> writes: > >> Both heap_expand_tuple() and minimal_expand_tuple() being unused, make > >> expand_tuple() unused in turn. > >> I think we probably ought to remove all three in 20? I didn't find > >> external > >> callers. It's nontrivial enough code that I don't think we should keep it > >> around just because somebody might be using it. > > ... BTW, upon looking at the code coverage report to confirm these > are unused, I noticed that the adjacent heap_copytuple_with_tuple is > also unreferenced.
Today I re-discovered this fact and recalled this discussion. Here is the patch in case it will speed-up things. -- Best regards, Aleksander Alekseev
From c282a224742a7851628cc48e7f38ccb57b8e1948 Mon Sep 17 00:00:00 2001 From: Aleksander Alekseev <[email protected]> Date: Wed, 9 Sep 2026 17:47:11 +0300 Subject: [PATCH v1] Remove unused tuple expansion and copying functions minimal_expand_tuple() has been unused since 4da597edf1b and heap_expand_tuple() since 20d3fe9009d, which in turn left the static expand_tuple() unreachable. heap_copytuple_with_tuple() has had no callers since 141fd1b66ce. Remove all four. Two regression test comments naming expand_tuple() are updated to refer to the code that handles missing attributes nowadays. Reported-by: Andres Freund <[email protected]> Reported-by: Tom Lane <[email protected]> Author: Aleksander Alekseev <[email protected]> Discussion: https://postgr.es/m/gf6hokynqjp6g7o4zh6yy7ekbdbig5cnnd6ilk6iw3aodilj43%405x7xrz52jhve --- src/backend/access/common/heaptuple.c | 280 --------------------- src/include/access/htup_details.h | 3 - src/test/regress/expected/fast_default.out | 4 +- src/test/regress/sql/fast_default.sql | 4 +- 4 files changed, 4 insertions(+), 287 deletions(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index f30346469ed..c6b868d622a 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -699,286 +699,6 @@ heap_copytuple(HeapTuple tuple) return newTuple; } -/* ---------------- - * heap_copytuple_with_tuple - * - * copy a tuple into a caller-supplied HeapTuple management struct - * - * Note that after calling this function, the "dest" HeapTuple will not be - * allocated as a single palloc() block (unlike with heap_copytuple()). - * ---------------- - */ -void -heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest) -{ - if (!HeapTupleIsValid(src) || src->t_data == NULL) - { - dest->t_data = NULL; - return; - } - - dest->t_len = src->t_len; - dest->t_self = src->t_self; - dest->t_tableOid = src->t_tableOid; - dest->t_data = (HeapTupleHeader) palloc(src->t_len); - memcpy(dest->t_data, src->t_data, src->t_len); -} - -/* - * Expand a tuple which has fewer attributes than required. For each attribute - * not present in the sourceTuple, if there is a missing value that will be - * used. Otherwise the attribute will be set to NULL. - * - * The source tuple must have fewer attributes than the required number. - * - * Only one of targetHeapTuple and targetMinimalTuple may be supplied. The - * other argument must be NULL. - */ -static void -expand_tuple(HeapTuple *targetHeapTuple, - MinimalTuple *targetMinimalTuple, - HeapTuple sourceTuple, - TupleDesc tupleDesc) -{ - AttrMissing *attrmiss = NULL; - int attnum; - int firstmissingnum; - bool hasNulls = HeapTupleHasNulls(sourceTuple); - HeapTupleHeader targetTHeader; - HeapTupleHeader sourceTHeader = sourceTuple->t_data; - int sourceNatts = HeapTupleHeaderGetNatts(sourceTHeader); - int natts = tupleDesc->natts; - int sourceNullLen; - int targetNullLen; - Size sourceDataLen = sourceTuple->t_len - sourceTHeader->t_hoff; - Size targetDataLen; - Size len; - int hoff; - uint8 *nullBits = NULL; - int bitMask = 0; - char *targetData; - uint16 *infoMask; - - Assert((targetHeapTuple && !targetMinimalTuple) - || (!targetHeapTuple && targetMinimalTuple)); - - Assert(sourceNatts < natts); - - sourceNullLen = (hasNulls ? BITMAPLEN(sourceNatts) : 0); - - targetDataLen = sourceDataLen; - - if (tupleDesc->constr && - tupleDesc->constr->missing) - { - /* - * If there are missing values we want to put them into the tuple. - * Before that we have to compute the extra length for the values - * array and the variable length data. - */ - attrmiss = tupleDesc->constr->missing; - - /* - * Find the first item in attrmiss for which we don't have a value in - * the source. We can ignore all the missing entries before that. - */ - for (firstmissingnum = sourceNatts; - firstmissingnum < natts; - firstmissingnum++) - { - if (attrmiss[firstmissingnum].am_present) - break; - else - hasNulls = true; - } - - /* - * Now walk the missing attributes. If there is a missing value make - * space for it. Otherwise, it's going to be NULL. - */ - for (attnum = firstmissingnum; - attnum < natts; - attnum++) - { - if (attrmiss[attnum].am_present) - { - CompactAttribute *att = TupleDescCompactAttr(tupleDesc, attnum); - - targetDataLen = att_datum_alignby(targetDataLen, - att->attalignby, - att->attlen, - attrmiss[attnum].am_value); - - targetDataLen = att_addlength_datum(targetDataLen, - att->attlen, - attrmiss[attnum].am_value); - } - else - { - /* no missing value, so it must be null */ - hasNulls = true; - } - } - } /* end if have missing values */ - else - { - /* - * If there are no missing values at all then NULLS must be allowed, - * since some of the attributes are known to be absent. - */ - hasNulls = true; - } - - len = 0; - - if (hasNulls) - { - targetNullLen = BITMAPLEN(natts); - len += targetNullLen; - } - else - targetNullLen = 0; - - /* - * Allocate and zero the space needed. Note that the tuple body and - * HeapTupleData management structure are allocated in one chunk. - */ - if (targetHeapTuple) - { - len += offsetof(HeapTupleHeaderData, t_bits); - hoff = len = MAXALIGN(len); /* align user data safely */ - len += targetDataLen; - - *targetHeapTuple = (HeapTuple) palloc0(HEAPTUPLESIZE + len); - (*targetHeapTuple)->t_data - = targetTHeader - = (HeapTupleHeader) ((char *) *targetHeapTuple + HEAPTUPLESIZE); - (*targetHeapTuple)->t_len = len; - (*targetHeapTuple)->t_tableOid = sourceTuple->t_tableOid; - (*targetHeapTuple)->t_self = sourceTuple->t_self; - - targetTHeader->t_infomask = sourceTHeader->t_infomask; - targetTHeader->t_hoff = hoff; - HeapTupleHeaderSetNatts(targetTHeader, natts); - HeapTupleHeaderSetDatumLength(targetTHeader, len); - HeapTupleHeaderSetTypeId(targetTHeader, tupleDesc->tdtypeid); - HeapTupleHeaderSetTypMod(targetTHeader, tupleDesc->tdtypmod); - /* We also make sure that t_ctid is invalid unless explicitly set */ - ItemPointerSetInvalid(&(targetTHeader->t_ctid)); - if (targetNullLen > 0) - nullBits = (uint8 *) ((char *) (*targetHeapTuple)->t_data - + offsetof(HeapTupleHeaderData, t_bits)); - targetData = (char *) (*targetHeapTuple)->t_data + hoff; - infoMask = &(targetTHeader->t_infomask); - } - else - { - len += SizeofMinimalTupleHeader; - hoff = len = MAXALIGN(len); /* align user data safely */ - len += targetDataLen; - - *targetMinimalTuple = (MinimalTuple) palloc0(len); - (*targetMinimalTuple)->t_len = len; - (*targetMinimalTuple)->t_hoff = hoff + MINIMAL_TUPLE_OFFSET; - (*targetMinimalTuple)->t_infomask = sourceTHeader->t_infomask; - /* Same macro works for MinimalTuples */ - HeapTupleHeaderSetNatts(*targetMinimalTuple, natts); - if (targetNullLen > 0) - nullBits = (uint8 *) ((char *) *targetMinimalTuple - + offsetof(MinimalTupleData, t_bits)); - targetData = (char *) *targetMinimalTuple + hoff; - infoMask = &((*targetMinimalTuple)->t_infomask); - } - - if (targetNullLen > 0) - { - if (sourceNullLen > 0) - { - /* if bitmap pre-existed copy in - all is set */ - memcpy(nullBits, - ((char *) sourceTHeader) - + offsetof(HeapTupleHeaderData, t_bits), - sourceNullLen); - nullBits += sourceNullLen - 1; - } - else - { - sourceNullLen = BITMAPLEN(sourceNatts); - /* Set NOT NULL for all existing attributes */ - memset(nullBits, 0xff, sourceNullLen); - - nullBits += sourceNullLen - 1; - - if (sourceNatts & 0x07) - { - /* build the mask (inverted!) */ - bitMask = 0xff << (sourceNatts & 0x07); - /* Voila */ - *nullBits = ~bitMask; - } - } - - bitMask = (1 << ((sourceNatts - 1) & 0x07)); - } /* End if have null bitmap */ - - memcpy(targetData, - ((char *) sourceTuple->t_data) + sourceTHeader->t_hoff, - sourceDataLen); - - targetData += sourceDataLen; - - /* Now fill in the missing values */ - for (attnum = sourceNatts; attnum < natts; attnum++) - { - CompactAttribute *attr = TupleDescCompactAttr(tupleDesc, attnum); - - if (attrmiss && attrmiss[attnum].am_present) - { - fill_val(attr, - nullBits ? &nullBits : NULL, - &bitMask, - &targetData, - infoMask, - attrmiss[attnum].am_value, - false); - } - else - { - fill_val(attr, - &nullBits, - &bitMask, - &targetData, - infoMask, - (Datum) 0, - true); - } - } /* end loop over missing attributes */ -} - -/* - * Fill in the missing values for a minimal HeapTuple - */ -MinimalTuple -minimal_expand_tuple(HeapTuple sourceTuple, TupleDesc tupleDesc) -{ - MinimalTuple minimalTuple; - - expand_tuple(NULL, &minimalTuple, sourceTuple, tupleDesc); - return minimalTuple; -} - -/* - * Fill in the missing values for an ordinary HeapTuple - */ -HeapTuple -heap_expand_tuple(HeapTuple sourceTuple, TupleDesc tupleDesc) -{ - HeapTuple heapTuple; - - expand_tuple(&heapTuple, NULL, sourceTuple, tupleDesc); - return heapTuple; -} - /* ---------------- * heap_copy_tuple_as_datum * diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h index 77a6c48fd71..23f9546e5b9 100644 --- a/src/include/access/htup_details.h +++ b/src/include/access/htup_details.h @@ -806,7 +806,6 @@ extern Datum heap_getsysattr(HeapTuple tup, int attnum, TupleDesc tupleDesc, extern Datum getmissingattr(TupleDesc tupleDesc, int attnum, bool *isnull); extern HeapTuple heap_copytuple(HeapTuple tuple); -extern void heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest); extern Datum heap_copy_tuple_as_datum(HeapTuple tuple, TupleDesc tupleDesc); extern HeapTuple heap_form_tuple(TupleDesc tupleDescriptor, const Datum *values, const bool *isnull); @@ -832,8 +831,6 @@ extern MinimalTuple heap_copy_minimal_tuple(MinimalTuple mtup, Size extra); extern HeapTuple heap_tuple_from_minimal_tuple(MinimalTuple mtup); extern MinimalTuple minimal_tuple_from_heap_tuple(HeapTuple htup, Size extra); extern size_t varsize_any(void *p); -extern HeapTuple heap_expand_tuple(HeapTuple sourceTuple, TupleDesc tupleDesc); -extern MinimalTuple minimal_expand_tuple(HeapTuple sourceTuple, TupleDesc tupleDesc); #ifndef FRONTEND /* diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index 20356467655..e586abd0262 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -600,7 +600,7 @@ SELECT comp(); Unchanged (1 row) --- query to exercise expand_tuple function +-- query to exercise the missing-attribute expansion code CREATE TABLE t1 AS SELECT 1::int AS a , 2::int AS b FROM generate_series(1,20) q; @@ -636,7 +636,7 @@ FROM t1; DROP TABLE T; -- test that we account for missing columns without defaults correctly --- in expand_tuple, and that rows are correctly expanded for triggers +-- in slot_getmissingattrs(), and that rows are correctly expanded for triggers CREATE FUNCTION test_trigger() RETURNS trigger LANGUAGE plpgsql diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql index 36f81a184f0..fdeb7d147b4 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -413,7 +413,7 @@ SELECT c_text FROM T WHERE c_int = -1; SELECT comp(); --- query to exercise expand_tuple function +-- query to exercise the missing-attribute expansion code CREATE TABLE t1 AS SELECT 1::int AS a , 2::int AS b FROM generate_series(1,20) q; @@ -429,7 +429,7 @@ FROM t1; DROP TABLE T; -- test that we account for missing columns without defaults correctly --- in expand_tuple, and that rows are correctly expanded for triggers +-- in slot_getmissingattrs(), and that rows are correctly expanded for triggers CREATE FUNCTION test_trigger() RETURNS trigger -- 2.43.0
