fix: drop relid-keyed catalogs after SET ACCESS METHOD heap

[OffgridwithJD]

Summary

• pgcolumnar.options and pgcolumnar.projection_declaration are keyed by relid, not storage id. SET ACCESS METHOD heap already drops the storage-id catalogs, but it leaves those rows.
• The drop hook used to return before touching them whenever the table was no longer columnar, so converting away and then DROP left dangling-oid rows. pg_extension_config_dump emits a bare oid, and rebuild_projections() aborts on an orphan declaration — the same blast radius DROP TABLE orphans projection_declaration, poisoning rebuild_projections() and pg_dump restore #304 closed for a still-columnar DROP.
• Relid-keyed cleanup now runs on every user-table drop, not only while the AM is still pgcolumnar. A heap round-trip still keeps set_options.

Test coverage

• test/alter_am_cleanup.sh
• test/drop_cleanup.sh (unchanged, still passes)

Made with Cursor

[OffgridwithJD]

Premise and fix both verified

A/B on pg18a against 8b39053. Columnar table, set_options, SET ACCESS METHOD heap, DROP:

                                   main     #872
after set_options   options=        1        1      (premise gated: must be >=1)
after SET ACCESS METHOD heap        1        1      rows still readable: 2000 both
after DROP          options=        1        0      <-- the leak, and the fix
                    decls=          0        0

The relid-keyed row outlives the table on main. #872 removes it. Confirmed.

The round-trip control holds too: convert to heap and back, and set_options
survives (1 row remains on #872 for the surviving table; main reads 2 only because
it is still carrying the leaked row from the first table).

The cost, measured — every DROP in the database pays it

This moves the relid-keyed cleanup out of the columnar branch, so it now runs on
every user table drop, including tables that were never columnar. 200 plain
heap tables, created and dropped in one DO loop, same box, same run:

main   43 ms
#872  118 ms      2.7x, or about +375 microseconds per plain heap DROP

That is two catalog index scans (options, projection_declaration) plus a
get_namespace_oid lookup on a path that previously did nothing at all for a heap
table. It is paid forever, by every database with the extension installed, to
clean up after an event — AM conversion — that most databases never perform.

Whether 375 us per drop matters is a judgement I would leave to the maintainer.
It is invisible for ordinary DDL and visible for DROP SCHEMA CASCADE over a few
thousand tables, temp-table churn, and test suites.

The obvious cheaper fix is wrong — do not reach for it

My first instinct was: do the cleanup in the SET ACCESS METHOD path (there is an
OAT_POST_ALTER and this file already handles OAT_POST_CREATE and OAT_DROP),
so the cost lands on the rare conversion instead of on every drop.

That would break behaviour this PR deliberately preserves. The PR body states that
a heap round trip keeps set_options, and my control confirms it does. Deleting
the options at conversion time destroys exactly that. The AM-agnostic drop hook is
not an oversight; it is what "keep the options across a round trip, clean them at
drop" forces. I am recording this so the next reader does not re-propose it.

If the cost is worth attacking, the cheap parts are inside the hook rather than in
its placement: get_namespace_oid(COLUMNAR_SCHEMA_NAME, true) is resolved on every
drop and could be cached, and the two deletes could short-circuit when the
extension's own tables are empty — the common case for a database that has never
converted anything.

One thing to add to the suite

Nothing pins the cost or the AM-agnostic reach. An arm asserting that dropping a
plain heap table leaves the columnar catalogs untouched and still succeeds would
document the intent, and would catch a future version of this hook that errored on
a database where the extension's catalogs are missing or inaccessible.

Reviewed as OffgridwithJD. Not approving -- same account as the author.

[OffgridwithJD]

Blocker: this stops DROP TABLE from working in every database of the cluster that does not have the extension installed

The premise is real — I reproduced the leak and the pg_dump symptom — but the fix
is over-broad in a way nothing in the tree can see. Everything below is measured on
pgcolumnar-audit, base 8b39053 against head 571b723, PostgreSQL 18.4
(/usr/local/pg18a), one tree per prefix, distinct .so fingerprints per arm
(60023eab5bfe base, b9d21fe8d452 head).

1. The two deletes left the AM branch, so they now run where the catalogs are not there

PgColumnarDeleteOptions opens its catalog through open_columnar_table("options", …),
which resolves the schema with get_namespace_oid(COLUMNAR_SCHEMA_NAME, false)
(src/columnar_metadata.c:152). That raises where the schema is absent, and the error
aborts the DROP itself. The library is in shared_preload_libraries, so the hook is
armed in every database of the cluster, not only the one where CREATE EXTENSION
ran. Before this PR the calls sat inside rel->rd_tableam == &pgcolumnar_am_methods,
which cannot be true without the extension.

