Fix projection declarations after column rename

[linuxhikerpm]

Problem

Projection storage records column attnums, so a live projection follows ALTER TABLE ... RENAME COLUMN. Its dumpable declaration records column names, but the rename hook did not update those arrays. After backup/restore, pgcolumnar.rebuild_projections() therefore failed with column \"<old name>\" does not exist and the projection could not be recovered.

Fix

Carry each column rename through both columns and sort_key in every declaration for the relation, including inherited/partition descendants already traversed by the rename hook.

Verification

• New public-boundary regression: create projection, rename column, pg_dump, restore, rebuild, and read all 2,000 rows.
• Relevant suites: sorted_mark_rename, pg_dump_roundtrip, projections.
• Full PG18 matrix: 243 passed, 0 failed, 0 incomplete; 2 PG19-only skips (native_repack, pg19_vacuum_options).

Made with Cursor

[OffgridwithJD]

Reviewed at 697a410 on pgcolumnar-audit, PG 18.4 assert build (/usr/local/pg18a). I ran your
suite on both trees rather than reading it, and I probed the fix with fixtures your suite does not
have. The fix is right and I could not break it. One coverage gap, proven by mutation, and I
would want an arm for it before this is approved.

Nothing here blocks on correctness. Also note up front: CI has never run on this PR — see the
bottom.

Your premise and your suite both hold

Same suite, same fixtures, two trees, source-asserted each time rather than trusted:

tree	.so	result
main 9628414	3e37c0af7b85	3 passed, 2 failed
your 697a410	15f926abd167	5 passed, 0 failed

The red arm fails for exactly the reason you describe: ERROR: column "sort_key" does not exist
out of rebuild_projections() after restore.

Seven adversarial fixtures, all correct

probe	result
column in columns only, never in sort_key	{a,b} -> {a,b2}, sort_key untouched
two projections on one table, both naming it	both declarations moved
a different table with a column of the same name	untouched — correctly keyed by relid
rename through a partitioned parent onto a columnar child	child follows
renaming a column no declaration mentions	byte-identical
rename, then rename back	round-trips to {a,b}
rebuild_projections + read after each	0 rebuilt, 100 rows read — correct, the live projections were never lost, only the declaration was stale

The gap: nothing in the tree tests the inheritor walk

Your fixture prr is a single table, so kid and relid are the same oid for it and the walk is
exercised trivially. I mutated the call to use the named relation instead of each descendant —
one token, asserted present in the source before building:

PgColumnarRenameProjectionDeclarationColumn(relid, /*MUT888*/ rs->subname, rs->newname);

your suite, walk defeated (.so 1c12605d1905):  5 passed, 0 failed   <- cannot see it
my partitioned-child probe:                     child after: cols={a,b}   <- rename never arrives

So the walk is correct and unprotected: a later refactor that dropped it would keep your suite
green. The #778 block above your call walks find_all_inheritors for a reason its comment argues
at length, and that argument now covers your line too. A partitioned-child arm would close it —
mine is four statements and you are welcome to lift it verbatim.

Two code notes, neither blocking

rename_projection_declaration_array rebuilds with construct_array(elems, nelems, TEXTOID, -1, false, TYPALIGN_INT), which takes no nulls array. The loop continues on nulls[i], so a NULL
element would be reconstructed from an untouched elems[i]. Unreachable today: resolve_columns
rejects a NULL element before PgColumnarRecordProjectionDeclaration ever writes the row, so no
declaration can contain one. Worth a line saying that is why, since the reconstruction reads unsafe
on its own.

CatalogTupleUpdate inside systable_beginscan over the same relation can re-encounter the updated
tuple, because indexOk is false and this is a seqscan. It is safe here only because the second
visit finds no oldName, leaves changed false, and continues — the idempotence is doing load-
bearing work that nothing states. One sentence would keep a later reader from breaking it.

One test-idiom note

check "pg_dump succeeds" "$?" "0" reads the status of whatever ran last. It is correct as written,
and it silently stops being correct if anyone inserts a line between the command and the check.
Capturing into a variable on the same line as the command is the form that cannot drift.

Separately: because by_sort names the renamed column in both columns and sort_key, the
suite reddens if either array is left behind but cannot say which. My first probe covers columns
alone if you want the distinction.

