Skip to content

feat: a per-column extent the planner reads in one lookup, recomputed whenever groups go (#1304 option 5) - #1325

Merged
jdatcmd merged 13 commits into
commandprompt:mainfrom
OffgridwithJD:feat/1304-column-extent
Oct 13, 2026
Merged

jdatcmd merged 13 commits into
commandprompt:mainfrom
OffgridwithJD:feat/1304-column-extent

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Option 5 from #1304, for #1292 phase 2. You accepted the design on #1304.

What it does

pgcolumnar.column_extent is a catalog of rows (storage_id, column_index, complete, minimum, maximum). Rows are inserted and deleted, never updated. The planner's bound for a bucketed GROUP BY date_trunc(...) / date_bin(...) now comes from one index lookup instead of every row group's zone map. pgcolumnar.column_extent(rel, col) shows that bound to anyone who can SELECT the table.

  • Writers. Each transaction folds the whole-chunk zone maps it flushes, in memory, using the column type's own comparison and collation. At pre-commit (PRE_PREPARE for 2PC) it inserts one complete = false row. Its groups and its row commit or abort together.
  • Complete rows. These come only from a recompute: the fold of every zone map one snapshot sees, which then replaces the rows that snapshot sees. A recompute refuses when a group lacks a zone map for the column, or has values without a min/max.
  • Shrinks are computed, never guessed. PgColumnarRetireGroup marks the storage, and pre-commit recomputes it under a blocking lock. That covers expire, compact_rewrite, VACUUM, compact and the reclusters.
  • Merge. With more than 8 rows and a complete one visible, the committing writer folds the rows themselves. It never reads zone maps for this.
  • Readers. A reader trusts the rows only when a complete one is visible, and then the fold is a true bound. The reader's own transaction adds its pending fold, since its groups are visible to it before its row is written. Otherwise it falls back to the zone maps.

Soundness: the thing you said you'd look at hardest

A stale extent may only be too wide, never too narrow.

path how the extent stays a true bound arm
expire marks the storage; recomputed before commit "after expire the extent is recomputed to the data that is left" (and it moved)
compact_rewrite the same "after compact_rewrite the extent is the live rows'"
VACUUM retiring a fully deleted group the same "after VACUUM retires a fully deleted group the extent is the live rows'"
rolled-back savepoint that flushed the fold keeps its values, so the extent is wider containment arm
aborted writer its row aborts with its groups "a writer that rolls back leaves the extent as the data has it"
2PC row written at PRE_PREPARE "a prepared transaction's groups are inside the extent after COMMIT PREPARED"
concurrent loaders (parallel_copy) one row each, never an in-place update "the extent covers what they loaded"
the reader's own transaction the bound adds its pending fold found while building this: 35 estimated groups against the zone maps' 5000 before the fix; red first in both twins

A deleted row that is still stored keeps the extent wider than the live data until its group is retired or rewritten. That's the safe direction, and every equality arm first removes such rows.

Evidence

  • column_extent.sh and test_column_extent.py (36 checks and 7 tests) pass on PG15–19, on this rebased tree.
  • 19 mutations; 18 are red on their arms. The survivor is equivalent: the existence check reading the catalog snapshot after the CommandCounterIncrement that refreshes both.
  • Ledger: 36 rows, 29 dated, no dated premise. The census is 1739 → 1746, rows 2192, and the gate gives rc=0.
  • On PG17, prepared_writes 11/11, native_groupagg 81 + 1 skip, entry_point_privilege 31/31, server_file_privilege 48/48 and native_upgrade_converge 21/21, all on this rebased tree. native_groupagg's one-scan arm, in both twins, now deletes the extent rows first so it still measures the fallback. Its pytest twin measures on a fresh connection, because pg_stat_get_xact_numscans reports a backend's pending stats.
  • cluster_tests 517 → 524, by collection. TESTS.md is section 102. The reference and CHANGELOG are updated.

#1310's PR is stacked on this one.

🤖 Generated with Claude Code

jdatcmd and others added 10 commits October 13, 2026 12:50
… 5), red

Against the design on commandprompt#1304. The oracle is the data's own min() and max();
equality only after deleted rows are retired or rewritten, containment for
the one case that may legitimately be wider (a rolled-back savepoint that
flushed a group).

Red on PG17 (main at 024e737): 6 passed + 25 failed. The function does not
exist (42883) and planning still scans zone_map once. The six that pass hold
on main by design: the fixture premise, the fallback and its estimate, two
concurrent writers committing, expire and compact_rewrite doing their jobs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ommandprompt#1304 option 5)

pgcolumnar.column_extent holds rows of (storage, column, complete, minimum,
maximum), inserted and deleted, never updated in place:

