From b9d161d6fa348d3401d3d63631f4966675c03876 Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Sun, 9 Aug 2026 17:05:42 -0600 Subject: [PATCH] refactor: columnar_metadata gets its own header, the first of six (#496) columnar.h declares 137 functions. 84 of them have exactly one consumer outside their defining file -- a private arrangement between two files that the other twenty are forced to recompile for, and that anyone reading the header has to scan past to reach the 49 that are genuinely shared. This moves the largest single group: the 22 declarations defined in columnar_metadata.c, into src/columnar_metadata.h beside it. The shared vocabulary they take -- the Native*Metadata structs, the GUCs, the format constants -- stays in columnar.h, which the new header includes. The census in #496 was stale and understated the problem. Re-measured on 3e86a7e: when filed today columnar.h lines 957 1045 extern declarations 159 164 single-consumer 79 84 so the header gained interface faster than #527 removed it. Re-measuring also caught an error in my own tool. Its first version attributed PgColumnarBloomBuild and PgColumnarEncodeChunk to columnar_write_state.c, which does not define either: it took whichever file matched first in directory order. Fixed to require a definition at column 0, which is the style this tree uses, and the ranking changed -- columnar_metadata went from outside the top three to the largest module at 22, and columnar_write_state from 25 to 11. The wrong module would have been extracted first. Gate: builds on pg15a, pg16a, pg17a, pg18a and pg19a, make clean between each (#536), zero warnings and zero errors on all five. That is the gate that matters for a declaration-only change: nothing here alters a line of executable code, and a consumer left without the declaration it needs fails to compile rather than misbehaving. native_vecdecode 24/24 and harness_selftest 54/54 on pg18a besides. Five modules remain -- reader (12), write_state (11), storage (10), customscan (6), delete_vector (6). One PR each: 84 declarations in a single diff is not a reviewable change. --- src/columnar.h | 34 --------------- src/columnar_metadata.c | 1 + src/columnar_metadata.h | 80 ++++++++++++++++++++++++++++++++++++ src/columnar_projection.c | 1 + src/columnar_reader.c | 1 + src/columnar_tableam.c | 1 + src/columnar_vacuum.c | 1 + src/columnar_vector.c | 1 + src/columnar_visibilitymap.c | 1 + src/columnar_write_state.c | 1 + 10 files changed, 88 insertions(+), 34 deletions(-) create mode 100644 src/columnar_metadata.h diff --git a/src/columnar.h b/src/columnar.h index 38fdca90..45890315 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -228,7 +228,6 @@ typedef struct PgColumnarMetapage } PgColumnarMetapage; - /* ------------------------------------------------------------------------- * Native format catalog row shapes (re-origination line, PGCN v1). These map * to pgcolumnar.storage / row_group / column_chunk (native spec section 11). @@ -424,19 +423,9 @@ typedef struct PgColumnarRowRange /* row groups every one of whose rows is deleted as-of oldestXmin. Returns a * List of palloc'd uint64 group numbers. */ -extern void PgColumnarRetireGroup(uint64 storageId, uint64 groupNumber); extern int64 PgColumnarRetireFullyDeletedGroups(Relation rel); -extern void PgColumnarLockChunkGroup(uint64 storageId, uint64 groupNumber); -extern bool PgColumnarAllocateFreeSpace(uint64 storageId, uint64 dataLength, - TransactionId oldestXmin, uint64 *fileOffset); -extern bool PgColumnarTrailingFreeSpaceSafe(uint64 storageId, uint64 liveEnd, - TransactionId oldestXmin); -extern void PgColumnarDeleteFreeSpaceAtOrAbove(uint64 storageId, uint64 liveEnd); -extern void PgColumnarReconcileFreeList(Relation dataRel); /* all-visible chunk-group row ranges: stripe committed past the horizon and no * deletes (committed or in-progress). Returns a List of PgColumnarRowRange *. */ -extern List *PgColumnarComputeAllVisibleGroups(uint64 storageId, - TransactionId oldestXmin); /* physical reclaim: split freed ranges on allocate and coalesce on free (GUC) */ extern bool pgcolumnar_reclaim_coalesce; @@ -463,38 +452,21 @@ extern void PgColumnarCheckFreeSpaceNoOverlap(uint64 storageId); * metadata layer (pgcolumnar_metadata.c) * ------------------------------------------------------------------------- */ extern uint64 PgColumnarNextStorageId(void); -extern void PgColumnarInsertNativeStorageRow(const NativeStorageMetadata *s); /* projection: needed attnos (pull_varattnos form) -> the reader's 0-based set */ extern Bitmapset *PgColumnarProjectionFromAttnos(Bitmapset *needed, int natts, int *nProjected); -extern void PgColumnarSetSortedExtent(uint64 storageId, int64 firstGroup, - int64 lastGroup); extern void PgColumnarCheckNativeFormatVersion(uint64 storageId, const char *relName); -extern void PgColumnarInsertRowGroupRow(const NativeRowGroupMetadata *rg); -extern void PgColumnarInsertColumnChunkRow(const NativeColumnChunkMetadata *cc); -extern void PgColumnarInsertZoneMapRow(const NativeZoneMapMetadata *z); -extern void PgColumnarInsertBloomRow(const NativeBloomMetadata *b); extern List *PgColumnarReadRowGroupList(uint64 storageId, Snapshot snapshot); -extern List *PgColumnarReadColumnChunkList(uint64 storageId, uint64 groupNumber, - Snapshot snapshot); extern List *PgColumnarReadZoneMapList(uint64 storageId, uint64 groupNumber, Snapshot snapshot); -extern List *PgColumnarReadZoneMapVectors(uint64 storageId, uint64 groupNumber, - Snapshot snapshot); -extern NativeBloomMetadata *PgColumnarReadBloomForColumn(uint64 storageId, - uint64 groupNumber, - int columnIndex, - Snapshot snapshot); extern void PgColumnarDeleteMetadata(uint64 storageId); /* per-table options catalog (spec 7.4) */ extern bool PgColumnarReadOptions(Oid relid, PgColumnarOptions *opts); -extern void PgColumnarDeleteOptions(Oid relid); /* declared physical sort key (#288); List of pstrdup'd column names, NIL if * none is declared. Names (not attnums) so the value survives dump/restore. */ -extern List *PgColumnarReadSortBy(Oid relid); /* projection catalog (gap 26, format 2.2). List entries are PgColumnarProjection* * palloc'd in the current context, ordered by projection_id. */ @@ -502,13 +474,8 @@ extern List *PgColumnarListProjections(uint64 storageId); extern void PgColumnarInsertProjectionRow(const PgColumnarProjection *proj); /* The dumpable declaration behind a projection, keyed by regclass and stored as * column names so a dump and restore can carry it (#266). */ -extern void PgColumnarRecordProjectionDeclaration(Oid relid, const char *name, - ArrayType *columns, - ArrayType *sortKey); -extern void PgColumnarDeleteProjectionDeclaration(Oid relid, const char *name); /* Every declaration for a relation, for the drop hook: a dropped table must not * leave rows behind whose regclass no longer resolves (#304). */ -extern void PgColumnarDeleteProjectionDeclarationsForRel(Oid relid); extern void PgColumnarDeleteProjectionRow(uint64 storageId, int projectionId); /* whether a relation uses the columnar table access method */ @@ -529,7 +496,6 @@ extern Snapshot PgColumnarCatalogSnapshot(Snapshot base); /* delete_vector catalog access (spec 7.5) */ extern List *PgColumnarReadDeleteVectorList(uint64 storageId, uint64 stripeId, Snapshot snapshot); -extern bool PgColumnarStorageHasDeleteVector(uint64 storageId, Snapshot snapshot); extern void PgColumnarUpsertDeleteVector(uint64 storageId, DeleteVectorMetadata *rm); /* ------------------------------------------------------------------------- diff --git a/src/columnar_metadata.c b/src/columnar_metadata.c index e59930bf..69f2f2e5 100644 --- a/src/columnar_metadata.c +++ b/src/columnar_metadata.c @@ -11,6 +11,7 @@ */ #include "columnar.h" +#include "columnar_metadata.h" #include "fmgr.h" #include "columnar_compat.h" #include "access/genam.h" diff --git a/src/columnar_metadata.h b/src/columnar_metadata.h new file mode 100644 index 00000000..4ccfee2c --- /dev/null +++ b/src/columnar_metadata.h @@ -0,0 +1,80 @@ +/*------------------------------------------------------------------------- + * + * columnar_metadata.h + * The catalog side of pgColumnar: what columnar_metadata.c offers the + * modules that read and write our own catalog tables. + * + * Split out of columnar.h (#496). Every declaration here had exactly ONE + * consumer outside its defining file, so it was a private arrangement between + * two files that the other twenty were forced to recompile for, and that anyone + * reading columnar.h had to scan past to reach the interface that is genuinely + * shared. + * + * The shared vocabulary -- the Native*Metadata structs these signatures take, + * the GUCs, the format constants -- stays in columnar.h, which this includes. + * + * Written fresh for pgColumnar. + * + *------------------------------------------------------------------------- + */ +#ifndef PGCOLUMNAR_METADATA_H +#define PGCOLUMNAR_METADATA_H + +#include "columnar.h" + +extern void PgColumnarRetireGroup(uint64 storageId, uint64 groupNumber); + +extern void PgColumnarLockChunkGroup(uint64 storageId, uint64 groupNumber); + +extern bool PgColumnarAllocateFreeSpace(uint64 storageId, uint64 dataLength, + TransactionId oldestXmin, uint64 *fileOffset); + +extern bool PgColumnarTrailingFreeSpaceSafe(uint64 storageId, uint64 liveEnd, + TransactionId oldestXmin); + +extern void PgColumnarDeleteFreeSpaceAtOrAbove(uint64 storageId, uint64 liveEnd); + +extern void PgColumnarReconcileFreeList(Relation dataRel); + +extern List *PgColumnarComputeAllVisibleGroups(uint64 storageId, + TransactionId oldestXmin); + +extern void PgColumnarInsertNativeStorageRow(const NativeStorageMetadata *s); + +extern void PgColumnarSetSortedExtent(uint64 storageId, int64 firstGroup, + int64 lastGroup); + +extern void PgColumnarInsertRowGroupRow(const NativeRowGroupMetadata *rg); + +extern void PgColumnarInsertColumnChunkRow(const NativeColumnChunkMetadata *cc); + +extern void PgColumnarInsertZoneMapRow(const NativeZoneMapMetadata *z); + +extern void PgColumnarInsertBloomRow(const NativeBloomMetadata *b); + +extern List *PgColumnarReadColumnChunkList(uint64 storageId, uint64 groupNumber, + Snapshot snapshot); + +extern List *PgColumnarReadZoneMapVectors(uint64 storageId, uint64 groupNumber, + Snapshot snapshot); + +extern NativeBloomMetadata *PgColumnarReadBloomForColumn(uint64 storageId, + uint64 groupNumber, + int columnIndex, + Snapshot snapshot); + +extern void PgColumnarDeleteOptions(Oid relid); + +extern List *PgColumnarReadSortBy(Oid relid); + +extern void PgColumnarRecordProjectionDeclaration(Oid relid, const char *name, + ArrayType *columns, + ArrayType *sortKey); + +extern void PgColumnarDeleteProjectionDeclaration(Oid relid, const char *name); + +extern void PgColumnarDeleteProjectionDeclarationsForRel(Oid relid); + +extern bool PgColumnarStorageHasDeleteVector(uint64 storageId, Snapshot snapshot); + +#endif /* PGCOLUMNAR_METADATA_H */ diff --git a/src/columnar_projection.c b/src/columnar_projection.c index dd22cea0..78629702 100644 --- a/src/columnar_projection.c +++ b/src/columnar_projection.c @@ -22,6 +22,7 @@ */ #include "columnar.h" +#include "columnar_metadata.h" #include "access/table.h" #include "catalog/pg_type.h" #include "funcapi.h" diff --git a/src/columnar_reader.c b/src/columnar_reader.c index 19797965..736af107 100644 --- a/src/columnar_reader.c +++ b/src/columnar_reader.c @@ -14,6 +14,7 @@ */ #include "columnar.h" +#include "columnar_metadata.h" #include "fmgr.h" #include "access/detoast.h" #include "access/htup_details.h" diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index 506c5e7e..34421fff 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -13,6 +13,7 @@ */ #include "columnar.h" +#include "columnar_metadata.h" #include "access/multixact.h" #include "access/genam.h" #include "access/table.h" diff --git a/src/columnar_vacuum.c b/src/columnar_vacuum.c index 70720b8a..118711a8 100644 --- a/src/columnar_vacuum.c +++ b/src/columnar_vacuum.c @@ -20,6 +20,7 @@ *------------------------------------------------------------------------- */ #include "columnar.h" +#include "columnar_metadata.h" #include "columnar_compat.h" #include "fmgr.h" diff --git a/src/columnar_vector.c b/src/columnar_vector.c index 2fcc91a0..89f3b6de 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -36,6 +36,7 @@ */ #include "columnar.h" +#include "columnar_metadata.h" #include #include "miscadmin.h" diff --git a/src/columnar_visibilitymap.c b/src/columnar_visibilitymap.c index f6a3d738..22885b93 100644 --- a/src/columnar_visibilitymap.c +++ b/src/columnar_visibilitymap.c @@ -33,6 +33,7 @@ */ #include "columnar.h" +#include "columnar_metadata.h" #include "fmgr.h" #include "columnar_compat.h" diff --git a/src/columnar_write_state.c b/src/columnar_write_state.c index 959590ba..12a99870 100644 --- a/src/columnar_write_state.c +++ b/src/columnar_write_state.c @@ -13,6 +13,7 @@ */ #include "columnar.h" +#include "columnar_metadata.h" #include "columnar_compat.h" #include "access/htup_details.h" #include "access/table.h"