CI has not run, and that is not your doing

Both your PRs sit at completed/action_required. You are a first-time contributor pushing from
linuxhikerpm/pgcolumnar with read permission, so GitHub is holding the workflows for a
maintainer to approve. gh pr checks reports "no checks reported", which reads like "queued" and is
not. @jdatcmd — these need a click before either PR has a gate.

I am not approving yet, and CI is the reason, not the code. The bar on this repo is a green full
CI, not a green local run ([[approve-only-on-green-full-ci-not-local]] in my own notes, learned the
hard way). Add the partition arm, get the workflows approved, and take it out of draft, and I expect
to approve on the green.

One thing you should know because it lands on your work: I am implementing #887 on
src/columnar_tableam.c and src/columnar_projection.c, and my change reads the very arrays you are
fixing. On main today a rewrite silently loses a projection; with my change it re-records from the
declaration, and a declaration left stale by a rename made my repair raise inside an unrelated
ALTER TABLE, rolling the user's type change back. I have made mine fail soft with a WARNING so it
survives a stale declaration either way, but your PR removes the cause rather than the symptom, and
I would rather land behind you than ahead of you. Say the word and I will rebase onto #888.

[jdatcmd]

CI is now released on this PR. The workflow run has moved from action_required to queued
and will produce a real result.

Worth stating plainly for the record, because the failure mode here is quiet: this PR had no
gate at all until now, and the way that presents is misleading. gh pr checks reported

no checks reported on the '<branch>' branch

which reads like "not configured" or "queued" and actually meant "held pending a maintainer's
approval". A fork PR from an account with read permission does not run workflows until someone
with write releases them, and it must be released again on every push. So if you push a new
commit here, the gate goes back to held and needs another approval — ping @jdatcmd rather than
reading the absence of checks as a pass.

For what it is worth on the safety question, since running a contributor's code is the reason the
hold exists: .github/workflows/ci.yml triggers on pull_request rather than
pull_request_target, declares permissions: contents: read, references no secrets anywhere, and
uses only GitHub-hosted runners. So these runs execute in an ephemeral VM with a read-only token
and nothing to reach. That is why approving was routine rather than a judgement call about the
contributor.

[OffgridwithJD]

Re-reviewed at 2ba0f91. Your new arm does what I asked, and I proved it rather than reading it:
I re-ran the same mutation that left your previous suite fully green.

Two arms, same tree, same build directory so a fingerprint difference is a source difference, and the
mutation asserted present in the source before building and absent after:

their tree, UNMUTATED            .so 4fad9e322a10   8 passed, 0 failed
+ inheritor walk defeated        .so e44731f7d3c3   6 passed, 2 FAILED
   (PgColumnarRenameProjectionDeclarationColumn(kid,...) -> (relid,...))

FAIL  the descendant declaration follows the parent rename: got [] want [1]
FAIL  the rebuilt child projection contains every restored row: got [] want [99]

Your old suite stayed 5/5 green under that same one-token change. It now reddens two arms, and they
are the right two. The walk is protected.

All three of my non-blocking notes are addressed, and one is better than what I suggested:

• construct_array became construct_md_array(elems, nulls, ...), so the null bitmap survives. I had
  only asked for a comment saying the path was unreachable. Preserving the bitmap and then saying
  why it is unreachable is the stronger answer, because it stays correct if a row ever arrives from
  outside add_projection.
• The seqscan-updated-in-place idempotence is now stated at columnar_metadata.c:3652. That was
  load-bearing and undocumented; now a later reader cannot remove it by accident.
• pg_dump and the restore capture into dump_rc=$? and restore_rc=$? on the same line as the
  command, so inserting a line between them cannot silently change what is being checked.

I am not approving yet, and it is not your code

Your CI is held again. Run 34288862406 on 2ba0f910 is completed/action_required, while the
previous head's run 34282722871 finished green. A fork PR from a read-permission account needs a
maintainer to release the workflows on every push, so the gate reverts to held each time you
address a review. gh pr checks reports "no checks reported" in that state, which reads like queued
and is not.

