Skip to content
Merged
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ which was true until that script existed.
setting to the other's storage.
- `default_version` moves from `1.0-dev` to `1.0-alpha`, so
`SELECT extversion FROM pg_extension` now agrees with `VERSION`.
- `CREATE INDEX` decodes only the columns the index needs (#413). The index
build received an `IndexInfo` carrying the key columns and the expression and
predicate trees, and discarded it, so a one-column index on a wide table read
every column. Both readers are now projected: the one a serial build opens for
itself, and the shared scan a parallel build arrives with, which comes through
the table-access-method scan interface and has nowhere to carry a projection.
The parallel branch is not a corner case. With every parallel setting left at
its default, a 1.5 million row table of incompressible text, 459 MB on disk,
is built in parallel, so that is the branch a table of consequential size
takes. On 300,000 rows of 20 columns on PostgreSQL 18, a one-column index
drops from 568 ms to 73 ms with workers allowed, against 563 ms for the same
index on a heap table.

### Upgrading

Expand Down
16 changes: 16 additions & 0 deletions src/columnar.h
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,10 @@ extern void PgColumnarCheckFreeSpaceNoOverlap(uint64 storageId);
* ------------------------------------------------------------------------- */
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);
Expand Down Expand Up @@ -610,6 +614,18 @@ extern int64 PgColumnarWriteParquetFile(Relation rel, Snapshot snapshot,
int nRestrictGroups);
extern void PgColumnarParquetCheckExportable(Relation rel);

/*
* Column projection on an already-opened reader (#413). The table-AM scan
* interface has nowhere to carry a projection, so a reader obtained through it
* reads every column; a caller that knows better narrows it here, before the
* first read. PgColumnarReadProjectedCount reports what the reader WILL decode,
* read off colWanted, so a caller reporting a projection cannot report one it
* failed to apply.
*/
extern void PgColumnarReadSetProjection(PgColumnarReadState *readState,
Bitmapset *projectedColumns);
extern int PgColumnarReadProjectedCount(PgColumnarReadState *readState);

