diff --git a/design/COLUMN_PROJECTION.md b/design/COLUMN_PROJECTION.md new file mode 100644 index 0000000..1729031 --- /dev/null +++ b/design/COLUMN_PROJECTION.md @@ -0,0 +1,127 @@ +# Column projection in the native reader (#338) + +Independent MIT design. References only the public PostgreSQL API (bitmapsets, +`pull_varattnos`, the buffer manager). No core/TimescaleDB/Citus/DuckDB source +consulted. + +## Problem + +`columnar_native_load_group` reads each visited row group's bytes whole and +decodes every column chunk in it, regardless of which columns the query needs. +The projection is computed correctly by `columnar_projected_columns` +(`columnar_customscan.c`), threaded down through `ColumnarBeginRead`, copied into +the read state, and then never read. + +Measured on a 12-column, 4M-row, 351 MB table (44,962 buffers): `sum(a)` touches +45,094 buffers, and so does a sum over all twelve. One column costs the same I/O +as twelve. The identical two-column query reads 100% of a 2-column table (7,620 +buffers) and 100% of a 12-column one (45,097). + +## Change + +Confined to the reader. No on-disk format change, no catalog change. + +1. **`colWanted`** — `ColumnarReadState` gains a `bool *` of length `natts`, + precomputed once in `ColumnarBeginReadWithStorage` from `projectedColumns`. A + NULL bitmap means every column is wanted, which is what every caller outside + the custom scan and the vectorized aggregates passes today, so their behavior + is unchanged by construction. + +2. **Read only the needed byte ranges.** The chunk list is fetched *before* the + data read rather than after it. When every column is wanted, the existing + single whole-group read is kept verbatim. Otherwise the wanted chunks' + `[pageOffset, pageOffset+pageLength)` ranges are read individually, with + file-adjacent ranges coalesced into one read. The full-size group buffer is + still allocated so the existing `base = nativeBuffer + (pageOffset - + fileOffset)` arithmetic stays valid; untouched pages are never faulted in, so + the unread regions cost no resident memory. + + Chunks are written column-major (`columnar_write_state.c:1377`), so a + projected subset is a small number of contiguous runs, not `natts` scattered + reads. + +3. **Skip the decode** of unwanted chunks. This matters as much as the I/O: the + decode loop previously ran `columnar_native_decode_chunk` over every column. + +4. **Emit unwanted columns as explicit NULL.** The row-fill loop treats a NULL + validity pointer as "column absent from this group, added by a later ADD + COLUMN" and substitutes `missingValues[c]`. For a column that was merely *not + projected* that would yield a plausible wrong value rather than an obvious + failure. The unwanted case is therefore tested first and produces an explicit + NULL, keeping the two reasons for an unset cursor distinct. + + This one is defensive, and the tests say so rather than implying otherwise: + removing it leaves all 35 checks passing. It cannot be observed through SQL + today, because nothing above the scan reads a column outside the projection. + It is kept because the cost is one array test per column and the failure it + forecloses -- a future narrowing of the projection silently returning ADD + COLUMN defaults in place of stored data -- is silent and data-shaped. + +5. **Validate that chunks tile the group.** Not part of the original plan; the + corruption suite found it. `corruption.sh` inflates `row_group.byte_length` + and requires a clean error. The whole-group read produced one incidentally, + by reading a length that ran past the end of the relation. A projected read + only touches chunk ranges, so the corrupt length would go unnoticed. + + Rather than weaken the corruption test to match the new path, the reader now + checks the invariant directly: the chunks must exactly tile the group, with + no gap at the start and none at the end. That was verified to hold across + plain inserts, ADD COLUMN, stored generated columns, updates and deletes, + `compact`, `vacuum_sorted`, block-compressed columns, and `VACUUM FULL` + before being relied on. It is a more direct check than the old one: it names + the inconsistency instead of surfacing as a short read. + + It is enforced only on the projected path, so + `enable_column_projection=off` remains a way back if some layout this does + not anticipate ever appears. + +### Why per-vector skipping stays correct + +`allDescriptor` gates per-vector zone-map skipping and is cleared by any +baseline-encoded chunk. Only wanted chunks are now considered, so a baseline +*unwanted* column no longer disables skipping. That is correct and strictly +better: the skip loop advances only cursors that are non-NULL with a non-NULL +`nativeVecRawLen`, which unwanted columns never have, and vector boundaries only +need to line up across the columns actually decoded. + +## Correctness argument + +`columnar_projected_columns` is conservative in exactly the ways this relies on: +it unions the targetlist and the qual, and returns NULL (meaning all columns) for +a whole-row Var, any system column, and for a reference to no column at all. So +a column that is skipped is one no operator above the scan can observe. + +That argument is not self-proving, so the test asserts it empirically rather than +trusting it: every shape is run with projection honored and with it disabled, and +the result sets must be identical. + +## Test (`test/column_projection.sh`) + +- **Equivalence.** Identical results with projection on and off across: single + and multi column targetlists, a qual on a column absent from the targetlist, + `count(*)`, `SELECT *`, whole-row Var, a system column (`ctid`), NULLs, an + all-NULL column, a column added by ALTER TABLE ADD COLUMN with a default + (the `missingValues` interaction), varlena and by-reference types, and after + deletes. +- **Buffers, not timing.** The regression assertion is a buffer count: a + one-column aggregate over a wide table must touch materially less than the + whole relation. Timing would be flaky; buffer counts are exact and fail loudly + if projection silently stops applying. +- **Removal proof.** With `allColumnsWanted` forced true, the buffer assertion + fails (`on: 2517, off: 2517`). Disabling the tiling check fails + `corruption.sh`'s `byte_length` assertion. Removing the explicit-NULL guard + fails nothing, which is why it is described above as defensive rather than + proven. + + Flipping the GUC's *default* proves nothing and was discarded: the suite sets + the GUC explicitly on both arms, so the default never reaches the code under + test. + +## Effect on other suites + +`cancel_decode` measures how quickly a long decode responds to a cancel. Its +query filtered on one column of eight, and with projection that scan no longer +takes long enough to cancel -- its own premise check caught this and failed. +The query now names every column, so the scan again decodes the whole relation +as the test intends, and the suite keeps testing cancellation on the default +path rather than being pinned to the old behavior. diff --git a/src/columnar.h b/src/columnar.h index 632912c..09ac4ca 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -168,6 +168,7 @@ extern int columnar_compression; /* one of COLUMNAR_COMPRESSION_* */ extern int columnar_compression_level; /* zstd level */ extern int columnar_fsst_min_gain_percent; /* min compressed FSST win to keep it (#155) */ extern bool columnar_enable_qual_pushdown; +extern bool columnar_enable_column_projection; extern bool columnar_enable_custom_scan; extern bool columnar_enable_bloom_filter; /* bloom equality skipping (I7) */ diff --git a/src/columnar_reader.c b/src/columnar_reader.c index 70a985f..71a9a48 100644 --- a/src/columnar_reader.c +++ b/src/columnar_reader.c @@ -69,6 +69,18 @@ struct ColumnarReadState bool *missingIsnull; /* [natts] */ Bitmapset *projectedColumns; /* 0-based; NULL means all columns */ + + /* + * Column projection (#338). colWanted[c] is true when column c must actually + * be read and decoded; it is the flattened form of projectedColumns, so the + * per-row and per-chunk tests are an array index rather than a bitmapset + * probe. A NULL projectedColumns means every column is wanted, which is what + * every caller outside the custom scan and the vectorized aggregates passes, + * so those paths keep reading whole groups exactly as before. + */ + bool *colWanted; /* [natts] */ + bool allColumnsWanted; /* true when colWanted is all true */ + SkipPredicate *predicates; /* [numPredicates], in readContext */ int numPredicates; @@ -105,10 +117,11 @@ struct ColumnarReadState /* * Native format (PGCN v1) read state. The scan reads row groups and column - * chunks from the native catalog. The current row group's bytes are read - * whole into nativeBuffer (in groupContext); nativeValidity[c] points at each - * column chunk's validity bitmap and nativeValueCursor[c] advances through its - * uncompressed values. + * chunks from the native catalog. The current row group's bytes are read into + * nativeBuffer (in groupContext) -- whole, or only the projected columns' + * ranges (#338); nativeValidity[c] points at each column chunk's validity + * bitmap and nativeValueCursor[c] advances through its uncompressed values. + * Both stay NULL for a column that was not read. */ List *rowGroupList; /* NativeRowGroupMetadata* */ int rowGroupIndex; /* next row group to load */ @@ -332,6 +345,24 @@ ColumnarBeginReadWithStorage(Relation rel, Snapshot snapshot, } readState->projectedColumns = bms_copy(projectedColumns); + + /* + * Flatten the projection (#338). A NULL bitmap means "all columns" -- that is + * how columnar_projected_columns reports a whole-row Var, any system column, + * and a query referencing no column at all (count(*)), and it is what every + * caller that does not compute a projection passes. + */ + readState->colWanted = palloc(sizeof(bool) * readState->natts); + readState->allColumnsWanted = (projectedColumns == NULL || + !columnar_enable_column_projection); + { + int pc; + + for (pc = 0; pc < readState->natts; pc++) + readState->colWanted[pc] = readState->allColumnsWanted || + bms_is_member(pc, projectedColumns); + } + readState->started = false; readState->exhausted = false; readState->parallelScan = parallelScan; @@ -872,12 +903,173 @@ columnar_native_build_skipvec(ColumnarReadState *rs, uint64 groupNumber, int vec rs->nativeSkipVec = any ? skip : NULL; } +/* a half-open span of the row group's bytes, used to build coalesced reads */ +typedef struct ColumnarByteRange +{ + uint64 start; + uint64 end; +} ColumnarByteRange; + +/* + * columnar_byte_range_cmp + * Order byte ranges by start offset, so adjacent ones can be coalesced. + */ +static int +columnar_byte_range_cmp(const void *a, const void *b) +{ + uint64 sa = ((const ColumnarByteRange *) a)->start; + uint64 sb = ((const ColumnarByteRange *) b)->start; + + if (sa < sb) + return -1; + if (sa > sb) + return 1; + return 0; +} + +/* + * columnar_native_read_projected + * Read only the byte ranges the projected columns occupy (#338), rather + * than the whole row group. + * + * Ranges that touch or overlap in the file are coalesced, so a projection + * covering neighbouring columns costs one read rather than one per column. + * Chunks are written column-major (columnar_write_state.c), so in practice + * a projection is a small number of runs. Everything lands at its natural + * offset inside the full-size group buffer, leaving the rest untouched. + */ +static void +columnar_native_read_projected(ColumnarReadState *rs, + NativeRowGroupMetadata *rg, List *chunks) +{ + ColumnarByteRange *ranges; + uint64 groupEnd = rg->fileOffset + rg->byteLength; + uint64 minStart = groupEnd; + uint64 maxEnd = rg->fileOffset; + bool sawChunk = false; + int n = 0; + int i; + ListCell *lc; + + if (chunks == NIL) + return; + + ranges = (ColumnarByteRange *) + palloc(sizeof(ColumnarByteRange) * list_length(chunks)); + + foreach(lc, chunks) + { + NativeColumnChunkMetadata *cc = (NativeColumnChunkMetadata *) lfirst(lc); + + if (cc->columnIndex < 0 || cc->columnIndex >= rs->natts) + continue; + + /* + * Track the extent of every chunk, wanted or not, so the group can be + * validated below. + */ + if (cc->pageLength > 0) + { + if (cc->pageOffset < minStart) + minStart = cc->pageOffset; + if (cc->pageOffset + cc->pageLength > maxEnd) + maxEnd = cc->pageOffset + cc->pageLength; + sawChunk = true; + } + + if (!rs->colWanted[cc->columnIndex]) + continue; + if (cc->pageLength == 0) + continue; + + /* + * The chunk must lie inside the group it belongs to. The whole-group + * read never had to check this because it read the group as one span; + * reading per chunk turns a bad catalog row into an out-of-bounds write, + * so it is checked rather than assumed. + */ + if (cc->pageOffset < rg->fileOffset || + cc->pageOffset + cc->pageLength > groupEnd) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("columnar chunk for column %d lies outside row group " UINT64_FORMAT, + cc->columnIndex + 1, rg->groupNumber), + errdetail("Chunk spans [" UINT64_FORMAT ", " UINT64_FORMAT ") but the row group is [" + UINT64_FORMAT ", " UINT64_FORMAT ").", + cc->pageOffset, cc->pageOffset + cc->pageLength, + rg->fileOffset, groupEnd))); + + ranges[n].start = cc->pageOffset; + ranges[n].end = cc->pageOffset + cc->pageLength; + n++; + } + + /* + * The chunks must exactly tile the row group they belong to: the writer + * lays them out column-major, back to back, and sets byte_length to their + * total, with no padding between them (verified across plain inserts, ADD + * COLUMN, stored generated columns, updates and deletes, compact, + * vacuum_sorted, block-compressed columns, and VACUUM FULL). + * + * Checking it here is what keeps a corrupt row_group.byte_length detectable. + * The whole-group read caught that incidentally, by trying to read a length + * that ran past the end of the relation; a projected read only touches the + * chunk ranges, so an inflated byte_length would otherwise go unnoticed and + * be silently tolerated. This is the more direct check anyway -- it names + * the inconsistency rather than surfacing as a short read. + * + * Reads with no projection keep the old path untouched, so + * pgcolumnar.enable_column_projection=off remains a way back if a layout + * this does not anticipate ever turns up. + */ + if (sawChunk && (maxEnd != groupEnd || minStart != rg->fileOffset)) + ereport(ERROR, + (errcode(ERRCODE_DATA_CORRUPTED), + errmsg("columnar row group " UINT64_FORMAT " is inconsistent with its column chunks", + rg->groupNumber), + errdetail("The group spans [" UINT64_FORMAT ", " UINT64_FORMAT ") but its chunks span [" + UINT64_FORMAT ", " UINT64_FORMAT ").", + rg->fileOffset, groupEnd, minStart, maxEnd))); + + /* every projected column postdates this group (ADD COLUMN): nothing to read */ + if (n == 0) + return; + + qsort(ranges, n, sizeof(ColumnarByteRange), columnar_byte_range_cmp); + + for (i = 0; i < n;) + { + uint64 start = ranges[i].start; + uint64 end = ranges[i].end; + int j = i + 1; + + while (j < n && ranges[j].start <= end) + { + if (ranges[j].end > end) + end = ranges[j].end; + j++; + } + + ColumnarReadLogicalData(rs->rel, start, + rs->nativeBuffer + (start - rg->fileOffset), + end - start); + i = j; + } + + pfree(ranges); +} + /* * columnar_native_load_group - * Load the next native row group (PGCN v1, Phase D3): read its bytes whole - * into the group context and set each column's validity-bitmap pointer and - * values cursor. Row groups the zone maps prove cannot match are skipped - * (Phase D5b). Returns false when no more row groups remain. + * Load the next native row group (PGCN v1, Phase D3): read the bytes of the + * projected columns into the group context and set each such column's + * validity-bitmap pointer and values cursor. Row groups the zone maps prove + * cannot match are skipped (Phase D5b). Returns false when no more row + * groups remain. + * + * Without a projection this reads the group whole, as it always has. With + * one it reads and decodes only the wanted columns (#338); the others are + * left unmaterialised and the row loop emits NULL for them. */ static bool columnar_native_load_group(ColumnarReadState *rs) @@ -930,13 +1122,32 @@ columnar_native_load_group(ColumnarReadState *rs) MemoryContextReset(rs->groupContext); oldContext = MemoryContextSwitchTo(rs->groupContext); rs->nativeGroup = rg; - rs->nativeBuffer = palloc(rg->byteLength > 0 ? rg->byteLength : 1); - if (rg->byteLength > 0) - ColumnarReadLogicalData(rs->rel, rg->fileOffset, rs->nativeBuffer, - rg->byteLength); + /* + * The chunk metadata is read before the data (#338) because it carries the + * per-column byte ranges the projected read needs. It is a catalog read and + * touches none of the group's data pages. + */ chunks = ColumnarReadColumnChunkList(rs->storageId, rg->groupNumber, rs->metaSnapshot); + + /* + * Column projection (#338). The buffer is always allocated at full group + * size so the base = nativeBuffer + (pageOffset - fileOffset) arithmetic + * below stays valid for every chunk; only the wanted ranges are read into + * it. palloc does not touch the pages it hands back, so the regions that are + * never read cost no resident memory. + */ + rs->nativeBuffer = palloc(rg->byteLength > 0 ? rg->byteLength : 1); + if (rg->byteLength > 0) + { + if (rs->allColumnsWanted) + ColumnarReadLogicalData(rs->rel, rg->fileOffset, rs->nativeBuffer, + rg->byteLength); + else + columnar_native_read_projected(rs, rg, chunks); + } + rs->nativeValidity = palloc0(sizeof(char *) * rs->natts); rs->nativeValueCursor = palloc0(sizeof(char *) * rs->natts); rs->nativeVecRawLen = (uint32 **) palloc0(sizeof(uint32 *) * rs->natts); @@ -954,6 +1165,16 @@ columnar_native_load_group(ColumnarReadState *rs) if (cc->columnIndex < 0 || cc->columnIndex >= rs->natts) continue; + + /* + * Not projected (#338): its bytes were never read, so decoding it would + * read uninitialized buffer. Leaving nativeValidity NULL is what marks + * the column unmaterialised for the row loop, which emits an explicit + * NULL for it rather than the ADD COLUMN missing value. + */ + if (!rs->colWanted[cc->columnIndex]) + continue; + base = rs->nativeBuffer + (cc->pageOffset - rg->fileOffset); rs->nativeValidity[cc->columnIndex] = base; @@ -1127,6 +1348,23 @@ columnar_native_next_row(ColumnarReadState *rs, Datum *values, bool *nulls, Form_pg_attribute att = TupleDescAttr(rs->tupdesc, c); char *vbits = rs->nativeValidity[c]; + /* + * Not projected (#338): never read, so there is no value to give. + * This is tested before the absent-column case on purpose. Both + * leave nativeValidity NULL, but they mean different things, and + * falling through to missingValues here would hand back an ADD + * COLUMN default for a column that simply was not fetched -- a + * plausible wrong value instead of an obvious one. Nothing above + * the scan can read this column (the projection unions the + * targetlist and the qual), so an explicit NULL is unobservable. + */ + if (!rs->colWanted[c]) + { + values[c] = (Datum) 0; + nulls[c] = true; + continue; + } + /* column absent from this group (added by a later ADD COLUMN) */ if (vbits == NULL) { diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index 1f6194e..ee9b8c8 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -64,6 +64,7 @@ int columnar_compression = COLUMNAR_COMPRESSION_ZSTD; int columnar_compression_level = 3; int columnar_fsst_min_gain_percent = 5; bool columnar_enable_qual_pushdown = true; +bool columnar_enable_column_projection = true; bool columnar_enable_bloom_filter = true; /* value set for columnar.compression (spec 5, 8.3) */ @@ -2227,6 +2228,18 @@ _PG_init(void) 0, NULL, NULL, NULL); + DefineCustomBoolVariable("pgcolumnar.enable_column_projection", + "Read only the columns a query references.", + "When off, every column of each visited row group is " + "read and decoded, as before the projection was honored. " + "Provided as an escape hatch and as the A/B oracle the " + "projection tests compare against.", + &columnar_enable_column_projection, + true, + PGC_USERSET, + 0, + NULL, NULL, NULL); + DefineCustomBoolVariable("pgcolumnar.enable_custom_scan", "Use the columnar custom scan path for columnar tables.", NULL, diff --git a/test/cancel_decode.sh b/test/cancel_decode.sh index ad45f09..877def9 100755 --- a/test/cancel_decode.sh +++ b/test/cancel_decode.sh @@ -57,7 +57,14 @@ build() { # build(table, stripe_row_limit) # The predicate matches nothing and is not pushed down, so the scan decodes the # whole relation and hands no tuple to the executor while doing it. -QUERY() { echo "SELECT count(*) FROM $1 WHERE (a %% 7) = 999"; } +# +# Every column is named on purpose. The reader decodes only the columns a query +# references (#338), so a predicate over one column would leave the other seven +# unread and the scan would finish before there was anything to cancel -- the +# premise check below would then fail, correctly, because nothing was being +# tested. Each conjunct is an expression rather than a comparison to a constant +# so that none of them is pushed down to the zone maps and used to skip groups. +QUERY() { echo "SELECT count(*) FROM $1 WHERE (a %% 7) = 999 AND (b %% 7) = 999 AND length(c) < 0 AND length(d) < 0 AND length(e) < 0 AND length(f) < 0 AND length(h) < 0 AND length(i) < 0"; } # Cancel latency: milliseconds from issuing the statement to the error coming # back. Echoes "FAILED" when the statement was not cancelled at all. diff --git a/test/column_projection.sh b/test/column_projection.sh new file mode 100755 index 0000000..a04e4c7 --- /dev/null +++ b/test/column_projection.sh @@ -0,0 +1,192 @@ +#!/usr/bin/env bash +# Column projection (#338): the reader must read and decode only the columns a +# query references. +# +# Two things have to hold, and they pull against each other, so both are checked +# rather than one being inferred from the other: +# +# 1. It is faithful. Skipping a column must not change a single result. The +# oracle is the same query with pgcolumnar.enable_column_projection off, +# which restores the old read-everything behavior, plus a heap table +# holding identical data. +# 2. It actually skips. Correctness alone is satisfied by doing nothing, so +# the win is asserted directly as a buffer count. Buffers, not timings: +# exact, reproducible on a shared runner, and they fail loudly if the +# projection ever silently stops applying. +set -uo pipefail +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" + +GUC=pgcolumnar.enable_column_projection +NOPAR="SET max_parallel_workers_per_gather=0" + +# scalar value of a query at a given setting of the projection GUC +val() { # val + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -Atq -c "$NOPAR" -c "SET $GUC=$1" -c "$2" 2>&1 +} + +# Buffers touched by the scan, at a given setting. +bufs() { # bufs + env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \ + -d "$PGC_DB" -Atq -c "$NOPAR" -c "SET $GUC=$1" \ + -c "EXPLAIN (ANALYZE, BUFFERS) $2" 2>&1 | + grep -m1 -oE 'Buffers: shared[^)]*' | + grep -oE '(hit|read)=[0-9]+' | cut -d= -f2 | paste -sd+ | bc +} + +# The core oracle: projection on must equal projection off, byte for byte. +ab() { # ab