diff --git a/CHANGELOG.md b/CHANGELOG.md index bb4188a..d611be9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,18 @@ which was true until that script existed. ### Changed +- Building an index on a columnar table reads only the columns the index needs + (#413). The table-AM scan interface has nowhere to carry a projection, so the + index-build callback opened a reader that decoded every column: on a 20-column + table, creating an index on one `int` column took 517 ms against heap's 442, + slower than heap at the shape columnar storage should win. The callback is + handed an `IndexInfo`, which names the key and `INCLUDE` columns and carries + the expression and predicate trees, so it now says which columns it needs. The + same build takes 72 ms, and build cost no longer scales with columns the index + does not reference. Expression and partial indexes project their expression and + predicate columns too, since a predicate evaluated against an unread column + would test an unset value. + - The unsupported-rewrite error names `REPACK` on PostgreSQL 19 (#399). `REPACK` replaces `CLUSTER` and `VACUUM FULL` in 19 and dispatches through the same copy-for-cluster path, which pgColumnar does not implement, so a 19 user who diff --git a/src/columnar.h b/src/columnar.h index 6741e6c..9df09a0 100644 --- a/src/columnar.h +++ b/src/columnar.h @@ -615,6 +615,8 @@ extern void PgColumnarParquetCheckExportable(Relation rel); * stripe indices, so several workers scanning the same relation each claim * distinct stripes. Set by the custom scan's DSM init callbacks. */ +extern void PgColumnarReadSetProjection(PgColumnarReadState *readState, + Bitmapset *projectedColumns); extern void PgColumnarReadSetParallelCounter(PgColumnarReadState *readState, pg_atomic_uint32 *counter); diff --git a/src/columnar_reader.c b/src/columnar_reader.c index 4a1ebe2..dac5892 100644 --- a/src/columnar_reader.c +++ b/src/columnar_reader.c @@ -1565,6 +1565,43 @@ PgColumnarReadSetParallelCounter(PgColumnarReadState *readState, * nothing" and is what the aggregate path relies on when every group is * clean. */ +/* + * 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; + + Assert(!readState->started); + if (readState->started) + return; + + bms_free(readState->projectedColumns); + readState->projectedColumns = bms_copy(projectedColumns); + 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); +} + void PgColumnarReadRestrictToGroups(PgColumnarReadState *readState, const uint64 *groupNumbers, int ngroups) diff --git a/src/columnar_tableam.c b/src/columnar_tableam.c index b497d69..ee9331a 100644 --- a/src/columnar_tableam.c +++ b/src/columnar_tableam.c @@ -12,6 +12,7 @@ *------------------------------------------------------------------------- */ #include "columnar.h" +#include "optimizer/optimizer.h" #include "access/multixact.h" #include "access/genam.h" @@ -1408,6 +1409,66 @@ pgcolumnar_relation_copy_for_cluster(COLUMNAR_COPY_FOR_CLUSTER_ARGS) "pgcolumnar.vacuum_sorted() to rewrite in sorted order."))); } +/* + * pgcolumnar_index_projected_columns + * The 0-based set of table columns an index build actually reads, from the + * IndexInfo the callback is already given (issue #413). + * + * ii_IndexAttrNumbers covers key and INCLUDE columns; a zero there marks an + * expression column, whose Vars come from ii_Expressions. A partial index + * also evaluates ii_Predicate per row, and reading a predicate column that + * was not projected would test it against an unset slot value, so the + * predicate's columns are needed exactly as much as the key's. + * + * Returns NULL for "all columns" on a whole-row or system-column reference, + * matching pgcolumnar_projected_columns in columnar_customscan.c. The two + * compute the same thing from different sources and share the convention + * deliberately. + */ +static Bitmapset * +pgcolumnar_index_projected_columns(struct IndexInfo *index_info, int natts) +{ + Bitmapset *needed = NULL; + Bitmapset *projected = NULL; + int i; + int attno; + + if (index_info == NULL) + return NULL; + + for (i = 0; i < index_info->ii_NumIndexAttrs; i++) + { + AttrNumber att = index_info->ii_IndexAttrNumbers[i]; + + /* 0 marks an expression column; its Vars are pulled below */ + if (att == InvalidAttrNumber) + continue; + if (att < 0) + return NULL; /* a system column: read everything */ + needed = bms_add_member(needed, + att - FirstLowInvalidHeapAttributeNumber); + } + + /* index expressions and a partial index's predicate reference varno 1 */ + pull_varattnos((Node *) index_info->ii_Expressions, 1, &needed); + pull_varattnos((Node *) index_info->ii_Predicate, 1, &needed); + + for (attno = FirstLowInvalidHeapAttributeNumber + 1; attno <= 0; attno++) + { + if (bms_is_member(attno - FirstLowInvalidHeapAttributeNumber, needed)) + return NULL; /* whole-row or system column */ + } + + for (attno = 1; attno <= natts; attno++) + { + if (bms_is_member(attno - FirstLowInvalidHeapAttributeNumber, needed)) + projected = bms_add_member(projected, attno - 1); + } + + /* nothing referenced at all: read everything, correct if wasteful */ + return projected; +} + /* * pgcolumnar_index_build_range_scan * Scan every live row of the columnar table and hand it to the index @@ -1480,6 +1541,17 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel, readState = pgcolumnar_scan_read_state((PgColumnarScanDesc) scan, RelationGetDescr(table_rel)); ownReadState = false; + + /* + * The scan was opened through the table-AM interface, which has nowhere + * to carry a projection, so this reader would decode every column. We + * know better here: narrow it before the first read (#413). Each + * participant in a parallel build computes the same set from the same + * IndexInfo, so they agree. + */ + PgColumnarReadSetProjection(readState, + pgcolumnar_index_projected_columns(index_info, + RelationGetDescr(table_rel)->natts)); } else { @@ -1490,7 +1562,10 @@ 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, + pgcolumnar_index_projected_columns(index_info, + RelationGetDescr(table_rel)->natts), + 0, NULL); ownReadState = true; } diff --git a/test/column_projection.sh b/test/column_projection.sh index a04e4c7..1be263d 100755 --- a/test/column_projection.sh +++ b/test/column_projection.sh @@ -189,4 +189,70 @@ check "parallel scan, qual on unselected column" \ "$(par "SELECT sum(v) FROM t WHERE k < 500")" \ "$(val on "SELECT sum(v) FROM h WHERE k < 500")" +# ---- index build: the projection reaches a path the custom scan cannot (#413) ---- +# +# Everything above tests the scan path, where the custom scan node computes the +# projection from the plan. CREATE INDEX does not go through it: the table-AM +# callback opens its own reader, and the reader had no projection, so building an +# index on one column of a wide table decoded all of them. The callback is told +# which columns it needs (IndexInfo), so it can say so. +# +# The metric here is a RATIO rather than the buffer counts used above, and the +# reason is worth stating: EXPLAIN does not cover CREATE INDEX, and +# pg_statio_user_tables counts the heap fork, which is nearly empty for a columnar +# table, so neither exact source is available for this operation. +# +# What is asserted instead is the property itself, which a timing threshold would +# not be: building the SAME single-column index on a narrow and a wide table with +# identical row counts must cost about the same. Two operations, same machine, +# same run, so the ratio does not depend on how fast the box is. +# Measured: 1.0x and 1.2x with projection on, 4.4x and 5.5x with it off. +IDXROWS=${PGC_PROJ_IDX_ROWS:-200000} +wcols=$(for i in $(seq 1 19); do printf ", a%d text" $i; done) +wvals=$(for i in $(seq 1 19); do printf ", repeat(chr(48+%d),80)" $i; done) +psql_run "DROP TABLE IF EXISTS pnarrow; DROP TABLE IF EXISTS pwide; + CREATE TABLE pnarrow (k int, a1 text) USING pgcolumnar; + INSERT INTO pnarrow SELECT g, repeat('1',80) FROM generate_series(1,$IDXROWS) g; + CREATE TABLE pwide (k int$wcols) USING pgcolumnar; + INSERT INTO pwide SELECT g$wvals FROM generate_series(1,$IDXROWS) g; + CREATE TABLE pwide_h (k int$wcols); + INSERT INTO pwide_h SELECT * FROM pwide;" >/dev/null + +idx_ms() { # projection-setting, table, index-name + local s e + psql_run "DROP INDEX IF EXISTS $3;" >/dev/null 2>&1 + s=$(date +%s%N) + psql_run "SET $GUC=$1; CREATE INDEX $3 ON $2 (k);" >/dev/null 2>&1 + e=$(date +%s%N); echo $(( (e - s) / 1000000 )) +} +on_n=$(idx_ms on pnarrow pn_k); on_w=$(idx_ms on pwide pw_k) +off_n=$(idx_ms off pnarrow pn_k); off_w=$(idx_ms off pwide pw_k) +echo "-- #413 index build: projection on ${on_n}/${on_w} ms, off ${off_n}/${off_w} ms (narrow/wide)" + +check_timing "an index build does not scale with columns it does not reference (#413)" \ + "$(awk -v a="$on_w" -v b="$on_n" 'BEGIN { print (b > 0 && a / b < 2.5) ? "yes" : "no" }')" \ + "yes" +# and the control: with projection off it DOES scale, so the check above is not +# passing because the fixture is too small to tell the two apart +check_timing "and with projection off it does scale, so that check discriminates (#413)" \ + "$(awk -v a="$off_w" -v b="$off_n" 'BEGIN { print (b > 0 && a / b > 2.5) ? "yes" : "no" }')" \ + "yes" + +# ---- and the index must still be right, which matters more than the speed ------ +psql_run "CREATE INDEX pw_e ON pwide ((a1 || a2)); + CREATE INDEX pwh_e ON pwide_h ((a1 || a2)); + CREATE INDEX pw_p ON pwide (k) WHERE a19 > CHR(48); + CREATE INDEX pwh_p ON pwide_h (k) WHERE a19 > CHR(48); + CREATE INDEX pwh_k ON pwide_h (k); + ANALYZE pwide; ANALYZE pwide_h;" >/dev/null +check "plain index over a projected build matches heap (#413)" \ + "$(val on "SET enable_seqscan=off; SET enable_bitmapscan=off; SELECT count(*) FROM pwide WHERE k BETWEEN 100 AND 5000")" \ + "$(val on "SET enable_seqscan=off; SET enable_bitmapscan=off; SELECT count(*) FROM pwide_h WHERE k BETWEEN 100 AND 5000")" +check "expression index over a projected build matches heap (#413)" \ + "$(val on "SET enable_seqscan=off; SET enable_bitmapscan=off; SELECT count(*) FROM pwide WHERE (a1 || a2) = repeat('1',80)||repeat('2',80)")" \ + "$(val on "SET enable_seqscan=off; SET enable_bitmapscan=off; SELECT count(*) FROM pwide_h WHERE (a1 || a2) = repeat('1',80)||repeat('2',80)")" +check "partial index over a projected build matches heap (#413)" \ + "$(val on "SET enable_seqscan=off; SET enable_bitmapscan=off; SELECT count(*) FROM pwide WHERE k < 900 AND a19 > CHR(48)")" \ + "$(val on "SET enable_seqscan=off; SET enable_bitmapscan=off; SELECT count(*) FROM pwide_h WHERE k < 900 AND a19 > CHR(48)")" + pgc_summary