/*
* Parallel scan (gap 23): point the read state at a shared atomic that hands out
* stripe indices, so several workers scanning the same relation each claim
Expand Down
33 changes: 26 additions & 7 deletions src/columnar_customscan.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,17 +169,23 @@ static const CustomExecMethods pgcolumnar_exec_methods = {
* carry a ctid system Var). This is the projection pushed into the reader
* (spec 9).
*/
static Bitmapset *
pgcolumnar_projected_columns(CustomScan *cscan, int natts, int *nProjected)
/*
* PgColumnarProjectionFromAttnos
* Turn a set of needed attribute numbers, in pull_varattnos' offset form,
* into the reader's 0-based projection. Returns NULL for "read every
* column", which is what a system column, a whole-row Var, or an empty set
* all mean.
*
* Shared because index_build_range_scan needs the same computation over a
* different source (#413): its columns come from IndexInfo rather than from
* a plan. The escapes are the interesting part and are worth having once.
*/
Bitmapset *
PgColumnarProjectionFromAttnos(Bitmapset *needed, int natts, int *nProjected)
{
Bitmapset *needed = NULL;
Bitmapset *projected = NULL;
Index scanrelid = cscan->scan.scanrelid;
int attno;

pull_varattnos((Node *) cscan->scan.plan.targetlist, scanrelid, &needed);
pull_varattnos((Node *) cscan->scan.plan.qual, scanrelid, &needed);

/* a system column or whole-row Var forces reading every column */
for (attno = FirstLowInvalidHeapAttributeNumber + 1; attno <= 0; attno++)
{
Expand Down Expand Up @@ -210,6 +216,19 @@ pgcolumnar_projected_columns(CustomScan *cscan, int natts, int *nProjected)
return projected;
}

static Bitmapset *
pgcolumnar_projected_columns(CustomScan *cscan, int natts, int *nProjected)
{
Bitmapset *needed = NULL;
Index scanrelid = cscan->scan.scanrelid;

pull_varattnos((Node *) cscan->scan.plan.targetlist, scanrelid, &needed);
pull_varattnos((Node *) cscan->scan.plan.qual, scanrelid, &needed);

return PgColumnarProjectionFromAttnos(needed, natts, nProjected);
}


/*
* pgcolumnar_commute_strategy
* The btree comparison strategy for "value op column" given the strategy
Expand Down
72 changes: 72 additions & 0 deletions src/columnar_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -1553,6 +1553,78 @@ PgColumnarReadSetParallelCounter(PgColumnarReadState *readState,
readState->parallelCounter = counter;
}

/*
* PgColumnarReadSetProjection
* Narrow an already-opened reader to a set of columns (issue #413).
*
* The table-AM scan interface has nowhere to put a projection, so a reader
* opened through pgcolumnar_scan_begin reads every column. A caller that
* does know which columns it needs -- an index build knows, from IndexInfo
* -- can say so here instead.
*
* Only legal before the first read. colWanted drives what the group loader
* decodes, and a group already loaded under a wider projection would be
* reused under a narrower one, so changing it mid-scan would silently
* return unset values rather than fail. The caller is expected to do this
* immediately after obtaining the reader; the assertion states the rule and
* the early return keeps a release build honest.
*
* A NULL set means "all columns", matching PgColumnarBeginRead.
*/
void
PgColumnarReadSetProjection(PgColumnarReadState *readState,
Bitmapset *projectedColumns)
{
int pc;
MemoryContext old;

Assert(!readState->started);
if (readState->started)
return;

/*
* Copy into the read state's own context, not the caller's. Nothing reads
* this field after the setter today, so this is consistency rather than a
* fixed bug -- PgColumnarBeginRead builds the same field in readContext and
* PgColumnarReadRestrictToGroups says so in its own comment. A field owned by
* two different contexts depending on which function set it is a trap for
* whoever reads it next.
*/
old = MemoryContextSwitchTo(readState->readContext);
bms_free(readState->projectedColumns);
readState->projectedColumns = bms_copy(projectedColumns);
MemoryContextSwitchTo(old);
readState->allColumnsWanted = (projectedColumns == NULL ||
!pgcolumnar_enable_column_projection);
for (pc = 0; pc < readState->natts; pc++)
readState->colWanted[pc] = readState->allColumnsWanted ||
bms_is_member(pc, projectedColumns);
}

/*
* PgColumnarReadProjectedCount
* How many columns this reader will actually decode.
*
* Read from colWanted, the field the group loader consults, so a caller
* reporting a projection reports what the reader WILL DO rather than what
* the caller computed and may have failed to apply. That distinction is
* the whole point: a projection computed and then dropped on the floor is
* exactly the bug this accessor exists to make visible (#413).
*/
int
PgColumnarReadProjectedCount(PgColumnarReadState *readState)
{
int pc;
int n = 0;

for (pc = 0; pc < readState->natts; pc++)
{
if (readState->colWanted[pc])
n++;
}
return n;
}

/*
* PgColumnarReadRestrictToGroups
* Restrict this scan to the given row group numbers (issue #149). Groups
Expand Down
83 changes: 82 additions & 1 deletion src/columnar_tableam.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "executor/tuptable.h"
#include "miscadmin.h"
#include "nodes/pathnodes.h"
#include "optimizer/optimizer.h"
#include "optimizer/pathnode.h"
#include "optimizer/plancat.h"
#include "port/atomics.h"
Expand Down Expand Up @@ -1431,6 +1432,8 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
{
PgColumnarReadState *readState;
bool ownReadState;
Bitmapset *projected;
int nProjected = 0;
EState *estate;
ExprContext *econtext;
ExprState *predicate;
Expand Down Expand Up @@ -1470,6 +1473,50 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
* snapshot. The reader advances the command id internally for
* read-your-writes.
*/
/*
* Project. The columns this build needs are all in our own arguments and we
* were throwing them away, so a one-column index on a wide table decoded
* every column (#413).
*
* Three sources, and missing any of them reads unset slot values:
* ii_IndexAttrNumbers the key columns
* ii_Expressions an expression index references more
* ii_Predicate a partial index evaluates against more
*
* PgColumnarProjectionFromAttnos returns NULL for "every column", which
* covers a whole-row or system-column reference, and is what the custom scan
* already does with the same escapes.
*
* Computed before the branch because BOTH readers need it. Every
* participant in a parallel build computes the same set from the same
* IndexInfo, so they agree without having to communicate.
*/
{
Bitmapset *needed = NULL;
int i;

for (i = 0; i < index_info->ii_NumIndexAttrs; i++)
{
AttrNumber attno = index_info->ii_IndexAttrNumbers[i];

/* 0 marks an expression column; Vars come from ii_Expressions */
if (attno != 0)
needed = bms_add_member(needed,
attno - FirstLowInvalidHeapAttributeNumber);
}
pull_varattnos((Node *) index_info->ii_Expressions, 1, &needed);
pull_varattnos((Node *) index_info->ii_Predicate, 1, &needed);

/*
* nProjected is a required out-parameter, not the number we report.
* The DEBUG1 line below reads the count off the reader instead; see
* the comment there for why.
*/
projected = PgColumnarProjectionFromAttnos(needed,
RelationGetDescr(table_rel)->natts,
&nProjected);
}

if (scan != NULL)
{
/*
Expand All @@ -1479,6 +1526,18 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
*/
readState = pgcolumnar_scan_read_state((PgColumnarScanDesc) scan,
RelationGetDescr(table_rel));

/*
* This reader was opened through the table-AM scan interface, which has
* nowhere to carry a projection, so it would decode every column. We
* know better here, so narrow it before the first read.
*
* This branch is not hypothetical and it is not the rare case: with
* parallel maintenance workers available, EVERY participant including
* the leader arrives here, and the serial branch below never runs. Fix
* only the serial branch and a parallel build stays unprojected.
*/
PgColumnarReadSetProjection(readState, projected);
ownReadState = false;
}
else
Expand All @@ -1490,10 +1549,32 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
else
snapshot = GetTransactionSnapshot();

readState = PgColumnarBeginRead(table_rel, snapshot, NULL, NULL, 0, NULL);
readState = PgColumnarBeginRead(table_rel, snapshot, NULL,
projected, 0, NULL);
ownReadState = true;
}

/*
* Say which branch ran and how wide the reader it produced will actually
* read, so a test can assert the projection NARROWED rather than infer it
* from a stopwatch. A fix that silently did nothing would pass every
* correctness check, and a wall-clock check on a quiet machine, which is
* the failure mode this projection is being added to avoid.
*
* The count comes from the READER, not from nProjected. Reporting what we
* computed would keep printing "1 of 20" if the parallel branch stopped
* applying it, and the assertion guarding that branch would pass while the
* build read every column. Reporting what the reader will decode cannot.
*
* DEBUG1, so it costs nothing at the default log level.
*/
elog(DEBUG1,
"columnar: %s index build on \"%s\" projecting %d of %d columns",
scan != NULL ? "parallel" : "serial",
RelationGetRelationName(index_rel),
PgColumnarReadProjectedCount(readState),
RelationGetDescr(table_rel)->natts);

while (true)
{
CHECK_FOR_INTERRUPTS();
Expand Down
Loading
Loading