arm	base 8b39053	head 571b723
DROP TABLE a_heap in a database with no extension	rc=0	ERROR: schema "pgcolumnar" does not exist, table still present
DROP TABLE after DROP EXTENSION pgcolumnar CASCADE	rc=0	ERROR: columnar metadata table "pgcolumnar.options" does not exist
CREATE TEMP TABLE t; DROP TABLE t; no extension	rc=0	ERROR: schema "pgcolumnar" does not exist
temp table left to backend-exit cleanup, no extension	gone	still in pg_class after the session ended
BEGIN; CREATE TEMP TABLE … ON COMMIT DROP; COMMIT; no extension	rc=0	the COMMIT fails
control: DROP TABLE where the extension is installed	rc=0	rc=0
control: DROP SCHEMA … CASCADE where the extension is installed	rc=0	rc=0

The two controls matter: this is not "drops are broken", it is "drops are broken
exactly where the catalogs cannot be reached". Note the second row — after
DROP EXTENSION the schema survives but its tables do not, so the failure moves one
step down the call chain and the message changes. Gating on the schema alone would not
have caught it.

2. A columnar table living in the pgcolumnar schema now leaks — the regression of #304 this cites

The new gate skips the whole extension schema, not the extension's own tables, so a
user table there is skipped along with them.

CREATE TABLE pgcolumnar.d_tab (id int, a int) USING pgcolumnar;
SELECT pgcolumnar.set_options('pgcolumnar.d_tab', stripe_row_limit => 7000);
SELECT pgcolumnar.add_projection('pgcolumnar.d_tab','dp',ARRAY['a'],ARRAY['a']);
DROP TABLE pgcolumnar.d_tab;

  orphan options / declaration rows      base 0/0      head 1/1

Control, run first in the same script so it cannot inherit the residue: the same
table in public leaves 0/0 on both trees. My first run of this arm did not isolate
them and reported the public case as failing too; that was my instrument, and the
isolated numbers above are the ones that stand.

3. The premise is real, and the pg_dump half is stronger than the body says

Base tree, SET ACCESS METHOD heap then DROP TABLE: orphans go 0/0 → 1/1, and the
dump carries them with a relid that no longer resolves:

COPY pgcolumnar.projection_declaration (rel, name, columns, sort_key) FROM stdin;
16566	p1	{a,b}	{a}
COPY pgcolumnar.options (regclass, chunk_group_row_limit, stripe_row_limit, …) FROM stdin;
16566	\N	5000	\N	\N	\N	{id}	\N	\N

Head tree: 0/0, and both COPY blocks are empty. The bug is worth fixing.

4. One sentence in the body and the code comment is not true of this tree

rebuild_projections() aborted on the orphan declaration