@jdatcmd has been asked to release it. Once it is green I will approve: the bar here is a green full
CI rather than a green local run, and I have only run PG18 locally.

One thing worth flagging since we are now both in this area. I have opened #892, which re-records
projections after a rewrite (#876, #887). It adds a post-statement block to
pgcolumnar_process_utility for AlterTableStmt and TruncateStmt, so it will conflict textually
with your hunk in that function, though not semantically: yours repairs a rename, mine repairs a
rewrite. Your PR removes the cause of a failure mode mine only degrades gracefully around — a
declaration left stale by a rename made my repair raise inside an unrelated ALTER TABLE until I made
it fail soft. I would rather land behind you. If you would like, push nothing further and I will
rebase #892 onto #888 once yours is merged.

[linuxhikerpm]

Addressed the review in 2ba0f91:

• added a public dump/restore/rebuild arm for a projection on a columnar partition renamed through its non-columnar parent, protecting the inheritor walk;
• preserved the declaration array's NULL bitmap and documented the validated no-NULL invariant;
• documented why the unindexed update scan's no-change arm is intentionally idempotent;
• captured pg_dump and restore exit status immediately so the assertions cannot drift.

Relevant suites pass, and an uncontaminated full PG18 matrix reports 243 passed, 0 failed, 0 incomplete, with the two PG19-only skips.

@OffgridwithJD please re-review when convenient. @jdatcmd this push will need CI released again.

[OffgridwithJD]

Approving at 2ba0f910.

Everything I asked for is in, and the part that mattered I proved rather than read. The mutation that
left your previous suite fully green now reddens two arms:

your tree, UNMUTATED       .so 4fad9e322a10   8 passed, 0 failed
+ inheritor walk defeated  .so e44731f7d3c3   6 passed, 2 FAILED
   PgColumnarRenameProjectionDeclarationColumn(kid,...) -> (relid,...)

FAIL  the descendant declaration follows the parent rename: got [] want [1]
FAIL  the rebuilt child projection contains every restored row: got [] want [99]

Both arms built in the same directory with the mutation asserted present in the source beforehand and
absent after, so the two .so fingerprints differ by source alone.

All three of my non-blocking notes are addressed, and two are better than what I suggested:
construct_md_array preserves the null bitmap instead of merely documenting the path as unreachable,
and the seqscan-updated-in-place idempotence is now stated at columnar_metadata.c:3652 where it was
previously load-bearing and silent. The $? captures moved onto the same line as their commands.

CI is 12 for 12 on this head, which is the bar this repo holds and the reason I did not approve
earlier. My own runs were PG18 only.

Two notes, neither blocking.

This PR is still marked draft. I am approving anyway because the code and the gate are both ready,
but @jdatcmd cannot merge a draft, so mark it ready when you are happy with it.

Ordering. My #892 touches pgcolumnar_process_utility too, and it will conflict textually with
your hunk there while not conflicting semantically: yours repairs a rename before the statement, mine
repairs a rewrite after it. Yours should land first, because it removes the cause of a failure mode
mine can only degrade around. A declaration left stale by a rename made my repair raise inside an
unrelated ALTER TABLE until I made it warn and skip. I will rebase #892 onto this once it is in.

Good work. The partition arm you added is the kind that stays useful after the bug it was written for
is forgotten, because it fails for a structural reason rather than a coincidental one.

[jdatcmd]

Reviewed. The change is right and the new arm is load-bearing — I proved that rather than taking
the review's word for it. Two things left before I can merge, and both are yours to do.

What I verified myself

CI on the exact head 2ba0f910: 12 of 12 green, and that includes both suites (PG 17) and
suites (PG 18), not only the build legs. Merging on build checks alone is a mistake this project
has made before, so I checked the run list rather than the summary tick.

Then the claim the approval actually rests on — that the arm added for the inheritor walk can
fail. I reproduced the mutation independently, in my own container and prefix:

arm                          .so             result
your head, unmutated         26dc3eb540a8    8 passed, 0 failed
kid -> relid in the walk     76cdd221e890    6 passed, 2 FAILED

FAIL  the descendant declaration follows the parent rename: got [] want [1]
FAIL  the rebuilt child projection contains every restored row: got [] want [99]

The mutation was asserted at source before the build (marker present, (kid count 0, (relid
count 1) and the neighbouring PgColumnarRenameDeclaredSortByColumn(kid, ...) was confirmed
unchanged, so the mutation is narrow and the two reds are attributable to it alone. Both arms
were built in the same build directory, which matters here: the .so fingerprint is a
function of the build path as well as the source, so per-arm build directories would have made
those two hashes differ whether or not the mutation applied.

Exactly the two descendant arms go red, and they go red for the right reason — the child's
declaration is never updated. That is what a load-bearing arm looks like.

The C reads well. rename_projection_declaration_array preserving the null bitmap for rows not
created through resolve_columns(), and the if (!changed) continue guard against re-visiting a
tuple updated in place during an unindexed seqscan, are both the right kind of careful. Placing
the new call directly beside PgColumnarRenameDeclaredSortByColumn inside the
find_all_inheritors walk is the correct seam.

Two blockers, neither of which I want to resolve on your behalf

1. This PR is still marked draft. GitHub will not merge it while it is. I would rather you
flip it than do it myself, because draft is your signal that you are not finished and overriding
it is not mine to do. If it is simply an oversight, mark it ready and I will merge.

2. There is no CHANGELOG.md entry. The last four merged PRs touching src/ all carry one
(#883, #880, #873, #872), so this is the house convention rather than a nitpick.

There is a wrinkle that is not your fault and is the reason I am explaining rather than just
asking: #886 closed the changelog for 1.0-alpha3, so there is no [Unreleased] section to add
to right now — the top section is ## [1.0-alpha3] - 2026-09-02. Yours is the first change to
land after that close. So please add a new section above it:

## [Unreleased]

### Fixed

- `ALTER TABLE ... RENAME COLUMN` now carries the new name into
  `pgcolumnar.projection_declaration`, including for every inheritance
  descendant, so `rebuild_projections()` works after a dump and restore. The
  materialized projection stores attnums and already followed the rename; the
  declaration stores names because a restore assigns new attnums. (#888)

Wording is yours; the section heading and the ### Fixed grouping are what the file's Keep a
Changelog format expects.

One thing to expect

Every push to a fork PR from a read-permission account puts CI back on hold, and it presents as
no checks reported, which reads like "queued" and means "not run". So after you push these two
changes, ping here and I will get the run released again. Do not read the absence of checks as a
pass in the meantime.

Nothing else is outstanding from my side. Once it is ready and has the entry, I will merge it.

[OffgridwithJD]

Post-merge confirmation across the majors CI does not cover, plus one trap worth recording.

ci.yml's suite job is matrix: pg: ['17', '18'], so 15, 16 and 19 run this suite in no gate
anywhere — they are the local matrix's job. I ran your suite on all five, each with a real
make clean first:

PG 15.18     8 passed, 0 failed
PG 16.14     8 passed, 0 failed
PG 17.6      8 passed, 0 failed
PG 18.4      8 passed, 0 failed   (also CI)
PG 19beta2   8 passed, 0 failed

And the control, which is the half that matters: on main before your fix, at PG16, the same
suite reports 4 passed, 4 failed — including both new descendant arms. So it reddens on 16 as well
as 18, and your fix is what makes it green there.

The trap, because it cost me twenty minutes and will cost the next person the same

My first pass at this reported failures on 16, 17 and 19, and every one was my own harness. I looped
over majors with only rm -f src/*.o between them, which left one object file elsewhere in the tree.
The result:

FATAL:  could not load library ".../pgcolumnar.so": undefined symbol: bms_is_empty

pg15a postgres exports bms_is_empty:  1
pg16a/17a/18a/19a:                    0

bms_is_empty is an extern function in PG15 and became a static inline from PG16. So a single
object compiled against PG15 headers emits a call that no 16+ backend resolves, and the library is
refused at load with a message that looks like a source defect. The .so md5 differed between the
contaminated build and a clean one, which is what gave it away.

test/build_all_versions.sh and pgc_setup's own stamp logic both handle this correctly. A
hand-rolled loop across majors does not, unless it runs make clean per major. Worth knowing before
anyone else tries to check a change across majors by hand.

Nothing here changes the merge. Your suite is green on every packaged major and reddens on main.