Skip to content
Merged
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
13 changes: 13 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,19 @@ Measured by the identity of the postmaster each
edit could move the test and leave the message quoting the old threshold. That is
the same defect one level along: a number the reader is told and nothing checks.
`IO_BOUND` now feeds both.
- A base columnar scan was priced from the whole relation file, so adding a
covering projection made the base scan look more expensive even though it
still reads only the base storage.

`pgcolumnar_relation_estimate_size` reported `smgrnblocks` of the main
fork. That file holds the base plus every projection. The planner's
`rel->pages` now subtracts the page-rounded footprints of sibling
projections. Tables with no extra projection keep the same page count.

Measured on PG18 with `seq_page_cost = 1000` and CPU terms zeroed, 20000
rows: the base-scan run stayed 22000 after a covering projection grew the
file from 180224 to 344064 bytes (ratio 1.000). Unfixed, the same scan
jumped to 42000 (ratio 1.909).

- Five premise arms carried a verdict about a number they never printed (#1164).

Expand Down
73 changes: 73 additions & 0 deletions src/columnar_tableam.c
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,50 @@ pgcolumnar_relation_needs_toast_table(Relation rel)
return false;
}

/*
* pgcolumnar_sibling_projection_pages
* Pages occupied by every non-base projection stored in this
* relation's file.
*
* The main fork holds the base plus every projection. The planner's
* rel->pages is smgrnblocks of that file, so a BASE scan is charged
* for pages it will not read. Subtracting this count is a no-op
* when the table has no extra projection.
*
* When projections DO exist this walks every projection's row-group
* list on every estimate_size call (every plan of the table). That is
* planning-time catalog work proportional to projections times groups;
* the no-projection case remains free.
*/
static BlockNumber
pgcolumnar_sibling_projection_pages(uint64 baseStorageId, Snapshot snapshot)
{
List *projs;
ListCell *lc;
uint64 bytes = 0;

projs = PgColumnarListProjections(baseStorageId);
foreach(lc, projs)
{
PgColumnarProjection *pr = (PgColumnarProjection *) lfirst(lc);
List *rgs;
ListCell *rgc;

if (pr->projectionId == 0)
continue;
if (pr->projStorageId == 0 || pr->projStorageId == baseStorageId)
continue;
rgs = PgColumnarReadRowGroupList(pr->projStorageId, snapshot);
foreach(rgc, rgs)
{
NativeRowGroupMetadata *rg = (NativeRowGroupMetadata *) lfirst(rgc);

bytes += COLUMNAR_PAGE_ROUND_UP(rg->byteLength);
}
}
return (BlockNumber) (bytes / COLUMNAR_BYTES_PER_PAGE);
}