It does not, and has not since #304. Its first statement is
DELETE FROM pgcolumnar.projection_declaration pd WHERE NOT EXISTS (… pg_class …),
so it self-heals before the loop can raise. Measured with an orphan present on the base
tree: returns 0, and the orphan count goes 1/1 → 1/0 — it repaired the declaration
and left the options row. test/projections.sh already asserts exactly this in three
arms ("an orphan left by an older build is present" / "the rebuild does not abort on
it" / "and it removed the orphan"). The config_dump half of the sentence is true and
proven above; please drop the rebuild_projections() half from the comment, or it
becomes the next reader's input.

5. The suite has a real removal proof and is blind to the other direction

Mutations on your head, each asserting it applied:

N1  revert the fix (both deletes back inside the AM branch)
    alter_am_cleanup.sh   4 passed + 3 failed = 7      RED, as it should be

N2  remove the extension-schema skip (the over-broad direction)
    alter_am_cleanup.sh   7 passed + 0 failed = 7      GREEN
    drop_cleanup.sh       8 passed + 0 failed = 8      GREEN

So the guard whose comment says opening options while it is being dropped "would
fail" is asserted by nothing, and neither is any part of §1 or §2. Every check in the
suite runs in $PGC_DB, the one database where CREATE EXTENSION ran. The control arm
"dropping a heap table that was never columnar leaves the catalogs alone" compares a
catalog snapshot against base. That follows, rather than being something I staged: a
DROP that raised leaves the catalogs untouched, so the snapshot still equals base
and the arm still prints PASS. It cannot distinguish "nothing was deleted" from
"nothing happened".

Three arms would close it: a DROP TABLE in a second database created without the
extension; a DROP TABLE after DROP EXTENSION; and the pgcolumnar-schema table from
§2. All three fit in this suite — psql_admin already exists in lib.sh for
CREATE DATABASE.

6. A shape that passes every arm above

Not a request to take it as written; it is here so the direction is a measurement
rather than an opinion. Gate on the catalogs being reachable, and skip the extension's
own member tables rather than the whole schema:

{
    Oid  columnarNsp = get_namespace_oid(COLUMNAR_SCHEMA_NAME, true);
    Oid  optionsOid  = OidIsValid(columnarNsp) ?
        get_relname_relid("options", columnarNsp) : InvalidOid;
    Oid  extOid      = get_extension_oid("pgcolumnar", true);

    if (OidIsValid(optionsOid) &&
        (!OidIsValid(extOid) ||
         getExtensionOfObject(RelationRelationId, objectId) != extOid))
    {
        PgColumnarDeleteOptions(objectId);
        PgColumnarDeleteProjectionDeclarationsForRel(objectId);
    }
}

(plus #include "commands/extension.h"). Built on top of your head and run on pg18a:

my §1 arms          11 passed + 0 failed = 11
my §2 arms           6 passed + 0 failed = 6
my §3 premise+dump   6 passed + 0 failed = 6
alter_am_cleanup.sh  7 passed + 0 failed = 7
drop_cleanup.sh      8 passed + 0 failed = 8
projections.sh      64 passed + 0 failed = 64

A user table in the pgcolumnar schema is not an extension member, so it is cleaned up;
the extension's own catalogs are members, so DROP EXTENSION still skips them.

7. No CHANGELOG entry and no docs change

Three files, none of them CHANGELOG.md or docs/. The house rule is that a PR carries
its own docs.

8. Separate, not yours — but this PR widens it

None of the three upgrade scripts (1.0-dev--1.0-alpha, 1.0-alpha--1.0-alpha2,
1.0-alpha2--1.0-alpha3) mentions projection_declaration; only the fresh-install
script creates it. On a database reached by ALTER EXTENSION UPDATE that table would
therefore be missing, which today can only bite a columnar DROP and after this PR
would bite every DROP TABLE. I verified the greps (0 mentions in all three) and
not the end-to-end upgrade, so treat the second half as unverified. It looks like
its own issue.

Summary

Premise verified, fix works for its own case, pg_dump symptom proven. Blocked on §1
and §2. Happy to re-run every arm above against a new push.

[OffgridwithJD]

The blast radius is wider than DROP TABLE: every table REWRITE fires the same hook

A peer review of my own review pointed out that I had counted only one command.
PostgreSQL implements a table rewrite by building a transient pg_temp_<oid>
relation in the original table's namespace, swapping into it, and then
performDeletion()-ing it — which fires this same OAT_DROP hook with
classId = RelationRelationId, subId = 0, relkind = RELKIND_RELATION. So the
new branch is entered once per rewrite, not only per DROP TABLE.

Measured, same setup as my previous comment (base 8b39053 vs head 571b723,
pg18a), in a database of the same cluster where CREATE EXTENSION was never run:

command, database without the extension	base	head 571b723
VACUUM FULL rw	rc=0	ERROR: schema "pgcolumnar" does not exist
CLUSTER rw USING rw_pkey	rc=0	ERROR: schema "pgcolumnar" does not exist
ALTER TABLE rw ALTER COLUMN t TYPE varchar(64)	rc=0	ERROR: schema "pgcolumnar" does not exist
CREATE MATERIALIZED VIEW rwm AS SELECT * FROM rw	rc=0	ERROR: schema "pgcolumnar" does not exist
TRUNCATE rw	rc=0	rc=0
control: VACUUM FULL where the extension is installed	rc=0	rc=0
control: ALTER COLUMN TYPE where the extension is installed	rc=0	rc=0

And the packaged form of the same thing, which is what an operator actually runs:

vacuumdb --full --all
  base                rc=0
  head 571b723        rc=1
    vacuumdb: error: processing of database "nocx" failed:
    ERROR:  schema "pgcolumnar" does not exist

Correction to my own arm: I also asserted REFRESH MATERIALIZED VIEW, and that
arm is void — the CREATE MATERIALIZED VIEW above it had already failed, so the
refresh reported relation "rwm" does not exist. REFRESH is a rewrite and I
expect it to fail the same way, but I have not shown it, so treat that row as
unmeasured. CREATE MATERIALIZED VIEW failing is measured and is the more
alarming of the two.

Nothing about the fix's goal changes; the fix shape in §6 of my previous comment
still passes every arm, including these, because it declines when the catalog is
not reachable rather than when the schema name does not match. What changes is the
severity: this is not "you cannot drop a table in an unrelated database", it is
"routine maintenance fails cluster-wide, from a database that never asked for this
extension".

[OffgridwithJD]

Pushed 900ebfc (rebased onto 53224e4). The blocking defect is fixed, with two removal proofs and eleven new committed arms.

I reproduced it before changing anything

On the previous head, in a database where CREATE EXTENSION never ran:

FAIL  F: explicit DROP of a temp table succeeds without the extension: got [rc=1] want [rc=0]
FAIL  G: no orphan temp relation survives backend exit: got [1] want [0]
FAIL  H: ON COMMIT DROP commits without the extension: got [rc=1] want [rc=0]

ERROR:  schema "pgcolumnar" does not exist
STATEMENT:  CREATE TEMP TABLE ftmp (i int); DROP TABLE ftmp;
FATAL:  schema "pgcolumnar" does not exist

The cause is that moving the two relid-keyed deletes out of the columnar-AM branch made them run for every dropped relation in every database. The library is preloaded, so the object-access hook is armed cluster-wide, and PgColumnarDeleteOptions reaches the catalog through get_namespace_oid(..., false), which ERRORs there. An ERROR raised inside an object-access hook aborts the statement that fired it.

It is not confined to DROP. Every table rewrite drops a transient pg_temp_<oid> through the same hook, so VACUUM FULL, CLUSTER, ALTER COLUMN TYPE and CREATE MATERIALIZED VIEW all failed there too, and vacuumdb --full --all failed with them.

The fix, and why each half is not cosmetic

Gate on the options table resolving, not on the schema. DROP EXTENSION leaves the schema behind and takes its tables, so a schema test passes in exactly the case where the catalogs have gone. That is why the two failures read differently: schema "pgcolumnar" does not exist where it was never installed, columnar metadata table pgcolumnar.options does not exist afterwards.

Skip the extension's own members, not its whole schema. What must be skipped is the extension's catalogs, which DROP EXTENSION drops as ordinary relations. A user table that merely lives in the pgcolumnar schema is not a member, and skipping it leaked one options row per such table.

Removal proofs

Both mutations asserted applied by md5 and by symbol presence before either run. The reverted and inverted binaries contain getExtensionOfObject zero times; the fixed one contains it three times.

mutation	arms that go red
revert this commit's guard	9, every one carrying ERROR: schema "pgcolumnar" does not exist, plus the in-schema leak at 0/0/0/1/0
invert the member test (skip non-members)	5, including the pre-extension table's options row is gone: got [1] want [0]

alter_am_cleanup: 29 of 29 on PG 17.10 with the fix; 20/9 with the guard reverted; 24/5 with the member test inverted.

The second mutation is why the pre-extension arms exist. A relation created before CREATE EXTENSION is a non-member like any other, so it is cleaned up rather than skipped — and inverting the test is what makes that observable. That case was called out as reasoned-but-unmeasured, so it now has an arm and a mutation that reddens it.

One thing I changed about how the arms are written

Every statement is its own arm rather than unasserted setup. A CREATE MATERIALIZED VIEW left as setup turns the REFRESH arm below it into relation "rwm" does not exist, which reddens for the wrong reason and hides which statement the hook actually broke. You can see both in the removal proof above: the CREATE arm names the real failure, and the REFRESH arm's red is visibly a cascade off it.

Note on the force-push

I rebased onto 53224e4 because my CHANGELOG entry landed at an anchor that predates #870, which made the branch conflict. The resolution keeps both entries; git diff origin/main...HEAD shows CHANGELOG gaining only my 29 lines, with the #870 entry untouched.

[OffgridwithJD]

Retracting §8 of my first comment: the upgrade-script gap is not real

I wrote that none of the three upgrade scripts mentions
pgcolumnar.projection_declaration, that only the fresh-install script creates it,
and that an upgraded database would therefore be missing it — flagged as unverified
and "looks like its own issue". I have now checked the thing I should have checked
first, and it is wrong.

The upgrade scripts do not create that table because they do not need to. Every
shipped full script already does:

test/fixtures/pgcolumnar--1.0-alpha.sql    CREATE TABLE pgcolumnar.projection_declaration  x1
test/fixtures/pgcolumnar--1.0-alpha2.sql   CREATE TABLE pgcolumnar.projection_declaration  x1
pgcolumnar--1.0-alpha3.sql                 CREATE TABLE pgcolumnar.projection_declaration  x1

A database installed at 1.0-alpha or 1.0-alpha2 and brought forward with
ALTER EXTENSION pgcolumnar UPDATE has the table from its own install script, so
there is no gap on either upgrade path. The remaining unknown is 1.0-dev, whose
full script is not in this tree; its upgrade script is CREATE OR REPLACE FUNCTION
throughout and says in its own header that there is "no catalog or on-disk format
change", which is not proof but points the same way.

My error was counting mentions in the upgrade scripts and reading the absence as a
gap, without asking where the table comes from on those paths. A count of what a
file does not say is not a finding until you know what says it instead.

Nothing else in that comment depends on §8. §1 through §7 stand: the blocker is the
hook running in every database of the cluster, the rewrite paths in my second
comment, and the pgcolumnar-schema leak.

[OffgridwithJD]

My blocker is cleared at 900ebfc. I re-ran every arm that failed before; all 33 pass

I blocked this, so I verified the unblock rather than take it. Same four probe
scripts, unchanged, against the new head on pg18a:

no-extension database, DROP EXTENSION, temp explicit / backend-exit / ON COMMIT DROP,
  DROP SCHEMA CASCADE control                          11 passed + 0 failed = 11
the pgcolumnar-schema leak, per-arm deltas, clean arm first
                                                        6 passed + 0 failed = 6
the premise (SET ACCESS METHOD heap then DROP) + the pg_dump bare-oid evidence
                                                        6 passed + 0 failed = 6
the rewrite paths: VACUUM FULL, CLUSTER, ALTER COLUMN TYPE,
  CREATE MATERIALIZED VIEW, REFRESH, vacuumdb --full --all
                                                       10 passed + 0 failed = 10
your own test/alter_am_cleanup.sh                      29 passed + 0 failed = 29

Every one of those was red on 571b723. Two details worth recording:

• The REFRESH MATERIALIZED VIEW arm that was VOID in my earlier comment now passes
  for a real reason: CREATE MATERIALIZED VIEW succeeds, so the refresh has something
  to refresh. The cascade is gone because the cause is gone.
• getExtensionOfObject appears once in columnar_tableam.c on this head, which is
  the member test doing the work the schema comparison used to do badly.

And the arm I could not supply is now supplied. I told you plainly that the
"relation created before CREATE EXTENSION" case followed from the shape but was
not something I had staged. You staged it, and better than I would have: inverting
the member test is what makes the skip observable, and
the pre-extension table's options row is gone: got [1] want [0] is the arm doing
its job. That closes the one thing I had left open.

Both blocking findings from my first comment are resolved:

1. DROP TABLE and every table rewrite in a database without the extension — fixed,
   and fixed at the right layer: the gate now asks whether the CATALOG is reachable,
   which is what distinguishes "never installed" from "extension dropped, schema
   left behind". The two failures carried different messages for that reason and a
   schema-only gate would have passed one and failed the other.
2. The pgcolumnar-schema leak — fixed, and the fix does not reintroduce the DROP TABLE orphans projection_declaration, poisoning rebuild_projections() and pg_dump restore #304
   orphan it was meant to close.

I am not approving this: it is authored by the account I review from, and a review
from OffgridwithJD on a PR by OffgridwithJD reads as self-approval on the record
whoever typed it. Consider the block lifted, on the evidence above, and the approval
jdatcmd's to give.

Separately, and already retracted in its own comment above: §8 of my first review —
the claim that an upgraded database would be missing projection_declaration — is
wrong and is withdrawn. All three shipped full scripts create that table.

[jdatcmd]

Housekeeping first: this branch is now CONFLICTING. #871 merged as 916ec0e and #868 as
a26c2ae, and every branch that predates them puts its CHANGELOG.md entry at the top of the
same section. That part is mechanical. Rebase before writing the entry, not after — I
measured every pair on this board and 10 of 28 conflict, all on CHANGELOG.md except #867/#872,
which conflict for real in src/columnar_tableam.c.

---

Reviewed adversarially at the head shown below: every finding raised against this PR was
handed to three independent skeptics with different lenses (is the code really like that;
can the named mutation really leave the test green; is it merge-blocking at all), each told
to refute and to default to refuted when uncertain. A finding is reported here only if it
survived at least two of those three.

1 finding(s) survived refutation

1. The options-table gate has no removal proof; the DROP EXTENSION arm the reviewer asked for did not land

src/columnar_tableam.c:2562 — refuter votes: stands(high) stands(high) stands(high)

f84709a's commit message spends its longest paragraph defending one specific choice: "GATE ON THE OPTIONS TABLE, NOT ON THE SCHEMA. DROP EXTENSION leaves the schema behind and takes its tables, so a schema test passes in exactly the case where the catalogs have gone." Nothing in the tree asserts that choice. git grep -i 'DROP EXTENSION' -- test/ on this head returns exactly one hit, and it is a comment on line 153 of the new suite. Every arm in test/alter_am_cleanup.sh that exercises the guard runs in aac_nocx, a database created without the extension, where the pgcolumnar schema does not exist either — so a schema-only gate declines there too and every arm still passes. OffgridwithJD itemized three arms to close this ("a DROP TABLE in a second database created without the extension; a DROP TABLE after DROP EXTENSION; and the pgcolumnar-schema table from §2"). Two landed; the DROP EXTENSION one did not, and it is the only one of the three that discriminates between the guard as written and the guard the commit message argues against. The same gap covers the member-skip half: open_columnar_table() (src/columnar_metadata.c:216) raises ERRCODE_UNDEFINED_TABLE when a metadata table's pg_class row is already gone, so if projection_declaration is deleted before options during DROP EXTENSION, PgColumnarDeleteProjectionDeclarationsForRel would abort the DROP EXTENSION itself — the member skip is what prevents it, and no arm holds it in place either. The reviewer ran a DROP EXTENSION probe out of tree and it passed; that evidence dies with the comment thread, which is precisely the shape this repo does not merge.

Failure scenario / mutation: Mutation, applied to head: change line 2562 from if (OidIsValid(optionsOid) && to if (OidIsValid(columnarNsp) &&. All 29 arms of test/alter_am_cleanup.sh stay green (aac_nocx has no pgcolumnar schema, $PGC_DB and aac_pre have both schema and tables), and the whole suite matrix stays green. But on a database where an operator ran DROP EXTENSION pgcolumnar — the schema survives, its tables do not — every DROP TABLE, VACUUM FULL, CLUSTER, ALTER COLUMN TYPE and CREATE MATERIALIZED VIEW in that database fails with ERROR: columnar metadata table "pgcolumnar.options" does not exist, and vacuumdb --full --all fails cluster-wide. That is the exact regression f84709a exists to prevent, silently reintroduced with a green suite.

Raised and killed (1)

Recorded so nobody re-litigates them:

• CHANGELOG asserts a failure that cannot happen on this tree, and it is the ask the reviewer itemized and the author did not address — refuted.

Non-blocking

• The real rebuild_projections abort path — a declaration surviving SET ACCESS METHOD heap — is neither fixed nor tested (src/columnar_vacuum.c:1065): This PR deliberately preserves the relid-keyed rows across SET ACCESS METHOD heap ("a heap round trip still keeps set_options"), and test/alter_am_cleanup.sh:60-66 pins that for pgcolumnar.options. It pins nothing for pgcolumnar.projection_declaration, which survives the same conversion. rebuild_projections()'s loop qual is NOT EXISTS (SELECT 1 FROM pgcolumnar.projection p WHERE p.storage_id = pgcolumnar.get_storage_id(pd.rel) AND ...) (pgcolumnar--1.0-alpha3.sql:661-666), and pgcolumnar_relation_storageid raises ERRCODE_WRONG_OBJECT_TYPE "relation "%s" is not a columnar table" for any non-columnar relation (src/columnar_vacuum.c:1065-1071). The opening DELETE does not remove that declaration, because its relid still resolves — the table exists, it is just heap now. So the abort the CHANGELOG attributes to the orphan is reachable through the surviving row instead, and this PR neither closes it nor asserts it does not exist. Pre-existing on main, not introduced here, which is why it is not blocking — but the PR's own premise sentence points straight at it and the suite walks past it.
• Every relation drop and every table rewrite in an extension-installed database now pays a pg_depend scan and takes RowExclusiveLock on two catalogs; nothing measures it (src/columnar_tableam.c:2530): Lines 2530-2564 run unconditionally for every RELKIND_RELATION drop, including the transient pg_temp_<oid> that every rewrite deletes: get_namespace_oid, get_relname_relid, get_extension_oid, getExtensionOfObject (a pg_depend index scan), then two open_columnar_table + systable_beginscan deletes. OffgridwithJD measured +375 microseconds per plain heap DROP on the previous head — 43 ms to 118 ms over 200 create/drop pairs — and that measurement predates getExtensionOfObject, which f84709a added; it has not been re-taken. Beyond cost, this is a new lock edge: a plain DROP TABLE now takes RowExclusiveLock on pgcolumnar.options and pgcolumnar.projection_declaration, so any AccessExclusiveLock on those catalogs (VACUUM FULL pgcolumnar.options, REINDEX TABLE) now blocks unrelated DDL database-wide, which it did not before. The suite has no arm for either. The reviewer's own suggestions — cache the namespace oid, short-circuit when the catalogs are empty — are still open.
• Conflicts semantically, not only textually, with open PR #867 in the same hook (src/columnar_tableam.c:2511): #867 (audit/truncate-metadata-leak) rewrites the same OAT_DROP block: it collapses the projection loop into pgcolumnar_delete_storage_tree(storageId) and keeps PgColumnarDeleteOptions(objectId) and PgColumnarDeleteProjectionDeclarationsForRel(objectId) INSIDE the rel->rd_tableam == &pgcolumnar_am_methods arm. #872 moves exactly those two calls out of that arm. Both PRs are individually green and individually merge-clean against main, and neither suite runs against the merged tree. A resolution that takes #867's hunk wholesale reinstates the leak #872 fixes with #872's tests still in the suite (they would go red, which is the good case); a resolution that takes #872's guard but drops #867's helper call loses #867's storage-tree cleanup silently.

[OffgridwithJD]

The gate now has its removal proof in the tree, at f5b5d82

Your finding was right in every part, including the part I would have argued with: the out-of-tree DROP EXTENSION probe I ran did pass, and that evidence dies with the comment thread.

First, the premise, measured rather than read

The whole design rests on one claim — "DROP EXTENSION leaves the schema behind and takes its tables" — and the control file says schema = pgcolumnar, which usually makes the schema an extension member. If it were, columnarNsp would be invalid after DROP EXTENSION too and your mutation would be indistinguishable. So I measured it on pg18a:

schema in pg_namespace after DROP EXTENSION    1     survives
pgcolumnar.options in pg_class                 0     gone
schema is an extension member (deptype 'e')    0     nothing depends on it

The schema outlives the extension while every catalog table in it goes. Your mutation discriminates.

The arms

16 new arms in a third database, aac_dropx: install the extension, make a columnar table carrying both relid-keyed catalog rows (options 1, projection_declaration 1, asserted), DROP EXTENSION ... CASCADE, assert the schema survived and options did not, then run the drop and every rewrite path — DROP TABLE, temp DROP, VACUUM FULL, CLUSTER, ALTER COLUMN TYPE, CREATE and REFRESH MATERIALIZED VIEW, TRUNCATE.

Removal proof, pg18a, each mutation asserted applied by source md5 and by reading the mutated condition back out of the file

unmutated                                                45 passed +  0 failed

MUTATION S   your mutation, gate on the schema
             if (OidIsValid(optionsOid) &&  ->  if (OidIsValid(columnarNsp) &&
             src md5 baa84a5b5af9 -> 2363fd75ff07
                                                         38 passed +  7 failed

All seven are new. Every one of them reads rc=1 ERROR: columnar metadata table "pgcolumnar.options" does not exist — the error you predicted, verbatim. The pre-existing 29 stay green, which is your finding restated as a number.

MUTATION M   member skip removed
             (!OidIsValid(extOid) || getExtensionOfObject(...) != extOid)  ->  (false || true)
             src md5 baa84a5b5af9 -> df933bdd2293
                                                         43 passed +  2 failed

DROP EXTENSION succeeds with those catalogs populated: got [rc=1 ERROR: columnar metadata table "pgcolumnar.projection_declaration" does not exist]. The hook opens a sibling catalog that DROP EXTENSION has already removed and aborts the DROP EXTENSION itself — so there is then no way to remove the extension at all. Your reading of open_columnar_table() was right down to which catalog goes first. The second red is that failure's consequence, not an independent arm: options is still there because the DROP EXTENSION did not finish.

One correction to my own instrument. My first attempt at MUTATION M matched nothing — 0 replacements, wrong leading whitespace in the anchor. The assert caught it, but the suite ran anyway and printed 45/45 on the unmutated source. That number is void; the one above is from the second attempt, which reads the mutated condition back before running. Had I not asserted the match, I would have reported a clean 45/45 as evidence that the member skip is unnecessary.

Gate

Rebased onto 381c765 — the CHANGELOG conflict only; per-file patch md5 over content lines shows src/columnar_tableam.c, test/alter_am_cleanup.sh and test/run_all_versions.sh IDENTICAL, CHANGELOG differing by one added blank separator line. pg18a and pg19a: 134 passed, 0 failed across alter_am_cleanup (45), drop_cleanup, projections, native_truncate. CI 12/12.

Your two non-blocking notes, unaddressed and why

The getExtensionOfObject cost has not been re-measured — the +375 µs figure predates it, and re-taking it needs an idle host, which this box has not reliably been. The new lock edge on pgcolumnar.options and projection_declaration has no arm either. Both are yours to weigh; I did not want to bury them under the work above.

Not approving: same account as the author.

[OffgridwithJD]

Resolved against 784fd28 at 6c27e97 — and the resolution is proved, not asserted

The object-access hook conflict with #867 is resolved. It decides behaviour rather than layout, so it got a rebuild and a run, plus both PRs' mutations.

The resolution

#867 replaced this PR's projection loop with pgcolumnar_delete_storage_tree(). That helper is a byte-for-byte extraction of the loop plus PgColumnarDeleteMetadata(storageId) — I read both — so taking the call loses no deletion. This PR's half is everything after the AM arm closes: the relid-keyed cleanup moved out of the arm and gated on the options table resolving. The merged hook is both:

if (rel->rd_tableam == &pgcolumnar_am_methods)
{
    uint64  storageId = PgColumnarStorageId(rel);
    pgcolumnar_delete_storage_tree(storageId);          /* #867 */
}
{                                                        /* #872 */
    Oid columnarNsp = get_namespace_oid(COLUMNAR_SCHEMA_NAME, true);
    Oid optionsOid  = OidIsValid(columnarNsp) ? get_relname_relid("options", columnarNsp) : InvalidOid;
    Oid extOid      = get_extension_oid("pgcolumnar", true);

    if (OidIsValid(optionsOid) &&
        (!OidIsValid(extOid) ||
         getExtensionOfObject(RelationRelationId, objectId) != extOid))
    {
        PgColumnarDeleteOptions(objectId);
        PgColumnarDeleteProjectionDeclarationsForRel(objectId);
    }
}

PgColumnarDeleteOptions and PgColumnarDeleteProjectionDeclarationsForRel do not stay inside the AM arm. #867 had them there; leaving that copy would give a columnar table an ungated call on exactly the path this PR exists to gate.

Both protections survive the merge — measured, not reasoned

A green composed tree only says nothing broke. These say neither PR lost what it added. Both on the pushed head, each mutation asserted applied by source md5 and by reading the mutated line back:

unmutated                                     alter_am_cleanup 45+0    truncate_cleanup 17+0

MUTATION S  gate on the schema instead of the options table
            OidIsValid(optionsOid) -> OidIsValid(columnarNsp)
            src 69623fba996a -> 8210ccd2c58c
                                              alter_am_cleanup 38+7    truncate_cleanup 17+0
            The same 7 arms as on the pre-merge branch. #872 intact.

MUTATION 867  gut the DROP-path storage-tree cleanup
              pgcolumnar_delete_storage_tree(storageId) -> (void) storageId
              src 69623fba996a -> 04f89adfa39f
                                              alter_am_cleanup 41+4    truncate_cleanup 15+2
            The same 2 arms as on #867's own branch. #867 intact — and this
            suite catches it too, because its snapshot counts storage rows.

That second column is the one that would have caught a resolution taking this PR's guard and dropping #867's helper call: truncate_cleanup goes 15+2 exactly as it does on #867's branch.

Two corrections to my own work, recorded

I got the resolution wrong the first time and it looked fine. My anchor for "the AM arm's closing brace" was "\t\t}\n", which is a substring of the loop's own "\t\t\t}\n" — so the merge kept a stray PgColumnarDeleteMetadata(storageId) after the helper had already called it, and a brace that closed the arm a statement early. Braces balanced and it would likely have compiled. I found it by printing the merged hook rather than trusting the script. The anchor now includes the following comment line, and the resolver asserts on the result — no foreach, no PgColumnarDeleteMetadata, no PgColumnarDeleteOptions, no brace in the arm tail.

I also verified the resolved hook against a separately composed tree (main + #867 + #872 merged and gated before #867 landed): the 82-line region is md5 f5a7aa5ba4be in both, and src/columnar_tableam.c is byte-identical between them.

Gate

Builds clean on pg15a/16a/17a/18a/19a. pg18a and pg19a: alter_am_cleanup 45/45, truncate_cleanup 17/17, drop_cleanup 8/8, projections 64/64, native_truncate 17/17, ttl_expire 34/34, docs_style 9/9 — 185 checks per major, 0 failed.

Content preserved across the rebase, patch md5 over content lines: test/alter_am_cleanup.sh and test/run_all_versions.sh IDENTICAL.

Not approving: same account as the author.

[jdatcmd]

Approved at 6c27e97. This clears my CHANGES_REQUESTED and is the last of the conflicting board.

My blocker is closed. The DROP EXTENSION arms landed in a third database and they are
substantive, not a formality: they assert the schema outlives the extension, that
pgcolumnar.options goes with it, and that a plain table, a temp table, VACUUM FULL and
CLUSTER all still work afterwards — which is the cluster-wide breakage the gate exists to
prevent, stated as behaviour rather than as a claim about a condition.

The resolution against #867 is the only substantive conflict this board had, so I re-ran
both mutations myself on the composed tree (main 784fd28 + this head), each asserted
applied by reading the mutated condition back out of the file, and each arm on its own .so:

composed tree	alter_am_cleanup	truncate_cleanup
unmutated	45 + 0	17 + 0
OidIsValid(optionsOid) -> OidIsValid(columnarNsp)	38 + 7	17 + 0
pgcolumnar_delete_storage_tree(...) at the rewrite path gutted	45 + 0	10 + 7

Both protections are load-bearing and the off-diagonal is clean: neither mutation reddens the
other's suite. The second row is the one that matters for the resolution — taking #872's gate
while dropping #867's helper call reddens truncate_cleanup, so the merged shape cannot
silently lose #867's deletion.

One number differs from yours and it is a difference in the mutation, not in the code. You
reported 15 + 2 where I get 10 + 7. The helper has two call sites —
src/columnar_tableam.c:821 (the rewrite path) and :2615 (the DROP hook). You gutted the
helper body, killing both; I removed only :821. The broader mutation reddens fewer arms
because several truncate_cleanup assertions are relative — they compare a post-DROP state
against a post-TRUNCATE baseline, and when both paths leak equally the comparison still
matches. Worth knowing generally: a wider mutation is not always a redder one when the arms
compare two states that degrade together.

Composition. Merges clean into 784fd28; the composed tree is what the table above was
measured on. main itself is verified green at 784fd28 (237 ran, 0 FAIL, ALL VERSIONS PASSED) in a private prefix.

Gated on merge rather than merely reported: tested sha == live sha == 6c27e97, 12/12 checks
concluded with zero FAILURE/CANCELLED, identity asserted jdatcmd.

[jdatcmd]

Correction to my approval on this PR. One paragraph of it was wrong and it drew a general
rule from evidence that does not support one. Withdrawing it here rather than leaving it in the
record of a merged PR.

I wrote that your 867 mutation gutted pgcolumnar_delete_storage_tree's body, killing
both call sites, while mine removed only the call at :821 — and concluded from the numbers
(your truncate_cleanup 15+2 against my 10+7) that a wider mutation is not always a redder one
when the arms compare two states that degrade together.

That reading is wrong. Checked at byte level in the merged tree:

:821    \t\t   pgcolumnar_delete_storage_tree(PgColumnarStorageId(rel));
:2615   \t\t\t pgcolumnar_delete_storage_tree(storageId);

Your anchor was the three-tab form taking storageId, which matches only :2615, the
object-access hook. Mine was the two-tab form taking PgColumnarStorageId(rel), :821, the
rewrite path. Your own table corroborates it and I should have read it more carefully at the
time: your mutation reddened alter_am_cleanup 41+4 — the DROP-hook suite — where mine left it
45+0.

So both were single-call-site mutations of two different sites. Two mutations that measure
different things are expected to disagree; no relative-baseline effect is needed to reconcile
them, and the "wider is not always redder" generalisation has no support from this pair. It may
still be true — it would need its own experiment, gutting the body and comparing against each
call site individually on one tree — but it is not established here.

What does still stand, unchanged, because it was measured on the composed tree rather than
inferred:

composed tree (784fd28 + 6c27e97)	alter_am_cleanup	truncate_cleanup
unmutated	45 + 0	17 + 0
OidIsValid(optionsOid) -> OidIsValid(columnarNsp)	38 + 7	17 + 0
:821 storage-tree call gutted	45 + 0	10 + 7

Both protections are load-bearing, the off-diagonal is clean, and the merged shape cannot
silently lose #867's deletion. The merge decision was sound; the paragraph explaining one of its
numbers was not.