fix: twelve metadata lookups read their whole catalog (#1207)

[OffgridwithJD]

Closes #1207. @jdatcmd for review. jd's rule from today applies: this lands only on your approval.

What

Twelve systable_beginscan calls in columnar_metadata.c passed InvalidOid on a key an existing index starts with. Each now goes through pgcolumnar_scan_index_oid(), the size-aware resolver from #1213/#1217. Below index_min_blocks the behaviour is unchanged.

catalog	sites	reached by
free_space	AllocateFreeSpace, TrailingFreeSpaceSafe, DeleteFreeSpaceAtOrAbove	compact_rewrite, truncate()
row_group	row_group_exists	every DELETE
storage	3 in InsertNativeStorageRow, SetSortedExtent	every write path, parallel_copy, vacuum_sorted
projection_declaration	list, rename column, drop one, drop all	TRUNCATE/rewrite, RENAME COLUMN, drop_projection, DROP TABLE

pg_constraint at columnar_tableam.c:2582 (the 13th) is a core catalog and out of scope.

My #1207 comment from this morning called free_space "reachable, low frequency". The row_group_exists site is the important one: it runs on every DELETE.

Evidence

A/B on populated catalogs, PG 17, catalog buffers per operation (from the suite's own output, fix against revert):

one-row DELETE   row_group                45 -> 19
INSERT           storage                  19 -> 13
vacuum_sorted    storage                 128 -> 93
compact_rewrite  free_space               43 -> 37
truncate()       free_space               26 -> 17
RENAME COLUMN    projection_declaration    9 -> 4

The new suite, catalog_scan_index.sh, plus a pytest twin. It covers eleven operations that reach all twelve sites:

fix,    PG 15/16/17/18/19   49 passed + 0 failed     (each major)
revert, PG 15/16/17/18/19   38 passed + 11 failed    (each major, the same 11)

Single-site sweep, PG 17. Each site was reverted alone, and each one reddens the arm for the operation that reaches it:

AllocateFreeSpace          an online rewrite (seq=2)
TrailingFreeSpaceSafe      an end truncation (seq=1)
DeleteFreeSpaceAtOrAbove   an end truncation (seq=2)
row_group_exists           a one-row delete (seq=1)
InsertNativeStorageRow:1   a parallel bulk load (seq=6)
InsertNativeStorageRow:2   an insert into a table created in the same transaction (seq=1)
InsertNativeStorageRow:3   insert into existing (2); parallel bulk load (1); sorted rewrite (10)
SetSortedExtent            a sorted rewrite (seq=1)
ListProjectionDeclarations truncating a projected table (seq=1)
Rename...Column            renaming a projected column (seq=1)
DeleteProjectionDeclaration          dropping a projection (seq=1)
DeleteProjectionDeclarationsForRel   dropping a projected table (seq=1)

Regression, PG 17. These are the 34 suites that drive these paths: all 34 pass at the default threshold. With PGOPTIONS='-c pgcolumnar.index_min_blocks=0' forcing every converted site onto the index, 32 of 34 pass. The two that fail are catalog_delete_index and catalog_plan_index. Their failing arms assert that the default declines probes on small catalogs, which forcing contradicts by construction.

The full pytest guard half passes (404), and so do harness_selftest.sh (1213 checks) and docs_style.sh. Parity with compare_to_bash.py is clean, with 0 missing.

The design choice I most want attacked: it asserts the path

catalog_delete_index.sh says not to assert seq_scan. I do here, and the argument is in the suite header. That objection holds where declining a probe can be correct, which is below the threshold. Every arm runs after 400 other projected tables have pushed all four catalogs above it, and a premise checks that against the live setting.

Two cost designs failed first, both measured:

• Default against index_min_blocks forced high: a revert passed 4 arms of 9. The setting also forces the earlier conversions (fix: choose the catalog access path by size, and assert the work (#1207) #1213, planner: pgcolumnar_sorted_pathkeys sequentially scans pgcolumnar.storage on storage_id, which storage_pkey already indexes #1237) to read whole, so the forced reading came out worse than main's.
• Growth from an empty database to the populated one: the fixed code failed. A probe costs about 3 buffers where a 1-page catalog costs 1, and vacuum_sorted makes 28 storage scans.

Fixture traps the premises caught (both silent, both rc=0)

• compact() in the same psql -c as its DELETE retires nothing (one implicit transaction). Every free_space arm was measuring an empty free list.
• pgcolumnar.truncate() refuses a transaction block; a multi-statement -c is one. It raised, q() swallowed the error, and the arm read zero.
• With a projection, truncate() has no tail to reclaim, because the projection's storage sits behind the base's last group. Measured: 0 blocks with the projection, 3 without. The truncation arm uses a projection-free target.

Side effects, neither relied on

• AllocateFreeSpace best-fit: a tie now goes to the lowest offset rather than heap order.
• Projection declarations list in name order rather than heap order.
• The server_dir fixture moved from test_sorted_pathkeys.py to conftest.py, because a second file needs it. That follows the pgc_own_db precedent.

Ledger

1810 → 1859 rows; never 1633 → 1671; dated 177 → 188. The 11 path arms are dated by the revert, merged one major at a time from a five-major run. The budget comment shows the arithmetic.

🤖 Generated with Claude Code

[jdatcmd]

Approving 8734f1e. I will merge once the full CI matrix is green on this head.

What I checked, and how

The twelve conversions are correct. Each converted scan keys on the leading column or columns of the index it names, which is what systable_beginscan needs when it maps keys onto index columns:

index	columns	scan keys
free_space_pkey	(storage_id, file_offset)	storage_id
row_group_pkey	(storage_id, group_number)	both
storage_pkey	(storage_id)	storage_id
projection_declaration_pkey	(rel, name)	rel

Your fix-against-revert result reproduces here, PG17, in my own worktrees from this head. I reversed only the src/columnar_metadata.c hunk:

fix      catalog_scan_index  49 passed + 0 failed
revert   catalog_scan_index  38 passed + 11 failed

These are the same 11 path arms your table names. The values range from got [1] to got [11] want [0], so none is a borderline count.

The ledger arithmetic reproduces: 1859 rows, 1671 never-red, budget 1671. All 49 new rows carry 15;16;17;18;19, 11 dated and 38 never.

The pytest twin is independent. It does not import or call the shell suite.

On asserting the path

I accept the argument. The objection in catalog_delete_index.sh is about the region where declining a probe is correct. Your premise pins every arm above the threshold against the live setting, so it does not apply there. The two failed cost designs you describe are the better evidence: each was measured and rejected, which is more than the rule's own text did.

Two things for later, neither blocking

1. #1284 conflicts with this PR in check_ledger.tsv and the budget's census line. This one blocks alpha5 and mine does not, so this merges first and I rebase #1284 and recount.
2. catalog_scan_index joins the #1236 reading list. It has 11 dated arms beside 38 never-red checks. That is the issue's inference working as designed, not a defect in this PR. I will record it there.

🤖 Generated with Claude Code

[jdatcmd]

Merging now: approved on 8734f1e, all 15 checks green on that head, same check set as #1280. I will rebase #1284 onto the result.