fix: size the options arrays from the catalog, not a stale constant (#834)

[jdatcmd]

Closes #834.

pgcolumnar.options has nine columns and src/columnar_metadata.c:49 said
seven, so PgColumnarRenameDeclaredSortByColumn handed heap_modify_tuple
three stack arrays that were two slots short of what it iterates.

ALTER TABLE ... RENAME COLUMN on a columnar table with a declared sort_by is
enough to reach it. On the sanitizer build the backend aborts and the postmaster
takes every other session into crash recovery.

How it was found

A full-matrix sanitizer sweep. The shipped gate runs 24 of the 233 registered
suites under ASAN; this ran all of them. One suite produced a genuine report:

logs examined              232
with a genuine report        1   (sorted_mark_rename)
clean                      231

sorted_mark_rename performs exactly this rename and passes on an ordinary
build, on both PG18 and PG19, because the overflow lands in adjacent stack slots
and has no symptom there.

The fix

Natts_options becomes 9, with a comment saying why the number is what it is.

The test

test/catalog_natts.sh checks the invariant rather than waiting for the crash,
so it runs on every major rather than only where a sanitizer does. For each
Natts_* constant it compares the value against the width the server reports
for the catalog it addresses. The server is what heap_modify_tuple iterates,
and an upgrade script can widen a table without changing any CREATE TABLE text.

It asserts its own premises before sweeping: that the constants exist, that they
still size arrays ([Natts_...]), and that heap_modify_tuple is still the
consumer. Without those, the suite would keep passing after the constants stopped
being load-bearing, which is the failure mode a text-grep check invites.

Eleven catalogs audited; ten were already correct.

Removal proof

With Natts_options put back to 7, and the mutation asserted applied:

	fixed	mutated
installed .so	9496bf09f2643178	cb74c148fa3d55cc
catalog_natts	PASSED	FAIL got [7] want [9]
sorted_mark_rename under ASAN	rc=0, 0 reports	rc=1, 2 reports, stack-buffer-overflow heaptuple.c:1240

The .so differed between the runs, so the before run was not the fixed binary.

Verification

• catalog_natts RED before the fix for the stated reason, GREEN after.
• The minimal reproducer no longer aborts; ALTER TABLE returns normally.
• sorted_mark_rename under the sanitizer: was rc=1 with 2 reports, now rc=0
  with none.
• harness_selftest: 168 checks, PASSED, so the new suite is registered in
  sorted position and the array still parses the way bash reads it.
• shellcheck -S error: clean.

Not claimed

Pre-fix, replace[7] and replace[8] were uninitialised stack, so replacing
ttl_column or ttl_interval with a garbage Datum was possible in principle. I
tried to demonstrate that on a non-sanitizer build and could not: both fields
survived the rename intact. The defect here is the out-of-bounds read and the
abort it causes, not a demonstrated corruption.

Follow-up, deliberately not in this PR

sorted_mark_rename is still outside the sanitizer subset. This PR makes the
class visible to the ordinary matrix, which is the stronger fix; widening the
sanitizer subset is a separate change.

[OffgridwithJD]

Verified by running, not by reading. Three arms at 2296ae53, base 808cd46, plus
independent checks of the two claims the PR rests on.

The fix is pinned by the test — proved, not assumed

arm	result
A parent src/ + this PR's test/catalog_natts.sh	RED — Natts_options matches the width of pgcolumnar.options: got [7] want [9]
B head as submitted	GREEN — 16 of 16
C head with src/ reverted to 808cd46	RED, same check

Arm C's tree is byte-identical to the parent under git diff --quiet -- src/, so the
red is the fix's absence and nothing else. Arm A is assertion-red: exactly one check
fired, not a suite falling over.

The suite is thorough, and I checked that rather than trusting the header

It derives its list dynamically by grepping columnar_metadata.c, so it is not a
second copy of the constant list that can drift from the first. My first pass grepped
it for literal names, found only Natts_options, and would have filed a false gap;
reading the body is what corrected that.

Its own arithmetic, from the arm B log:

constants defined: 11   array declarations using them: 26   heap_modify_tuple calls: 4
constants swept: 11 = checked 11 + unmapped 0
checks run: 16          = 4 premises + 11 constants + 1 coverage premise

That decomposes exactly, so the sweep covered every constant rather than a subset.
Reading widths from the live server instead of the shipped .sql is the right call:
the server is what heap_modify_tuple will actually iterate.

Two things I checked independently

The fix is complete. Every Natts_* now equals the highest Anum_* for its
table — options 9, projection_declaration 4, projection 6, native_storage 9,
row_group 7, column_chunk 8, zone_map 9, bloom 4, load_fingerprint 4,
free_space 4, delete_vector 4. options was the only one that had drifted.

The bug class is confined to this file. No Natts_* is defined anywhere else.
Four files call heap_form_tuple/heap_modify_tuple; the two outside
columnar_metadata.c that size arrays are both safe, for different reasons worth
recording:

• columnar_parquet_reader.c:3862 — Datum values[4] sits fifteen lines below a
  retdesc built with exactly four TupleDescInitEntry calls in the same
  function. Same shape of coupling, but both halves are visible at once, so it
  cannot drift the way a constant in another part of the file can.
• columnar_arrow.c:1627 — palloc(sizeof(Datum) * n->structDesc->natts). Sized
  from the descriptor itself, so the class is impossible there by construction.

That second one is the pattern this whole bug class needs, and I mention it only as
an observation: I am not asking for it in this PR. A stack array with a
compile-time size is cheaper than a palloc, and now that the invariant is pinned by a
matrix suite the constant is defensible.

One finding: the detection gap you named is still open

Your own comment explains why the matrix never saw this — "sorted_mark_rename was
not in the sanitizer subset". That is still true at this head, and it is structural
rather than an oversight.

• sorted_mark_rename appears 0 times in test/run_san.sh.
• Its line 49 is ALTER TABLE $1 RENAME COLUMN a TO tmp_swap; — literally #834's
  reproducer.
• The subset's 24 suites are scoped, by nightly.yml's own comment, to
  "write/read/encode/import". Catalog DDL is not in that scope at all.
• selftest/130 cannot require it either: its rule is "every suite that drives
  debug_encoding_selftest is in the subset", and sorted_mark_rename does not
  drive it. So nothing in the tree can currently notice the omission.

So #834 is evidence that the subset's scope is wrong, not that a name was
forgotten. Your invariant check is the better answer for this drift class — it is a
matrix check on every major rather than a sanitizer-only one, exactly as your header
argues. But the sanitizer stays blind to the rename path for any other memory error
there, and that is now a known blind spot rather than an unknown one.

Not a blocker for this PR. Worth its own issue, and I will file one if you would
rather not.

Gate

arm	result
preflight PG 15/16/17/18/19	built 5 of 5, 0 warnings
matrix PG18	232 ran, 2 skipped, ALL PASSED
matrix PG19	234 ran, 0 skipped, ALL PASSED
catalog_natts	PASS on both arms
harness_selftest	PASS on both arms
failing suites	none

The suite counts are the registration check: main at 808cd46 gave 231 and 233 on
the same box today, so +1 on each arm is catalog_natts actually joining the matrix
rather than merely existing in the tree.

Registration is correct and alphabetical (cancel_decode / catalog_natts /
column_projection) — checked because an unregistered suite is a gate that never
runs.