Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 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
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
56 changes: 55 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 @@ -1490,7 +1491,60 @@ pgcolumnar_index_build_range_scan(Relation table_rel, Relation index_rel,
else
snapshot = GetTransactionSnapshot();

readState = PgColumnarBeginRead(table_rel, snapshot, NULL, NULL, 0, NULL);
/*
* 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.
*/
{
Bitmapset *needed = NULL;
Bitmapset *projected;
int nProjected = 0;
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);

projected =
PgColumnarProjectionFromAttnos(needed,
RelationGetDescr(table_rel)->natts,
&nProjected);

/*
* Say what was projected, so a test can assert the projection
* NARROWED rather than infer it from a stopwatch. A fix here 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.
*
* DEBUG1, so it costs nothing at the default log level.
*/
elog(DEBUG1,
"columnar: index build on \"%s\" projecting %d of %d columns",
RelationGetRelationName(index_rel), nProjected,
RelationGetDescr(table_rel)->natts);

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

Expand Down
117 changes: 117 additions & 0 deletions test/native_index_projection.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
#!/usr/bin/env bash
#
# CREATE INDEX projects only the columns the index needs (issue #413).
#
# pgcolumnar_index_build_range_scan opened its reader with no projection, so building a
# one-column index on a wide table decoded every column. It never had to: the callback
# receives IndexInfo, which carries ii_IndexAttrNumbers and the expression and predicate
# trees. The information was in its own arguments and was thrown away.
#
# Measured on 300,000 rows of 20 columns, index on the key alone, non-assert PG18:
#
# before columnar 1,403 ms heap 149 ms 9.4x slower than heap
# after columnar 87 ms heap 144 ms 1.65x faster
#
# What is asserted, in the order the risk sits:
#
# 1. the projection NARROWED, read from the build's own DEBUG1 line. A fix that
# silently did nothing passes every correctness check below, and a wall-clock check
# on a quiet machine, so this is the assertion with teeth.
# 2. expression and partial indexes project their EXTRA columns. Getting those wrong
# evaluates against unset slot values, which is a wrong answer and not a slow one.
# 3. a forced index scan and a forced seq scan agree over the FULL ordered result.
# Point lookups can be satisfied by an index that is wrong for keys nobody asked for.
#
# amcheck runs where the build has contrib and skips VISIBLY where it does not. Source
# builds have no contrib, and a silent skip is the defect this file exists to prevent.
#
# Usage: test/native_index_projection.sh [PG_CONFIG]
# Written fresh for pgColumnar.
set -uo pipefail
. "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
pgc_setup "${1:-/usr/local/pg17/bin/pg_config}"

ROWS=${PGC_IDXPROJ_ROWS:-300000}

# lib.sh's q() emits psql's output for every statement, so "SET ...; SELECT ..." in one
# call returns "SET" as well as the value. Keep the session but read only the result.
qset() { # $1 = SET clause(s), $2 = query
env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \
-d "$PGC_DB" -At -q -c "$1" -c "$2" 2>/dev/null | tail -1
}


# 20 columns: an int key, an int secondary, 18 wide text.
cols="k int, k2 int"
vals="g, g*2"
for i in $(seq 1 18); do cols="$cols, c$i text"; vals="$vals, repeat('x',80)||g"; done
psql_run "CREATE TABLE w ($cols) USING pgcolumnar;"
psql_run "INSERT INTO w SELECT $vals FROM generate_series(1,$ROWS) g;"
check "fixture rows" "$(q 'SELECT count(*) FROM w')" "$ROWS"
check "fixture is wide" \
"$(q "SELECT count(*) FROM pg_attribute WHERE attrelid='w'::regclass AND attnum>0")" "20"

# The projection each build chose, from its own DEBUG1 line. This is the check a
# do-nothing fix would fail and the correctness checks below would not.
proj() { # $1 index name, $2 CREATE INDEX statement -> "n of m"
env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres -d "$PGC_DB" \
-c "SET client_min_messages=debug1;" -c "$2" 2>&1 \
| grep -oE "index build on \"$1\" projecting [0-9]+ of [0-9]+" \
| grep -oE '[0-9]+ of [0-9]+' | head -1
}

check "plain index projects one column of twenty" \
"$(proj w_k 'CREATE INDEX w_k ON w (k)')" "1 of 20"
check "two-column index projects two" \
"$(proj w_k12 'CREATE INDEX w_k12 ON w (k, k2)')" "2 of 20"
check "an index on a late column projects one, not everything before it" \
"$(proj w_c18 'CREATE INDEX w_c18 ON w (c18)')" "1 of 20"
check "an expression index projects its expression's columns" \
"$(proj w_expr 'CREATE INDEX w_expr ON w ((k + k2))')" "2 of 20"
check "a partial index projects the predicate's columns too" \
"$(proj w_part 'CREATE INDEX w_part ON w (k) WHERE k2 > 100')" "2 of 20"
check "an expression over a text column projects that column" \
"$(proj w_len 'CREATE INDEX w_len ON w ((length(c1)))')" "1 of 20"

# Correctness. An index built from under-projected data indexes unset slot values, so
# each of these must find what a sequential scan finds.
check "plain index finds the row" \
"$(qset 'SET enable_seqscan=off' 'SELECT k2 FROM w WHERE k = 12345')" "24690"
check "expression index finds the row" \
"$(qset 'SET enable_seqscan=off' 'SELECT count(*) FROM w WHERE (k + k2) = 30000')" "1"
check "partial index finds the row inside its predicate" \
"$(qset 'SET enable_seqscan=off' 'SELECT count(*) FROM w WHERE k = 50000 AND k2 > 100')" "1"

# The oracle, depending on nothing but PostgreSQL. Compare the FULL ordered result of a
# forced index scan against a forced seq scan.
agree() { # $1 label, $2 predicate, $3 selected expression
local viaix viaseq
viaix=$(qset "SET enable_seqscan=off; SET enable_bitmapscan=off" \
"SELECT md5(string_agg(t::text, ',' ORDER BY t))
FROM (SELECT $3 AS t FROM w WHERE $2) s")
viaseq=$(qset "SET enable_indexscan=off; SET enable_bitmapscan=off; SET enable_indexonlyscan=off" \
"SELECT md5(string_agg(t::text, ',' ORDER BY t))
FROM (SELECT $3 AS t FROM w WHERE $2) s")
check "$1" "$viaix" "$viaseq"
}
agree "plain index agrees with a sequential scan" "k BETWEEN 1000 AND 9999" "k"
agree "late-column index agrees" "c18 > repeat('x',80)||'99000'" "k"
agree "expression index agrees" "(k + k2) BETWEEN 300 AND 30000" "k"
agree "partial index agrees on its subset" "k2 > 100 AND k < 5000" "k"
agree "text expression index agrees" "length(c1) = 83" "k"

# amcheck where available. The skip must be visible: a check that reports nothing is
# indistinguishable from a check that passes, which is what this file is about.
if psql_run "CREATE EXTENSION IF NOT EXISTS amcheck;" >/dev/null 2>&1 &&
[ "$(q "SELECT count(*) FROM pg_proc WHERE proname='bt_index_check'")" != "0" ]; then
for ix in w_k w_k12 w_c18 w_expr w_part w_len; do
out=$(env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \
-d "$PGC_DB" -c "SELECT bt_index_check('$ix'::regclass)" 2>&1)
check "bt_index_check($ix)" \
"$(grep -qE 'ERROR' <<<"$out" && echo bad || echo ok)" "ok"
done
else
echo "SKIP amcheck is not installed on this build; the seq-scan oracle above still ran"
fi

pgc_summary
2 changes: 1 addition & 1 deletion test/run_all_versions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ SRCDIR="${PGC_RUN_SRCDIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)}"
SUITES=(harness_selftest docs_style smoke phase2 phase3 phase4 phase5 phase6 audit concurrency unique_conc \
differential recovery replication native_backend_crash fuzz fuzz_parquet fuzz_arrow hardening concurrent_diff parallel sorted_projection \
arrow_export parquet_export read_stream corruption \
generated_columns temporal arrow_import index_only projections arrow_nested parquet_import parquet_nested arrow_nested_import parquet_nested_import native_writer native_roundtrip native_encoding native_fastdecode native_zonemap write_minmax_fastpath write_fsst_compressed fsst_margin encode_invariants encode_effort native_skip pushdown_report native_agg native_agg_deletes native_agg_addcolumn native_groupagg ungrouped_vector_agg parallel_vector_agg native_bloom bloom_setting bloom_lazy native_vecskip native_index native_fetch_position native_dml alter_column_type native_ios native_projection native_cluster native_repack native_compact native_recluster recluster_extent native_vacuum_race native_sort_by sort_status native_reclaim native_ownership drop_cleanup pg_dump_roundtrip native_reclaim_cycles native_reclaim_frag native_reclaim_reconcile native_gap native_format native_truncate native_rewrite native_rewrite_conc rewrite_group_scan native_parquet_schema native_read_parquet native_parquet_fdw native_parquet_pushdown native_parquet_hardening server_file_privilege native_parquet_stack native_parquet_units native_parquet_flba native_parquet_codecs native_parquet_projection native_parquet_multifile native_parquet_streaming native_parquet_partition native_cancel cancel_decode wal_envelope decode_interrupts import_exclusion import_deferred parallel_copy parallel_export_parquet fk_referencing row_triggers native_lazy_slot native_ctas native_fetch_cache native_fetch_interrupt analyze_stats analyze_reltuples native_fetch_projection column_projection isolation)
generated_columns temporal arrow_import index_only projections arrow_nested parquet_import parquet_nested arrow_nested_import parquet_nested_import native_writer native_roundtrip native_encoding native_fastdecode native_zonemap write_minmax_fastpath write_fsst_compressed fsst_margin encode_invariants encode_effort native_skip pushdown_report native_agg native_agg_deletes native_agg_addcolumn native_groupagg ungrouped_vector_agg parallel_vector_agg native_bloom bloom_setting bloom_lazy native_vecskip native_index native_index_projection native_fetch_position native_dml alter_column_type native_ios native_projection native_cluster native_repack native_compact native_recluster recluster_extent native_vacuum_race native_sort_by sort_status native_reclaim native_ownership drop_cleanup pg_dump_roundtrip native_reclaim_cycles native_reclaim_frag native_reclaim_reconcile native_gap native_format native_truncate native_rewrite native_rewrite_conc rewrite_group_scan native_parquet_schema native_read_parquet native_parquet_fdw native_parquet_pushdown native_parquet_hardening server_file_privilege native_parquet_stack native_parquet_units native_parquet_flba native_parquet_codecs native_parquet_projection native_parquet_multifile native_parquet_streaming native_parquet_partition native_cancel cancel_decode wal_envelope decode_interrupts import_exclusion import_deferred parallel_copy parallel_export_parquet fk_referencing row_triggers native_lazy_slot native_ctas native_fetch_cache native_fetch_interrupt analyze_stats analyze_reltuples native_fetch_projection column_projection isolation)

# Default matrix: one assert-enabled pg_config per major, 15 through 19.
DEFAULT_CONFIGS=(
Expand Down
Loading