From 1e2af6fff1868eecd97f6405e8daf4b6119f7aee Mon Sep 17 00:00:00 2001 From: ChronicallyJD Date: Sun, 9 Aug 2026 19:19:30 -0600 Subject: [PATCH] refactor: the last four module headers, in one change (#496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_state (11), storage (10), customscan (6) and delete_vector (6) — the 33 remaining declarations with exactly one consumer outside their defining file. ONE PR rather than four, and the reason is how #546 and #547 were reviewed: by running checks — every new include used, no moved symbol having more than one consumer — not by reading declarations. Correctness here is established by script, so four diffs are no easier to review than one, and one diff collides with #546 and #547 once instead of up to fifteen times. Every pair of module PRs sharing a consumer conflicts on the #include line, which is what made a six-PR series expensive. Not stacked, deliberately: a stacked PR auto-closes when its base is deleted on merge (#534, this morning). Merge in any order; whichever is second wants a one-minute rebase of the include lines. A defect in my splitting tool, found by the count assert and worth recording because it silently produced a WRONG module assignment rather than an error: columnar.h:353 is extern void PgColumnarEnsureStorageRow(Relation rel);\t/* pre-create ... */ which ends in "*/", not ";". The block scanner ran to the next line ending in ";" and swallowed the declaration below it, filing PgColumnarReserveRowNumbers — a storage symbol — under write_state.h. It compiles either way, because a declaration is visible from whichever header the consumer includes; only the "matched 9 of 10" assert on the NEXT module caught it. Terminator detection now strips trailing comments. I checked whether it reached the open PRs. It did not: #546 and #547 carry exactly their intended sets, 22 and 12, nothing extra, nothing missing, nothing left behind in columnar.h. That is luck — neither module's declarations happened to sit beside one with a trailing comment. All four verified exact: 11/11, 10/10, 6/6, 6/6, no extra symbol, none missing, none stranded. columnar.h goes 1045 -> 997 lines and 164 -> 131 externs. Gate: pg15a, pg16a, pg17a, pg18a, pg19a, make clean between each (#536), zero warnings and zero errors on all five. native_vecdecode 24/24, harness_selftest 54/54, replication 41/41 on pg18a. --- src/columnar.h | 48 +---------------------------------- src/columnar_customscan.c | 1 + src/columnar_customscan.h | 37 +++++++++++++++++++++++++++ src/columnar_delete_vector.c | 1 + src/columnar_delete_vector.h | 35 ++++++++++++++++++++++++++ src/columnar_parallel_copy.c | 1 + src/columnar_projection.c | 1 + src/columnar_reader.c | 2 ++ src/columnar_storage.c | 1 + src/columnar_storage.h | 49 ++++++++++++++++++++++++++++++++++++ src/columnar_tableam.c | 4 +++ src/columnar_vacuum.c | 2 ++ src/columnar_vector.c | 1 + src/columnar_write_state.c | 2 ++ src/columnar_write_state.h | 47 ++++++++++++++++++++++++++++++++++ 15 files changed, 185 insertions(+), 47 deletions(-) create mode 100644 src/columnar_customscan.h create mode 100644 src/columnar_delete_vector.h create mode 100644 src/columnar_storage.h create mode 100644 src/columnar_write_state.h diff --git a/src/columnar.h b/src/columnar.h index 84c68227..16345ec1 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -344,26 +344,8 @@ typedef struct PgColumnarProjection * ------------------------------------------------------------------------- */ struct SMgrRelationData; -extern void PgColumnarWriteNewMetapage(const RelFileLocator *newrlocator, - struct SMgrRelationData *srel, - char persistence, uint64 storageId); extern void PgColumnarReadMetapage(Relation rel, PgColumnarMetapage *meta); extern uint64 PgColumnarStorageId(Relation rel); -extern void PgColumnarEnsureStorageRow(Relation rel); /* pre-create storage row (#300 parallel_copy) */ -extern void PgColumnarReserveRowNumbers(Relation rel, uint64 rowCount, - uint64 *stripeId, uint64 *firstRowNumber); -extern void PgColumnarReserveOffset(Relation rel, uint64 dataLength, - uint64 *fileOffset); -extern void PgColumnarAdvanceReservedOffset(Relation rel, uint64 addBytes); -extern void PgColumnarDebugSetMetapageVersion(Relation rel, uint32 versionMajor, - uint32 versionMinor); -extern void PgColumnarSetReservedOffset(Relation rel, uint64 newOffset); -extern void PgColumnarTruncateMainFork(Relation rel, BlockNumber newnblocks); -extern void PgColumnarWriteLogicalData(Relation rel, uint64 logicalOffset, - char *data, uint64 length); -extern void PgColumnarReadLogicalData(Relation rel, uint64 logicalOffset, - char *dest, uint64 length); -extern void PgColumnarResetMetapage(Relation rel); /* row number <-> item pointer (spec 6) */ extern void PgColumnarRowNumberToItemPointer(uint64 rowNumber, ItemPointer tid); @@ -453,8 +435,7 @@ extern void PgColumnarCheckFreeSpaceNoOverlap(uint64 storageId); extern uint64 PgColumnarNextStorageId(void); /* projection: needed attnos (pull_varattnos form) -> the reader's 0-based set */ -extern Bitmapset *PgColumnarProjectionFromAttnos(Bitmapset *needed, int natts, - int *nProjected); + extern void PgColumnarCheckNativeFormatVersion(uint64 storageId, const char *relName); extern List *PgColumnarReadRowGroupList(uint64 storageId, Snapshot snapshot); extern List *PgColumnarReadZoneMapList(uint64 storageId, uint64 groupNumber, @@ -503,37 +484,17 @@ extern void PgColumnarUpsertDeleteVector(uint64 storageId, DeleteVectorMetadata typedef struct PgColumnarWriteState PgColumnarWriteState; extern PgColumnarWriteState *PgColumnarGetWriteState(Relation rel); -extern int PgColumnarWriteStateStripeCount(PgColumnarWriteState *ws); -extern uint64 *PgColumnarWriteStateStripeIds(PgColumnarWriteState *ws, int *n); -extern uint64 *PgColumnarWriteStateProjStripeIds(PgColumnarWriteState *ws, int *n); extern uint64 PgColumnarWriteRow(PgColumnarWriteState *writeState, Relation rel, Datum *values, bool *nulls); extern void PgColumnarProjectionFanoutRow(Relation rel, PgColumnarWriteState *baseWs, uint64 rowNumber, Datum *values, bool *nulls); -extern void PgColumnarBackfillProjection(Relation rel, - const PgColumnarProjection *proj); -extern bool PgColumnarBufferedRowByNumber(Relation rel, uint64 rowNumber, - Datum *values, bool *nulls); extern void PgColumnarFlushWriteStateForRelation(Oid relid); -extern void PgColumnarForgetWriteStateForRelation(Oid relid); -extern void PgColumnarFlushAllPendingWrites(void); -extern void PgColumnarDiscardAllPendingWrites(void); -extern void PgColumnarWriteStateDiscardSubXact(SubTransactionId subid); -extern void PgColumnarWriteStatePromoteSubXact(SubTransactionId subid, - SubTransactionId parent); /* ------------------------------------------------------------------------- * delete vector / delete tracking (pgcolumnar_delete_vector.c, spec 7.5, 9) * ------------------------------------------------------------------------- */ -extern void PgColumnarMarkRowDeleted(Relation rel, uint64 rowNumber); -extern bool PgColumnarDeleteVectorBufferedDeleted(Relation rel, uint64 rowNumber); extern void PgColumnarFlushDeleteVectorForRelation(Relation rel); -extern void PgColumnarFlushAllDeleteVectors(void); -extern void PgColumnarDiscardAllDeleteVectors(void); -extern void PgColumnarDeleteVectorDiscardSubXact(SubTransactionId subid); -extern void PgColumnarDeleteVectorPromoteSubXact(SubTransactionId subid, - SubTransactionId parent); /* ------------------------------------------------------------------------- * reader (pgcolumnar_reader.c) @@ -831,7 +792,6 @@ extern void PgColumnarUniqueInit(void); /* ------------------------------------------------------------------------- * planner integration (pgcolumnar_customscan.c, spec 8.3, 9) * ------------------------------------------------------------------------- */ -extern void PgColumnarCustomScanInit(void); /* * The single registered CustomScanMethods, shared by the base custom scan and @@ -899,12 +859,6 @@ typedef struct PgColumnarGroupStats uint64 vectorsRuledOutByValue; } PgColumnarGroupStats; -extern void PgColumnarExplainPushedDown(int64 nfilters, ExplainState *es); -extern void PgColumnarExplainVectorPredicates(int64 npreds, ExplainState *es); -extern int PgColumnarCountScanKeys(List *qual, Index scanrelid, - TupleDesc tupdesc); -extern void PgColumnarExplainGroupStats(const PgColumnarGroupStats *stats, - ExplainState *es); extern ScanKey PgColumnarBuildScanKeys(List *qual, Index scanrelid, TupleDesc tupdesc, int *nkeys); diff --git a/src/columnar_customscan.c b/src/columnar_customscan.c index 61c4121f..8cff2232 100644 --- a/src/columnar_customscan.c +++ b/src/columnar_customscan.c @@ -26,6 +26,7 @@ */ #include "columnar.h" +#include "columnar_customscan.h" #include "columnar_reader.h" #include diff --git a/src/columnar_customscan.h b/src/columnar_customscan.h new file mode 100644 index 00000000..5c5d674c --- /dev/null +++ b/src/columnar_customscan.h @@ -0,0 +1,37 @@ +/*------------------------------------------------------------------------- + * + * columnar_customscan.h + * The planner side: what columnar_customscan.c offers the table AM and the vectorized path. + * + * 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. + * + * The shared vocabulary these signatures take stays in columnar.h, which this + * includes. + * + * Written fresh for pgColumnar. + * + *------------------------------------------------------------------------- + */ +#ifndef PGCOLUMNAR_CUSTOMSCAN_H +#define PGCOLUMNAR_CUSTOMSCAN_H + +#include "columnar.h" + +extern Bitmapset *PgColumnarProjectionFromAttnos(Bitmapset *needed, int natts, + int *nProjected); + +extern void PgColumnarCustomScanInit(void); + +extern void PgColumnarExplainPushedDown(int64 nfilters, ExplainState *es); + +extern void PgColumnarExplainVectorPredicates(int64 npreds, ExplainState *es); + +extern int PgColumnarCountScanKeys(List *qual, Index scanrelid, + TupleDesc tupdesc); + +extern void PgColumnarExplainGroupStats(const PgColumnarGroupStats *stats, + ExplainState *es); + +#endif /* PGCOLUMNAR_CUSTOMSCAN_H */ diff --git a/src/columnar_delete_vector.c b/src/columnar_delete_vector.c index 792288da..8520afbd 100644 --- a/src/columnar_delete_vector.c +++ b/src/columnar_delete_vector.c @@ -21,6 +21,7 @@ */ #include "columnar.h" +#include "columnar_delete_vector.h" #include "access/xact.h" #include "utils/memutils.h" #include "utils/rel.h" diff --git a/src/columnar_delete_vector.h b/src/columnar_delete_vector.h new file mode 100644 index 00000000..5168bff7 --- /dev/null +++ b/src/columnar_delete_vector.h @@ -0,0 +1,35 @@ +/*------------------------------------------------------------------------- + * + * columnar_delete_vector.h + * Row deletion: what columnar_delete_vector.c offers the reader and the table AM. + * + * 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. + * + * The shared vocabulary these signatures take stays in columnar.h, which this + * includes. + * + * Written fresh for pgColumnar. + * + *------------------------------------------------------------------------- + */ +#ifndef PGCOLUMNAR_DELETE_VECTOR_H +#define PGCOLUMNAR_DELETE_VECTOR_H + +#include "columnar.h" + +extern void PgColumnarMarkRowDeleted(Relation rel, uint64 rowNumber); + +extern bool PgColumnarDeleteVectorBufferedDeleted(Relation rel, uint64 rowNumber); + +extern void PgColumnarFlushAllDeleteVectors(void); + +extern void PgColumnarDiscardAllDeleteVectors(void); + +extern void PgColumnarDeleteVectorDiscardSubXact(SubTransactionId subid); + +extern void PgColumnarDeleteVectorPromoteSubXact(SubTransactionId subid, + SubTransactionId parent); + +#endif /* PGCOLUMNAR_DELETE_VECTOR_H */ diff --git a/src/columnar_parallel_copy.c b/src/columnar_parallel_copy.c index e8bd6763..fc9e8134 100644 --- a/src/columnar_parallel_copy.c +++ b/src/columnar_parallel_copy.c @@ -43,6 +43,7 @@ #include "columnar.h" +#include "columnar_write_state.h" #include "access/relation.h" #include "access/table.h" #include "access/twophase.h" diff --git a/src/columnar_projection.c b/src/columnar_projection.c index 78629702..e33714a6 100644 --- a/src/columnar_projection.c +++ b/src/columnar_projection.c @@ -23,6 +23,7 @@ #include "columnar.h" #include "columnar_metadata.h" +#include "columnar_write_state.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 37237cd2..3971647f 100644 --- a/src/columnar_reader.c +++ b/src/columnar_reader.c @@ -14,8 +14,10 @@ */ #include "columnar.h" +#include "columnar_delete_vector.h" #include "columnar_metadata.h" #include "columnar_reader.h" +#include "columnar_storage.h" #include "fmgr.h" #include "access/detoast.h" #include "access/htup_details.h" diff --git a/src/columnar_storage.c b/src/columnar_storage.c index 2bce800d..e930ea3c 100644 --- a/src/columnar_storage.c +++ b/src/columnar_storage.c @@ -12,6 +12,7 @@ */ #include "columnar.h" +#include "columnar_storage.h" #include "fmgr.h" #include "columnar_compat.h" #include "access/rmgr.h" diff --git a/src/columnar_storage.h b/src/columnar_storage.h new file mode 100644 index 00000000..ed4cff4b --- /dev/null +++ b/src/columnar_storage.h @@ -0,0 +1,49 @@ +/*------------------------------------------------------------------------- + * + * columnar_storage.h + * The physical layer: what columnar_storage.c offers for metapages, offsets and logical data. + * + * 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. + * + * The shared vocabulary these signatures take stays in columnar.h, which this + * includes. + * + * Written fresh for pgColumnar. + * + *------------------------------------------------------------------------- + */ +#ifndef PGCOLUMNAR_STORAGE_H +#define PGCOLUMNAR_STORAGE_H + +#include "columnar.h" + +extern void PgColumnarWriteNewMetapage(const RelFileLocator *newrlocator, + struct SMgrRelationData *srel, + char persistence, uint64 storageId); + +extern void PgColumnarReserveRowNumbers(Relation rel, uint64 rowCount, + uint64 *stripeId, uint64 *firstRowNumber); + +extern void PgColumnarReserveOffset(Relation rel, uint64 dataLength, + uint64 *fileOffset); + +extern void PgColumnarAdvanceReservedOffset(Relation rel, uint64 addBytes); + +extern void PgColumnarDebugSetMetapageVersion(Relation rel, uint32 versionMajor, + uint32 versionMinor); + +extern void PgColumnarSetReservedOffset(Relation rel, uint64 newOffset); + +extern void PgColumnarTruncateMainFork(Relation rel, BlockNumber newnblocks); + +extern void PgColumnarWriteLogicalData(Relation rel, uint64 logicalOffset, + char *data, uint64 length); + +extern void PgColumnarReadLogicalData(Relation rel, uint64 logicalOffset, + char *dest, uint64 length); + +extern void PgColumnarResetMetapage(Relation rel); + +#endif /* PGCOLUMNAR_STORAGE_H */ diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index 15f8d6c6..7001ab6b 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -13,8 +13,12 @@ */ #include "columnar.h" +#include "columnar_customscan.h" +#include "columnar_delete_vector.h" #include "columnar_metadata.h" #include "columnar_reader.h" +#include "columnar_storage.h" +#include "columnar_write_state.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 118711a8..d87d38f3 100644 --- a/src/columnar_vacuum.c +++ b/src/columnar_vacuum.c @@ -21,6 +21,8 @@ */ #include "columnar.h" #include "columnar_metadata.h" +#include "columnar_storage.h" +#include "columnar_write_state.h" #include "columnar_compat.h" #include "fmgr.h" diff --git a/src/columnar_vector.c b/src/columnar_vector.c index 2d009bb0..489ed5a7 100644 --- a/src/columnar_vector.c +++ b/src/columnar_vector.c @@ -36,6 +36,7 @@ */ #include "columnar.h" +#include "columnar_customscan.h" #include "columnar_metadata.h" #include "columnar_reader.h" #include diff --git a/src/columnar_write_state.c b/src/columnar_write_state.c index 12a99870..cc773d20 100644 --- a/src/columnar_write_state.c +++ b/src/columnar_write_state.c @@ -14,6 +14,8 @@ #include "columnar.h" #include "columnar_metadata.h" +#include "columnar_storage.h" +#include "columnar_write_state.h" #include "columnar_compat.h" #include "access/htup_details.h" #include "access/table.h" diff --git a/src/columnar_write_state.h b/src/columnar_write_state.h new file mode 100644 index 00000000..91d12651 --- /dev/null +++ b/src/columnar_write_state.h @@ -0,0 +1,47 @@ +/*------------------------------------------------------------------------- + * + * columnar_write_state.h + * The write side: what columnar_write_state.c offers the modules that stage and flush rows. + * + * 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. + * + * The shared vocabulary these signatures take stays in columnar.h, which this + * includes. + * + * Written fresh for pgColumnar. + * + *------------------------------------------------------------------------- + */ +#ifndef PGCOLUMNAR_WRITE_STATE_H +#define PGCOLUMNAR_WRITE_STATE_H + +#include "columnar.h" + +extern void PgColumnarEnsureStorageRow(Relation rel); /* pre-create storage row (#300 parallel_copy) */ + +extern int PgColumnarWriteStateStripeCount(PgColumnarWriteState *ws); + +extern uint64 *PgColumnarWriteStateStripeIds(PgColumnarWriteState *ws, int *n); + +extern uint64 *PgColumnarWriteStateProjStripeIds(PgColumnarWriteState *ws, int *n); + +extern void PgColumnarBackfillProjection(Relation rel, + const PgColumnarProjection *proj); + +extern bool PgColumnarBufferedRowByNumber(Relation rel, uint64 rowNumber, + Datum *values, bool *nulls); + +extern void PgColumnarForgetWriteStateForRelation(Oid relid); + +extern void PgColumnarFlushAllPendingWrites(void); + +extern void PgColumnarDiscardAllPendingWrites(void); + +extern void PgColumnarWriteStateDiscardSubXact(SubTransactionId subid); + +extern void PgColumnarWriteStatePromoteSubXact(SubTransactionId subid, + SubTransactionId parent); + +#endif /* PGCOLUMNAR_WRITE_STATE_H */