- each transaction folds the whole-chunk zone maps it flushes (the type's
  comparison, the column's collation) and inserts one row at pre-commit, or
  pre-prepare; its groups and its row commit together, and two writers never
  touch one tuple;
- a complete row comes only from a recompute over every zone map one snapshot
  sees, which replaces the rows that snapshot sees; it refuses when a group has
  no zone map for the column or values without an ordering;
- PgColumnarRetireGroup marks the storage and it is recomputed before commit,
  so expire, compact_rewrite, VACUUM and the reclusters shrink the extent by
  recomputing it;
- a writer recomputes when it finds no complete row and merges the rows when
  there are more than eight (a fold of those rows only).

The planner's time range reads the extent when a complete row is visible and
otherwise the zone maps. pgcolumnar.column_extent(rel, col) shows it, to a role
that can SELECT the table.

Found while making the suite green: pre-commit has no active snapshot (an
assert-build TRAP), and the catalog snapshot can predate a storage row this
transaction inserted, which made the existence check skip every table created
and written in one transaction.

column_extent.sh 33/33 on PG17, with two arms added since the red commit
(PREPARE TRANSACTION, and a table created in a rolled-back savepoint).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…roupagg measures the fallback (commandprompt#1304 option 5)

- The concurrency arm now uses parallel_copy. Ordinary writers into one table
  are serialized by the storage-row lock (class 102) from first flush to commit,
  so the old two-writer arm never overlapped them. parallel_copy's loaders flush
  into one storage at once, each through PREPARE.
- native_groupagg's one-scan arm measures the zone-map fallback, so it removes
  the fixture's extent rows first; with a complete extent the plan scans zone_map
  not at all. The pytest twin measures on a fresh connection: the xact scan
  counters are a backend's pending stats, flushed about once a second rather than
  at transaction end, and in the fixture's connection they still held the
  INSERT's own zone_map reads (the extent's first recompute, at its commit).
- test_column_extent.py: the independent twin. Its COMMIT PREPARED runs in another
  session; in the preparing one it crashes the backend on main (to be filed).
- column_extent.sh is executable (harness_selftest).

PG17: column_extent 33/33, native_groupagg 81 + 1 skip, harness_selftest 1223.
PG16: test_column_extent.py + test_native_groupagg.py 122 + 1 unrunnable (no ICU
on this build).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a recompute (commandprompt#1304 option 5)

Forget was redundant: pre-commit already skips any entry whose storage row is
gone, so no arm could tell it was there. The no-ordering arm could not fail: a
point column never gets a writer row, so no recompute ever ran for it, and a
complete row with NULL bounds printed as "none" anyway. It now runs VACUUM over
a fully deleted group, and both twins print a NULL bound as '-'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he pair, cluster_tests 517 (commandprompt#1304 option 5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PG19 no longer reaches tuplestore_putvalues through the headers that 15-18 do;
the PG19 build failed on an implicit declaration.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tions (commandprompt#1304 option 5)

Clean: column_extent.sh and test_column_extent.py 33/33 on PG15-19. Each
mutation log merged one at a time with --mutation and --target. A nineteenth
(the existence check on the catalog snapshot) survived and is equivalent.
Census 1698 -> 1704; rows 2092; the gate agrees.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… reads, red (commandprompt#1304 option 5)

Its extent row is written at pre-commit, but its groups are visible to it
as soon as they flush. Seen red on PG16 in both twins: column_extent reads
'no', and the in-transaction estimate is 35 rows (61 in the twin) against
the zone maps' 5000 (4500), an under-estimate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mmandprompt#1304 option 5)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…'s docs (commandprompt#1304 option 5)

column_extent.sh 36/36 and test_column_extent.py 7/7 on each of PG15-19. The
two arms are dated by the run before the fix; the instrument premise arrives
never. Ledger 2095 / 1705. cluster_tests 517 -> 518 by collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at 585cc17. One blocker, one test-discipline point, and one finding on main that this PR exposes. I found no way to make the extent too narrow through the PR's own paths.

Blocker: CI cannot build

All build jobs fail. CI treats warnings as errors, and gcc reports:

c.h:1015:50: warning: ‘zlo’ may be used uninitialized [-Wmaybe-uninitialized]
c.h:1021:50: warning: ‘zhi’ may be used uninitialized [-Wmaybe-uninitialized]
##[error]compiler warnings are failures in this project

The variables are at src/columnar_vector.c around 2326–2332, passed to pgcolumnar_zonemap_time_range. The new early return through the extent path is the likely cause, but I haven't confirmed that. Initialise both. The suites were skipped, so CI has run nothing yet.

Four never-red checks that are not premises (#1236)

The ledger has seven column_extent rows never observed red. Three are labelled premise:. These four are not:

  • compact_rewrite rewrites the groups the deletes thinned
  • expire retires the old row groups with the extent in place
  • parallel_copy's loaders, flushing into one table at once, all commit
  • without the extent rows the planner falls back to one zone_map scan

The first three read as fixture facts, so rename them premise:. The fourth is a real behaviour, so it needs a mutation that reddens it, for example the fallback removed. The budget's 1746 does equal the never rows, so the arithmetic is right.

What I ran locally at 585cc17, from /tmp

  • column_extent: 36/36 on PG17 and on PG18
  • native_groupagg: 81 passed + 1 skipped on PG17, 83/83 on PG18
  • test_column_extent.py on PG17: green

What I checked by reading, not by an adversarial run

  • writer and recompute ordering under concurrency;
  • the merge running only when a complete row is visible;
  • every retire path going through PgColumnarRetireGroup;
  • the parallel-flush leader noting the extent after its workers finish;
  • 2PC at PRE_PREPARE;
  • the reader's own pending fold.

I also converted timestamp to timestamptz in America/New_York: the table was rewritten, and the extent matched exactly. I did not look at dump/restore or pg_upgrade of the new catalog.

On main, not this PR: a collation change leaves zone maps wrong

ALTER COLUMN s TYPE text COLLATE "en_US.utf8" on a column that was "C" keeps the relfilenode, so the zone maps keep C ordering. I reproduced it on main at 036d452, PG17: columnar min|max = B|m where heap says a|m, and WHERE s < 'B' returns nothing where heap returns a,b. column_extent() would print the same narrow bound, though your planner bound reads only timestamp columns. I'm filing it as its own issue; it should not hold this PR.

jdatcmd and others added 3 commits October 13, 2026 13:03
…mandprompt#1325 CI)

gcc 13/14 with LTO warned that zlo and zhi may be used uninitialized, and CI
treats warnings as failures. Every path that returns true sets both, and the
extent's emin/emax are read only when the column has a value; gcc cannot see
either through the out-parameters. Not reproduced locally: gcc 15 with the
same flags and LTO is silent, so CI's build is the evidence.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…es (commandprompt#1236)

compact_rewrite and expire retiring with the extent in place, and
parallel_copy's loaders all committing, are facts about the fixture that no
mutation of this change reddens. Renamed in both twins, from jdatcmd's review
of commandprompt#1325.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dprompt#1325 review)

Three renamed rows replaced from a five-major run of the renamed suite; the
fallback arm dated by a mutation that removes the zone-map fallback. Ledger
2192 / 1745, gate rc=0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 13, 2026
…e planner's

The same out-parameter shape gcc 13/14 with LTO reported as maybe-uninitialized
on commandprompt#1325; emin and emax are read only when hasValues is true.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

@jdatcmd Thanks. Both points are addressed at 3c71b6a.

The build (8b4da2a). zlo/zhi at the call site and the extent block's emin/emax are now initialised. I could not reproduce the warning here: gcc 15 with Ubuntu PG18's own flags, -flto=auto included, is silent. So CI is the evidence. All nine build jobs fail at 585cc17 and pass at 3c71b6a, on both arches and the PG19 source build. Every path that returns true set both values, so this was gcc failing to see through the out-parameters, not a read of garbage. #1326's expire had the same shape and is initialised there too (6e81d07).

The four never-red checks (277ac41, 3c71b6a).

  • Three are renamed premise: in both twins: compact_rewrite and expire retiring with the extent in place, and parallel_copy's loaders all committing. The old rows are removed. The renamed rows come from a five-major run of the renamed suite: 36/36 on 15–19, and the twin 7/7.
  • "without the extent rows the planner falls back to one zone_map scan" is dated by a new mutation that removes the fallback. It goes red as got [0] want [1], and the estimate check beside it goes red as rows=835 against rows=50000.
  • All six of column_extent's never rows are now premises. The census goes 1746 → 1745 over 2192 rows, and the gate gives rc=0.

#1327. Thanks for filing it separately. I'll take it once this one settles, unless you already have it. Red-first in both twins, as you described.

#1326 is restacked on 3c71b6a, and its census is restated to 1796. On PG17, ttl_expire gives 67/67, column_extent 36/36 and both twins 16/16, and the gate gives rc=0.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at 3c71b6a. The build fix initialises the four bounds; the three fixture checks are premises in both twins; the fallback arm is dated by its own mutation; the budget's 1745 equals the never rows on this head. CI 15/15. The collation finding is #1327, on main, and does not hold this PR. Merging now.

@jdatcmd
jdatcmd merged commit 11f954e into commandprompt:main Oct 13, 2026
15 checks passed
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Oct 13, 2026
…e planner's

The same out-parameter shape gcc 13/14 with LTO reported as maybe-uninitialized
on commandprompt#1325; emin and emax are read only when hasValues is true.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants