Repository navigation
feat: a per-column extent the planner reads in one lookup, recomputed whenever groups go (#1304 option 5) - #1325
Conversation
… 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
left a comment
There was a problem hiding this comment.
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.pyon 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.
…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>
…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>
|
@jdatcmd Thanks. Both points are addressed at 3c71b6a. The build (8b4da2a). The four never-red checks (277ac41, 3c71b6a).
#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, |
jdatcmd
left a comment
There was a problem hiding this comment.
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.
…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>
Option 5 from #1304, for #1292 phase 2. You accepted the design on #1304.
What it does
pgcolumnar.column_extentis a catalog of rows(storage_id, column_index, complete, minimum, maximum). Rows are inserted and deleted, never updated. The planner's bound for a bucketedGROUP 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.complete = falserow. Its groups and its row commit or abort together.PgColumnarRetireGroupmarks the storage, and pre-commit recomputes it under a blocking lock. That coversexpire,compact_rewrite, VACUUM, compact and the reclusters.Soundness: the thing you said you'd look at hardest
A stale extent may only be too wide, never too narrow.
expirecompact_rewriteparallel_copy)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.shandtest_column_extent.py(36 checks and 7 tests) pass on PG15–19, on this rebased tree.prepared_writes11/11,native_groupagg81 + 1 skip,entry_point_privilege31/31,server_file_privilege48/48 andnative_upgrade_converge21/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, becausepg_stat_get_xact_numscansreports a backend's pending stats.cluster_tests517 → 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