From 93266bdc5abdb58669b87a0df7056bac4ddfc74e Mon Sep 17 00:00:00 2001 From: Antonin Houska Date: Tue, 16 Jun 2026 13:54:16 +0200 Subject: [PATCH 2/8] Move functions to repack.c. In the next patches, the functions will be called from other modules. Use a separate diff for the move so that the following diffs are a bit easier to read. --- src/backend/access/heap/heapam_handler.c | 97 +----------------------- src/backend/commands/repack.c | 91 ++++++++++++++++++++++ src/include/commands/repack.h | 7 ++ 3 files changed, 99 insertions(+), 96 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 127d4415084..2866a5e696e 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -34,6 +34,7 @@ #include "catalog/storage.h" #include "catalog/storage_xlog.h" #include "commands/progress.h" +#include "commands/repack.h" #include "executor/executor.h" #include "miscadmin.h" #include "pgstat.h" @@ -50,11 +51,6 @@ static void reform_and_rewrite_tuple(TupleTableSlot *src, TupleTableSlot *reform, RewriteState rwstate); -static void heap_insert_for_repack(Relation rel, TupleTableSlot *src, - TupleTableSlot *reform, - BulkInsertStateData *bistate); -static bool tuple_needs_reform(HeapTuple tuple, TupleDesc tupDesc); -static void clear_dropped_attributes(HeapTuple tuple, TupleTableSlot *reform); static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer, HeapTuple tuple, @@ -2382,97 +2378,6 @@ reform_and_rewrite_tuple(TupleTableSlot *src, TupleTableSlot *reform, heap_freetuple(newtuple); } -/* - * Insert tuple when processing REPACK CONCURRENTLY. - * - * rewriteheap.c is not used in the CONCURRENTLY case because it'd be - * difficult to do the same in the catch-up phase (as the logical - * decoding does not provide us with sufficient visibility - * information). Thus we must use heap_insert() both during the - * catch-up and here. - * - * 'reform' is a slot to use for tuple "reforming", typically to get set - * values of dropped columns to NULL. - * - * We pass the NO_LOGICAL flag to heap_insert() in order to skip logical - * decoding: as soon as REPACK CONCURRENTLY swaps the relation files, it drops - * this relation, so no logical replication subscription should need the data. - * - * BulkInsertState is used because many tuples are inserted in the typical - * case. - */ -static void -heap_insert_for_repack(Relation rel, TupleTableSlot *src, - TupleTableSlot *reform, BulkInsertStateData *bistate) -{ - HeapTuple tuple; - bool shouldFree; - TupleTableSlot *slot; - - tuple = ExecFetchSlotHeapTuple(src, false, &shouldFree); - if (tuple_needs_reform(tuple, src->tts_tupleDescriptor)) - { - clear_dropped_attributes(tuple, reform); - slot = reform; - } - else - slot = src; - - /* - * clear_dropped_attributes() should have deformed the tuple, so nothing - * should depend on it now. - */ - if (shouldFree) - heap_freetuple(tuple); - - table_tuple_insert(rel, slot, GetCurrentCommandId(true), - TABLE_INSERT_NO_LOGICAL, bistate); -} - -static bool -tuple_needs_reform(HeapTuple tuple, TupleDesc tupDesc) -{ - /* - * A short tuple might require values from attmissing val, so activate the - * coding unconditionally in that case. The value might legitimally be - * NULL otherwise, so this is slightly wasteful, but it probably beats - * having to test each attribute for presence of attmissingval each time. - */ - if (HeapTupleHeaderGetNatts(tuple->t_data) < tupDesc->natts) - return true; - - /* Does it have dropped attributes? */ - for (int i = 0; i < tupDesc->natts; i++) - { - if (TupleDescCompactAttr(tupDesc, i)->attisdropped && - !heap_attisnull(tuple, i + 1, tupDesc)) - return true; - } - - return false; -} - -/* - * Subroutine for reform_and_rewrite_tuple and heap_insert_for_repack. - * - * Set values of dropped columns to NULL, - */ -static void -clear_dropped_attributes(HeapTuple tuple, TupleTableSlot *reform) -{ - TupleDesc tupDesc = reform->tts_tupleDescriptor; - - /* Assuming 'reform' is virtual, this deforms the tuple. */ - Assert(TTS_IS_VIRTUAL(reform)); - ExecForceStoreHeapTuple(tuple, reform, false); - - for (int i = 0; i < tupDesc->natts; i++) - { - if (TupleDescCompactAttr(tupDesc, i)->attisdropped) - reform->tts_isnull[i] = true; - } -} - /* * Check visibility of the tuple. */ diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index ec100e3eef5..fa98b8b9247 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -1268,6 +1268,97 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod, return OIDNewHeap; } +/* + * Insert tuple when processing REPACK CONCURRENTLY. + * + * rewriteheap.c is not used in the CONCURRENTLY case because it'd be + * difficult to do the same in the catch-up phase (as the logical + * decoding does not provide us with sufficient visibility + * information). Thus we must use heap_insert() both during the + * catch-up and here. + * + * 'reform' is a slot to use for tuple "reforming", typically to get set + * values of dropped columns to NULL. + * + * We pass the NO_LOGICAL flag to heap_insert() in order to skip logical + * decoding: as soon as REPACK CONCURRENTLY swaps the relation files, it drops + * this relation, so no logical replication subscription should need the data. + * + * BulkInsertState is used because many tuples are inserted in the typical + * case. + */ +void +heap_insert_for_repack(Relation rel, TupleTableSlot *src, + TupleTableSlot *reform, BulkInsertStateData *bistate) +{ + HeapTuple tuple; + bool shouldFree; + TupleTableSlot *slot; + + tuple = ExecFetchSlotHeapTuple(src, false, &shouldFree); + if (tuple_needs_reform(tuple, src->tts_tupleDescriptor)) + { + clear_dropped_attributes(tuple, reform); + slot = reform; + } + else + slot = src; + + /* + * clear_dropped_attributes() should have deformed the tuple, so nothing + * should depend on it now. + */ + if (shouldFree) + heap_freetuple(tuple); + + table_tuple_insert(rel, slot, GetCurrentCommandId(true), + TABLE_INSERT_NO_LOGICAL, bistate); +} + +bool +tuple_needs_reform(HeapTuple tuple, TupleDesc tupDesc) +{ + /* + * A short tuple might require values from attmissing val, so activate the + * coding unconditionally in that case. The value might legitimally be + * NULL otherwise, so this is slightly wasteful, but it probably beats + * having to test each attribute for presence of attmissingval each time. + */ + if (HeapTupleHeaderGetNatts(tuple->t_data) < tupDesc->natts) + return true; + + /* Does it have dropped attributes? */ + for (int i = 0; i < tupDesc->natts; i++) + { + if (TupleDescCompactAttr(tupDesc, i)->attisdropped && + !heap_attisnull(tuple, i + 1, tupDesc)) + return true; + } + + return false; +} + +/* + * Subroutine for reform_and_rewrite_tuple and heap_insert_for_repack. + * + * Set values of dropped columns to NULL, + */ +void +clear_dropped_attributes(HeapTuple tuple, TupleTableSlot *reform) +{ + TupleDesc tupDesc = reform->tts_tupleDescriptor; + + /* Assuming 'reform' is virtual, this deforms the tuple. */ + Assert(TTS_IS_VIRTUAL(reform)); + ExecForceStoreHeapTuple(tuple, reform, false); + + for (int i = 0; i < tupDesc->natts; i++) + { + if (TupleDescCompactAttr(tupDesc, i)->attisdropped) + reform->tts_isnull[i] = true; + } +} + /* * Do the physical copying of table data. * diff --git a/src/include/commands/repack.h b/src/include/commands/repack.h index 45e5440a311..27105c10591 100644 --- a/src/include/commands/repack.h +++ b/src/include/commands/repack.h @@ -15,6 +15,8 @@ #include +#include "access/hio.h" +#include "nodes/execnodes.h" #include "nodes/parsenodes.h" #include "parser/parse_node.h" #include "storage/lockdefs.h" @@ -48,6 +50,11 @@ extern void mark_index_clustered(Relation rel, Oid indexOid, bool is_internal); extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod, char relpersistence, LOCKMODE lockmode); +extern void heap_insert_for_repack(Relation rel, TupleTableSlot *src, + TupleTableSlot *reform, + BulkInsertStateData *bistate); +extern bool tuple_needs_reform(HeapTuple tuple, TupleDesc tupDesc); +extern void clear_dropped_attributes(HeapTuple tuple, TupleTableSlot *reform); extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap, bool is_system_catalog, bool swap_toast_by_content, -- 2.52.0