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 @@ -390,6 +390,19 @@ true until the next version shipped.
and cuts sentences with `sed`; the pytest half splits on a lookbehind and collects
with `re`, so a parsing mistake in one is not a parsing mistake in the other.

- Planning a columnar query sequentially scanned `pgcolumnar.options` and `pgcolumnar.projection` (#1198).

`options_pkey` is `(regclass)` and `projection_pkey` leads with `storage_id`. The planner looks those catalogs up by those columns, and the scans passed `InvalidOid`. One filtered scan of one table then walked every columnar table's options row and every projection row. Measured on PG18 before the change, after `pg_stat_reset()`:

| catalog | idx_scan | seq_scan |
| --- | ---: | ---: |
| options | 0 | 2 |
| projection | 0 | 2 |

Both readers now pass their primary key, as `row_group` and `delete_vector` already did. The same lookup on `pgcolumnar.storage` stays a sequential scan: `storage_pkey` is on `storage_id` and that reader looks up by `relation_oid`.

After the change the same scan reports `idx_scan=2` and `seq_scan=0` on both catalogs, and the filtered count is still every row. Putting `InvalidOid` back on the two planner scans returns `idx_scan=0` and `seq_scan=2`.

- `native_reclaim_cycles` could not reach the defect it guards (#1138).

It is the declared regression guard for #84. Deleting the #84 fix left it reporting
Expand Down
99 changes: 88 additions & 11 deletions src/columnar_metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -2031,7 +2031,49 @@ PgColumnarRenameDeclaredSortByColumn(Oid relid, const char *oldName,
tupdesc = RelationGetDescr(rel);
ScanKeyInit(&key[0], Anum_options_regclass, BTEqualStrategyNumber,
F_OIDEQ, ObjectIdGetDatum(relid));
scan = systable_beginscan(rel, InvalidOid, false, NULL, 1, key);

/*
* options_pkey, like every other scan whose key is that column. The sixth
* site: five moved when the others did and this one did not, and the
* population was a list of function names rather than the property
* "the key column IS the index's column" -- which is what a list silently
* regrows past.
*
* AND THE PROPERTY IS WIDER THAN THIS CHANGE. Counted on both trees rather
* than on one:
*
* main 44 systable_beginscan calls, 31 InvalidOid, 13 indexed
* this branch 44 , 24 , 20
*
* so this change resolves SEVEN sites, and 21 of the remaining 24 have a scan
* key that is a prefix of an existing index. None of the 21 is called a defect
* here, because none has been measured the way these seven were. #1207 carries
* them, with line numbers.
*
* NO PER-CATALOG TABLE HERE, DELIBERATELY. Three independent sweeps agreed on
* 21 and disagreed on how it splits, because a sweep that searches BACKWARDS
* for the nearest ScanKeyInit mis-assigns any function that scans two
* catalogs -- PgColumnarCheckFreeSpaceNoOverlap scans row_group at 1020 and
* free_space at 1041, and a backward search gives both to whichever key it
* meets first. The relation HANDLE passed to systable_beginscan is the ground
* truth; tracing it to its open_columnar_table("<name>") settles it. A table
* of counts in a comment is the thing this comment is warning about.
*
* THE COUNTS ABOVE ARE OF CALLS, NOT OF THE STRING. A first draft said 46 and
* "six", from a `grep -c systable_beginscan` that counted this very
* paragraph's own sentence about systable_beginscan. A comment describing a
* sweep is input to that sweep, which is the same trap one level down from the
* one it is describing.
*
* NO MEASUREMENT WILL SHOW THIS ONE. It runs on ALTER TABLE ... RENAME
* COLUMN, not on a plan, so the planner-path probe that found the others
* cannot reach it. That is the reason to fix it rather than a reason not to.
*/
{
Oid optIdx = pgcolumnar_index_oid("options_pkey");

scan = systable_beginscan(rel, optIdx, OidIsValid(optIdx), NULL, 1, key);
}
if (HeapTupleIsValid(tuple = systable_getnext(scan)))
{
bool isnull;
Expand Down Expand Up @@ -3183,7 +3225,16 @@ PgColumnarReadOptions(Oid relid, PgColumnarOptions *opts)
ScanKeyInit(&key[0], Anum_options_regclass, BTEqualStrategyNumber,
F_OIDEQ, ObjectIdGetDatum(relid));

scan = systable_beginscan(rel, InvalidOid, false, snapshot, 1, key);
/*
* options_pkey is (regclass), the same column this key names. Passing
* InvalidOid walked every columnar table's options row on a plan that
* asked about one of them.
*/
{
Oid optIdx = pgcolumnar_index_oid("options_pkey");

scan = systable_beginscan(rel, optIdx, OidIsValid(optIdx), snapshot, 1, key);
}
if (HeapTupleIsValid(tuple = systable_getnext(scan)))
{
bool isnull;
Expand Down Expand Up @@ -3298,8 +3349,9 @@ pgcolumnar_effective_stripe_row_limit(Oid relid)
* of groups on every plan, which is too much to spend refining a term that
* is approximate by construction.
*
* Scanned without an index, like PgColumnarReadOptions immediately above:
* the storage index is on storage_id and this looks up by relation_oid.
* Scanned without an index: storage_pkey is on storage_id and this
* looks up by relation_oid. options_pkey does match its lookup, and
* PgColumnarReadOptions uses it; this one cannot.
*/
int
pgcolumnar_written_stripe_row_limit(Oid relid)
Expand Down Expand Up @@ -3400,7 +3452,11 @@ PgColumnarReadTtl(Oid relid, char **column, Interval **interval)

ScanKeyInit(&key[0], Anum_options_regclass, BTEqualStrategyNumber,
F_OIDEQ, ObjectIdGetDatum(relid));
scan = systable_beginscan(rel, InvalidOid, false, snapshot, 1, key);
{
Oid optIdx = pgcolumnar_index_oid("options_pkey");

scan = systable_beginscan(rel, optIdx, OidIsValid(optIdx), snapshot, 1, key);
}
tuple = systable_getnext(scan);
if (HeapTupleIsValid(tuple))
{
Expand Down Expand Up @@ -3443,7 +3499,11 @@ PgColumnarReadSortBy(Oid relid)
ScanKeyInit(&key[0], Anum_options_regclass, BTEqualStrategyNumber,
F_OIDEQ, ObjectIdGetDatum(relid));

scan = systable_beginscan(rel, InvalidOid, false, snapshot, 1, key);
{
Oid optIdx = pgcolumnar_index_oid("options_pkey");

scan = systable_beginscan(rel, optIdx, OidIsValid(optIdx), snapshot, 1, key);
}
if (HeapTupleIsValid(tuple = systable_getnext(scan)))
{
bool isnull;
Expand Down Expand Up @@ -3493,7 +3553,11 @@ PgColumnarDeleteOptions(Oid relid)
ScanKeyInit(&key[0], Anum_options_regclass, BTEqualStrategyNumber,
F_OIDEQ, ObjectIdGetDatum(relid));

scan = systable_beginscan(rel, InvalidOid, false, NULL, 1, key);
{
Oid optIdx = pgcolumnar_index_oid("options_pkey");

scan = systable_beginscan(rel, optIdx, OidIsValid(optIdx), NULL, 1, key);
}
while (HeapTupleIsValid(tuple = systable_getnext(scan)))
CatalogTupleDelete(rel, &tuple->t_self);
systable_endscan(scan);
Expand Down Expand Up @@ -3910,9 +3974,18 @@ PgColumnarListProjections(uint64 storageId)
ScanKeyInit(&key[0], Anum_projection_storage_id, BTEqualStrategyNumber,
F_INT8EQ, Int64GetDatum((int64) storageId));

/* NULL snapshot -> catalog snapshot: sees committed rows plus this
* transaction's own writes after a CommandCounterIncrement (DDL semantics). */
scan = systable_beginscan(rel, InvalidOid, false, NULL, 1, key);
/*
* NULL snapshot -> catalog snapshot: sees committed rows plus this
* transaction's own writes after a CommandCounterIncrement (DDL
* semantics). projection_pkey leads with storage_id, so the index
* answers this key. The snapshot is what makes the index scan see
* those writes; dropping it would not.
*/
{
Oid projIdx = pgcolumnar_index_oid("projection_pkey");

scan = systable_beginscan(rel, projIdx, OidIsValid(projIdx), NULL, 1, key);
}
while (HeapTupleIsValid(tuple = systable_getnext(scan)))
{
PgColumnarProjection *p = palloc0(sizeof(PgColumnarProjection));
Expand Down Expand Up @@ -3953,7 +4026,11 @@ PgColumnarDeleteProjectionRow(uint64 storageId, int projectionId)
ScanKeyInit(&key[1], Anum_projection_projection_id, BTEqualStrategyNumber,
F_INT4EQ, Int32GetDatum(projectionId));

scan = systable_beginscan(rel, InvalidOid, false, NULL, 2, key);
{
Oid projIdx = pgcolumnar_index_oid("projection_pkey");

scan = systable_beginscan(rel, projIdx, OidIsValid(projIdx), NULL, 2, key);
}
while (HeapTupleIsValid(tuple = systable_getnext(scan)))
CatalogTupleDelete(rel, &tuple->t_self);
systable_endscan(scan);
Expand Down
76 changes: 76 additions & 0 deletions test/catalog_plan_index.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
#!/usr/bin/env bash
#
# Planning a columnar query must probe pgcolumnar.options and
# pgcolumnar.projection through their primary keys.
#
# options_pkey is (regclass) and projection_pkey is (storage_id,
# projection_id). The planner looks options up by regclass and projections
# up by storage_id, and both scans passed InvalidOid, so every plan
# sequentially scanned those catalogs. A database with many columnar
# tables pays that on a query that touches one of them.
#
# After one filtered scan of this suite's own table, pg_stat_all_tables
# must show idx_scan > 0 and seq_scan = 0 for both catalogs. The filtered
# scan is its own psql, so the session that wrote the rows is not the
# session being measured.
#
# Usage: test/catalog_plan_index.sh [PG_CONFIG]

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

q "CREATE EXTENSION IF NOT EXISTS pgcolumnar;" >/dev/null

# Other columnar tables sit in the same catalogs. A sequential scan of
# options or projection walks their rows too; an index probe does not.
q "CREATE TABLE noise_a (id int) USING pgcolumnar;
CREATE TABLE noise_b (id int) USING pgcolumnar;
INSERT INTO noise_a SELECT g FROM generate_series(1,40) g;
INSERT INTO noise_b SELECT g FROM generate_series(1,60) g;
CREATE TABLE plan_cat (id int) USING pgcolumnar;
INSERT INTO plan_cat SELECT g FROM generate_series(1,800) g;" >/dev/null

check_num "premise: the measured table holds its rows" \
"$(q "SELECT count(*) FROM plan_cat;")" "800"

q "SELECT pg_stat_reset();" >/dev/null
q "SELECT count(*) FROM plan_cat WHERE id > 0;" >/dev/null
q "SELECT pg_stat_force_next_flush();" >/dev/null

check_num "premise: the filtered scan returned every row" \
"$(q "SELECT count(*) FROM plan_cat WHERE id > 0;")" "800"

opt="$(q "SELECT coalesce(idx_scan,0)::text || ' ' || coalesce(seq_scan,0)::text
FROM pg_stat_all_tables
WHERE schemaname = 'pgcolumnar' AND relname = 'options';")"
opt_idx="${opt%% *}"
opt_seq="${opt##* }"
echo "-- options idx_scan=$opt_idx seq_scan=$opt_seq"
if [ "$opt_idx" -ge 1 ]; then
opt_idx_ok=1
else
opt_idx_ok=$opt_idx
fi
check_num "planning probed pgcolumnar.options through options_pkey" \
"$opt_idx_ok" "1"
check_num "planning did not sequentially scan pgcolumnar.options" \
"$opt_seq" "0"

prj="$(q "SELECT coalesce(idx_scan,0)::text || ' ' || coalesce(seq_scan,0)::text
FROM pg_stat_all_tables
WHERE schemaname = 'pgcolumnar' AND relname = 'projection';")"
prj_idx="${prj%% *}"
prj_seq="${prj##* }"
echo "-- projection idx_scan=$prj_idx seq_scan=$prj_seq"
if [ "$prj_idx" -ge 1 ]; then
prj_idx_ok=1
else
prj_idx_ok=$prj_idx
fi
check_num "planning probed pgcolumnar.projection through projection_pkey" \
"$prj_idx_ok" "1"
check_num "planning did not sequentially scan pgcolumnar.projection" \
"$prj_seq" "0"

pgc_summary
6 changes: 6 additions & 0 deletions test/check_ledger.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ base_scan_io base_scan_io premise: the later plan is still a base columnar scan
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 -
catalog_plan_index catalog_plan_index planning did not sequentially scan pgcolumnar.options 15;16;17;18;19 2026-09-22 InvalidOid on the planner options and projection scans
catalog_plan_index catalog_plan_index planning did not sequentially scan pgcolumnar.projection 15;16;17;18;19 2026-09-22 InvalidOid on the planner options and projection scans
catalog_plan_index catalog_plan_index planning probed pgcolumnar.options through options_pkey 15;16;17;18;19 2026-09-22 InvalidOid on the planner options and projection scans
catalog_plan_index catalog_plan_index planning probed pgcolumnar.projection through projection_pkey 15;16;17;18;19 2026-09-22 InvalidOid on the planner options and projection scans
catalog_plan_index catalog_plan_index premise: the filtered scan returned every row 15;16;17;18;19 never -
catalog_plan_index catalog_plan_index premise: the measured table holds its rows 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
17 changes: 16 additions & 1 deletion test/check_ledger_budget.txt
Original file line number Diff line number Diff line change
Expand Up @@ -211,4 +211,19 @@ suites_not_covered 249
# sat, so neither the branch's previous value nor main's is the composed one.
# 1499 -> 1506 for #1155's own nine rows, re-counted after this branch was
# rebuilt on main rather than added to either side.
checks_never_observed_red 1506
# REBUILT ON MAIN CARRYING #1155, and the number is neither side's. This branch
# derived 1501 against the previous main; main now holds 1506. 1501 is not 1506
# minus anything and 1506 is not 1501 plus anything -- each was correct for a tree
# that no longer exists. Re-derived by COUNTING on this tree:
#
# awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l -> 1508
#
# with the premise printed beside it, because a pattern that matches nothing
# counts 0 and reads like a clean answer: 1541 rows total, 33 not `never`.
#
# suites_not_covered does NOT move. catalog_plan_index is a new registered suite
# that arrives WITH ledger rows, so it raises the registered count and the covered
# count by one each and leaves the difference alone. Measured on both trees rather
# than argued: not-covered identical on main and on this tree, ledger-only suites 0,
# and registered == covered + not-covered on each.
checks_never_observed_red 1508
15 changes: 15 additions & 0 deletions test/pytest/TESTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ behaviour, the source of that number is named.
- [77. test_projection_parallel.py: a covering projection can be a parallel scan](#77-test_projection_parallelpy-a-covering-projection-can-be-a-parallel-scan)
- [78. test_ttl_expire.py: the one function that deletes rows, tested twice](#78-test_ttl_expirepy-the-one-function-that-deletes-rows-tested-twice)
- [79. test_projection_scan_io.py: a covering projection is not priced from the base table's pages](#79-test_projection_scan_iopy-a-covering-projection-is-not-priced-from-the-base-tables-pages)
- [80. test_catalog_plan_index.py: planning uses the options and projection indexes](#80-test_catalog_plan_indexpy-planning-uses-the-options-and-projection-indexes)

## 1. How to read a test in here

Expand Down Expand Up @@ -6144,3 +6145,17 @@ this file uses `pciot` / `onck` / 36000 rows. Assertion names match.
| test | what it holds |
| --- | --- |
| `test_projection_scan_io` | the table and covering projection exist; the plan uses that projection; the covering scan has a positive run cost; the projection occupies a minority of the relation; the covering run is not priced from the base table's pages; a covering path whose storage cannot be found is not priced as one page |

## 80. test_catalog_plan_index.py: planning uses the options and projection indexes

Port of `catalog_plan_index.sh`. A plan that asks what one columnar table was written with, and whether it has a projection, sequentially scanned `pgcolumnar.options` and `pgcolumnar.projection`. Both catalogs already have a primary key on the column the scan key names.

The measured statement runs on a second connection. The shell suite gets that by using a fresh `psql` for every statement. This file holds one connection for the writes and opens another for the scan, for the same reason `test_native_delete_vector_index.py` does: the session that just wrote is a different path.

`pg_stat_reset()` is database-wide. The corpus runs serially within a worker.

### Every test

| test | what it holds |
| --- | --- |
| `test_catalog_plan_index` | the measured table's row count, that the filtered scan returned every row, and that `options` and `projection` were probed by index with `seq_scan` still 0 |
11 changes: 10 additions & 1 deletion test/pytest/expected_tests.txt
Original file line number Diff line number Diff line change
Expand Up @@ -500,4 +500,13 @@ guard_tests 403
# after a THIRD rebase. This value has read 464, 468, 469 and now 476 as main
# went 463, 467, 468 and 475 under it; every one was correct for the main of
# its hour and none survived the next merge. `476 tests collected`.
cluster_tests 476
# cluster_tests moved 476 -> 477 for #1198's one new cluster file,
# test_catalog_plan_index.py. RE-DERIVED BY COLLECTION on the composed tree, not
# by adding one to either side. Both this branch and main carried 476 and the
# merge was silent, because the branch's 476 was measured against the previous
# main and main's 476 was measured without this branch:
#
# main alone guard=403 cluster=476 (50 cluster files)
# composed guard=403 cluster=477 (51 cluster files)
#
cluster_tests 477
Loading
Loading