static void
pgcolumnar_relation_estimate_size(Relation rel, int32 *attr_widths,
BlockNumber *pages, double *tuples,
Expand Down Expand Up @@ -1097,6 +1141,35 @@ pgcolumnar_relation_estimate_size(Relation rel, int32 *attr_widths,
liveRows = (double) (physicalRows - deleted);
}

/*
* rel->pages is smgrnblocks of the one shared file: base row groups
* plus every projection stored beside them. A base scan reads only
* the base storage. Subtract the sibling projection footprints so
* the planner does not charge that scan for pages it will not visit.
*
* Without a projection this is a no-op, so tables that never grew a
* second copy keep the same page count they had.
*/
{
BlockNumber projPages;

projPages = pgcolumnar_sibling_projection_pages(storageId, snapshot);
if (nblocks > projPages)
nblocks -= projPages;
else
{
/*
* Sibling footprints meeting or exceeding the file should not
* happen (they live in the same file), but stale or orphaned
* projection row groups, a rewrite, or PAGE_ROUND_UP can reach
* it. Floor at one page rather than underflow; a one-page
* estimate for a large table is the wrong answer that would
* otherwise look like a planner bug somewhere else.
*/
nblocks = 1;
}
}

*pages = Max(nblocks, 1);
*tuples = Max(liveRows, 0);

Expand Down
109 changes: 109 additions & 0 deletions test/base_scan_io.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
#!/usr/bin/env bash
#
# pgColumnar: a base scan must not be priced from sibling projection pages.
#
# Projections share the relation's main fork. relation_estimate_size reports
# smgrnblocks of that file as rel->pages, so a scan of the BASE storage is
# charged for every projection stored beside it. Adding a covering projection
# does not make the base scan read more bytes; the planner must not quote it
# as if it did.
#
# This suite pins the PLANNER number, not a runtime. Independent of
# test/pytest/test_base_scan_io.py: same public seam (EXPLAIN cost of a base
# scan before and after a sibling projection lands), own fixture, own
# observations.
#
# Usage: test/base_scan_io.sh [PG_CONFIG]
# Written fresh for pgColumnar.

set -uo pipefail
. "$(dirname "${BASH_SOURCE[0]}")/lib.sh"
pgc_setup "${1:-/usr/lib/postgresql/18/bin/pg_config}"

N=20000
psql_run "CREATE TABLE bsio (nid int, blob text) USING pgcolumnar;"
psql_run "SELECT pgcolumnar.set_options('bsio', stripe_row_limit => 1000, chunk_group_row_limit => 250);"
psql_run "INSERT INTO bsio SELECT nid, repeat('p', 850) FROM generate_series(1, $N) nid ORDER BY md5(nid::text);"
psql_run "ANALYZE bsio;"

explain_base() {
env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U postgres \
-d "$PGC_DB" -Atq \
-c "SET max_parallel_workers_per_gather = 0;" \
-c "SET pgcolumnar.enable_ungrouped_vector_agg = off;" \
-c "SET pgcolumnar.enable_group_vectorization = off;" \
-c "SET jit = off;" \
-c "SET seq_page_cost = 1000;" \
-c "SET cpu_tuple_cost = 0;" \
-c "SET cpu_operator_cost = 0;" \
-c "SET cpu_index_tuple_cost = 0;" \
-c "SET pgcolumnar.enable_projection_scan = off;" \
-c "EXPLAIN (COSTS ON) $1" \
| grep -v '^SET$'
}

scan_cost_pair() {
echo "$1" | grep -F "Custom Scan (PgColumnarScan)" | head -1 \
| grep -oE "cost=[0-9.]+\.\.[0-9.]+" | head -1 \
| sed -E "s/cost=([0-9.]+)\\.\\.([0-9.]+)/\\1 \\2/"
}

run_of() {
local pair start total
pair="$(scan_cost_pair "$1")"
start="${pair%% *}"
total="${pair##* }"
awk -v t="$total" -v s="$start" "BEGIN{ print t-s }"
}

SQL="SELECT nid, blob FROM bsio"
before_plan="$(explain_base "$SQL")"
before_run="$(run_of "$before_plan")"
before_bytes="$(q "SELECT pg_relation_size('bsio')")"

check "premise: the table holds every inserted row" \
"$(q "SELECT count(*) FROM bsio")" "$N"

check "premise: the plan is a base columnar scan" \
"$(echo "$before_plan" | grep -c 'Custom Scan (PgColumnarScan)')" "1"

check "premise: the base scan does not name a covering projection" \
"$(echo "$before_plan" | grep -c 'Columnar Projection:')" "0"

check "premise: the base scan has a positive run cost" \
"$(awk -v c="$before_run" "BEGIN{ print (c>0) ? \"yes\" : \"no\" }")" "yes"

psql_run "SELECT pgcolumnar.add_projection('bsio', 'bynid', ARRAY['nid','blob'], ARRAY['nid']);"

after_bytes="$(q "SELECT pg_relation_size('bsio')")"
after_plan="$(explain_base "$SQL")"
after_run="$(run_of "$after_plan")"
ratio="$(awk -v a="$after_run" -v b="$before_run" "BEGIN{ if (b<=0) print 0; else printf \"%.3f\", a/b }")"

echo "-- before_run=$before_run after_run=$after_run ratio=$ratio"
echo "-- before_bytes=$before_bytes after_bytes=$after_bytes"

check "premise: a covering projection exists" \
"$(q "SELECT count(*) FROM pgcolumnar.projection_declaration WHERE rel = 'bsio'::regclass AND name = 'bynid'")" "1"

# Without this, a pass could mean the projection wrote nothing and both
# formulae agree because the file did not grow.
check "premise: adding the projection enlarged the relation file" \
"$(awk -v a="$after_bytes" -v b="$before_bytes" "BEGIN{ print (b>0 && a > b*1.3) ? \"grew\" : \"stayed before=\" b \" after=\" a }")" \
"grew"

check "premise: the later plan is still a base columnar scan" \
"$(echo "$after_plan" | grep -c 'Custom Scan (PgColumnarScan)')" "1"

check "premise: the later plan still does not name a covering projection" \
"$(echo "$after_plan" | grep -c 'Columnar Projection:')" "0"

# Unfixed: after_run tracks the whole file, so ratio is about the size jump.
# Fixed: the base scan still charges the base storage, so ratio stays near 1.
# Band, not a ceiling: over-subtraction (ratio too small) is the failure mode
# this code newly makes reachable, and a one-sided bound would green it.
check "a base scan is not priced from sibling projection pages" \
"$(awk -v r="$ratio" "BEGIN{ print (r+0 > 1.25 || r+0 < 0.8) ? \"moved ratio=\" r : \"stable\" }")" \
"stable"

pgc_summary
9 changes: 9 additions & 0 deletions test/check_ledger.tsv
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
base_scan_io base_scan_io a base scan is not priced from sibling projection pages 15;16;17;18;19 2026-09-21 projPages = 0 (keep whole-file pages)
base_scan_io base_scan_io premise: a covering projection exists 15;16;17;18;19 never -
base_scan_io base_scan_io premise: adding the projection enlarged the relation file 15;16;17;18;19 never -
base_scan_io base_scan_io premise: the base scan does not name a covering projection 15;16;17;18;19 never -
base_scan_io base_scan_io premise: the base scan has a positive run cost 15;16;17;18;19 never -
base_scan_io base_scan_io premise: the later plan is still a base columnar scan 15;16;17;18;19 never -
base_scan_io base_scan_io premise: the later plan still does not name a covering projection 15;16;17;18;19 never -
base_scan_io base_scan_io premise: the plan is a base columnar scan 15;16;17;18;19 never -
base_scan_io base_scan_io premise: the table holds every inserted row 15;16;17;18;19 never -
differential differential agg avg 15;16;17;18;19 never -
differential differential agg count 15;16;17;18;19 never -
differential differential agg minmax 15;16;17;18;19 never -
Expand Down
6 changes: 5 additions & 1 deletion test/check_ledger_budget.txt
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,8 @@ suites_not_covered 249
# comment about a moving census goes stale by construction, and this comment sits
# directly above the value it contradicts. The command is the durable half.
# Reported by @OffgridwithJD.
checks_never_observed_red 1453
# RESEATED onto origin/main 133c3fbd (post-#1193). Main stated 1453; this
# branch previously stated 1459 against a5c7d5d. Re-counted on THIS tree,
# never by adding:
# awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l
checks_never_observed_red 1461
18 changes: 18 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ behaviour, the source of that number is named.
- [71. test_native_groupagg.py: the grouped vectorized aggregate must answer what core answers](#71-test_native_groupaggpy-the-grouped-vectorized-aggregate-must-answer-what-core-answers)
- [72. test_analyze_function.py: statistics collected by reading, not by sampling](#72-test_analyze_functionpy-statistics-collected-by-reading-not-by-sampling)
- [73. test_assertion_carries_its_measurement.py: a failure must say what it measured](#73-test_assertion_carries_its_measurementpy-a-failure-must-say-what-it-measured)
- [74. test_base_scan_io.py: a base scan is not priced from sibling projection pages](#74-test_base_scan_iopy-a-base-scan-is-not-priced-from-sibling-projection-pages)

## 1. How to read a test in here

Expand Down Expand Up @@ -5777,3 +5778,20 @@ the corpus; none is live, and the file's header carries the measurement behind e
| `test_the_carve_out_is_driven_at_its_boundary` | `> 0` and `>= 1` exactly, written either way round; `r > 1` and `r >= 0` are not the same shape and were being excused |
| `test_a_comparison_wrapped_in_anything_is_still_examined` | `any(r <= 0 for r in runs)` is the natural rewrite of `min(runs) > 0`, so the escape hatch is closed rather than left beside the door |
| `test_a_boolean_combination_is_examined_operand_by_operand` | one lossy operand is enough; a determinate one beside it is no excuse |

## 74. test_base_scan_io.py: a base scan is not priced from sibling projection pages

Port of `base_scan_io.sh`. A base columnar scan inherited `rel->pages` from
`smgrnblocks` of the relation file. That file holds the base plus every
projection stored beside it. Adding a covering projection does not make the
base scan read more bytes; the planner must not quote it as if it did.

Public seam: `EXPLAIN` cost of a base scan (`pgcolumnar.enable_projection_scan
= off`) before and after a sibling projection lands, with `seq_page_cost`
raised and CPU terms zeroed so the run is pages. The shell twin uses `bsio` /
`bynid` / 20000 rows; this file uses `bpages` / `onrid` / 30000 rows.
Assertion names match.

| test | what it holds |
| --- | --- |
| `test_base_scan_io` | the table exists; the plan is a base columnar scan with no covering projection name and a positive run cost; a covering projection then exists and enlarged the file; the later plan is still a base scan; the run cost is not priced from sibling projection pages |
6 changes: 5 additions & 1 deletion test/pytest/expected_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -470,4 +470,8 @@ guard_tests 398
#
# Re-derived by collection on the rebuilt branch: `462 tests collected`. `guard_tests`
# was re-derived in the same run and did NOT move: 382.
cluster_tests 463
# 463 -> 464 when test_base_scan_io.py landed (main already at 463 from
# #1189). Re-derived by collection on this tree, never by adding one:
# `464 tests collected`. `guard_tests` was re-derived in the same run and
# did NOT move: 393.
cluster_tests 464
Loading
Loading