storage.relation_oid is orphaned by ALTER COLUMN TYPE, silently disabling the written-stripe-limit lookup

[jdatcmd]

ALTER TABLE ... ALTER COLUMN TYPE leaves pgcolumnar.storage.relation_oid
pointing at the dropped transient rewrite relation. The row survives, the data
survives, and the one lookup keyed on that column silently finds nothing from
then on.

Found while attributing the unindexed catalog scans for #1207. It is not a
performance finding and it changes what #1210 should build, so it is filed
separately.

Measurement

Premise, action, control, all in one run on ca3089fe:

PREMISE: a written limit exists and the row points at the live table
   oid_of_t=64874
   storage: relation_oid=64874 row_group_limit=1234
     lookup by live oid finds: 1

ACTION: ALTER TABLE t ALTER COLUMN id TYPE bigint;

AFTER:
   oid_of_t=64874  relfilenode=64881
   storage: relation_oid=64881 row_group_limit=150000
     lookup by live oid finds: 0
     relation_oid resolves in pg_class: 0
  rows still readable: 5000

CONTROL: a table that was never rewritten
     lookup by live oid finds: 1

64881 is the transient relation PostgreSQL creates for the rewrite. Its OID
becomes the table's new relfilenode and its pg_class row is then dropped, so
the stored value resolves to nothing — relation_oid resolves in pg_class: 0.
The writer is columnar_write_state.c:756, s.relationOid = RelationGetRelid(rel), which is correct at the time it runs: during a rewrite
the relation it is handed is the transient one.

row_group_limit reverts to the default in the same step. That part is honest —
the rewrite genuinely ran under the session default — but it means the recorded
geometry is lost twice over.

Which paths are affected

  ALTER COLUMN TYPE          premise_before=1  after=0  orphaned_rows=1  data=3000
  VACUUM FULL                premise_before=1  after=1  orphaned_rows=0  data=3000  ERROR: columnar: CLUSTER and VACUUM FULL are not supported
  CLUSTER                    premise_before=1  after=1  orphaned_rows=0  data=3000  ERROR: columnar: CLUSTER and VACUUM FULL are not supported
  TRUNCATE + reinsert        premise_before=1  after=1  orphaned_rows=0  data=10
  ADD COLUMN DEFAULT         premise_before=1  after=1  orphaned_rows=0  data=3000
  SET ACCESS METHOD heap     premise_before=1  after=0  orphaned_rows=0  data=3000
  no-op control              premise_before=1  after=1  orphaned_rows=0  data=3000

Only ALTER COLUMN TYPE orphans a row. SET ACCESS METHOD heap also ends at
after=0, but with orphaned_rows=0 — the row is deleted, which is right,
since the table is no longer columnar. The no-op control holds at 1, so
after=0 is not something this fixture produces on its own.

A note on the two ERROR lines, because they nearly cost me the finding. My
first pass read "VACUUM FULL leaves relation_oid intact" as evidence that the
rewrite path was selective. It is not evidence of anything: the extension
refuses VACUUM FULL and CLUSTER, so nothing ran. The premise column is
what separated "unaffected" from "never executed".

Consequence

storage.relation_oid has exactly one writer and one reader:

src/columnar_metadata.c:1979   values[Anum_native_storage_relation_oid - 1] = ObjectIdGetDatum(s->relationOid);
src/columnar_metadata.c:3335   ScanKeyInit(&key[0], Anum_native_storage_relation_oid, ...);

The reader is pgcolumnar_written_stripe_row_limit(). After a type rewrite it
matches no row and returns 0, so the cost model loses the written-geometry term
for that table permanently — until the next rewrite happens to run under a
session that sets the GUC. Nothing errors and no query returns a wrong answer;
the plan is simply costed without the term. That is why it has gone unnoticed.

What it means for #1210

@OffgridwithJD — this answers the question you left open and it answers it
awkwardly. relation_oid is unique: no duplicate survived ALTER COLUMN TYPE, TRUNCATE, ADD COLUMN ... DEFAULT, SET ACCESS METHOD, or an aborted
first write. So a UNIQUE index is buildable.

But uniqueness was the wrong question to be blocked on, and that is my doing as
much as yours — I would have gone looking for the same thing. The column is
unique and stale, so an index on it makes a lookup that returns the wrong
answer return it faster. Your measurement in #1210 stands exactly as it is: the
scan is real, it is 2 x pages, and position not size is the driver. It is the
remedy that needs a step in front of it — keep relation_oid current across a
rewrite — before the index is worth adding. I would rather fix that first and
then index, than index and inherit this.

I will take both, since they are one change to one column and I have the
upgrade-script machinery out for #1207 anyway.

Repro

CREATE EXTENSION pgcolumnar;
SET pgcolumnar.stripe_row_limit = 1234;
CREATE TABLE t (id int) USING pgcolumnar;
INSERT INTO t SELECT g FROM generate_series(1,5000) g;

SELECT count(*) FROM pgcolumnar.storage WHERE relation_oid = 't'::regclass::oid;  -- 1

ALTER TABLE t ALTER COLUMN id TYPE bigint;

SELECT count(*) FROM pgcolumnar.storage WHERE relation_oid = 't'::regclass::oid;  -- 0
SELECT count(*) FROM pgcolumnar.storage s
  WHERE NOT EXISTS (SELECT 1 FROM pg_class c WHERE c.oid = s.relation_oid);       -- 1

Verified on PG18, ca3089fe, clean main build (probe residue checked at 0
after restoring the container install).

🤖 Generated with Claude Code

https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n

[jdatcmd]

Plan: stop reading relation_oid, resolve through the metapage instead

This fixes #1211 and #1210 with one change, no new index and no catalog
migration — and it means the remedy proposed in #1210 should probably not be
built. Posting the plan before the production code, with the measurement
attached so it can be refused on evidence rather than on preference.

The measurement that decides it

PgColumnarStorageId(rel) reads the storage id from the relation's own
metapage, not from a catalog, so it cannot go stale the way a stored OID can.
Driven across the rewrite that breaks relation_oid:

BEFORE the rewrite:
   metapage storage_id : 10000000000
   catalog row         : storage_id=10000000000 relation_oid=68228 limit=1234

AFTER ALTER COLUMN TYPE:
   metapage storage_id : 10000000001
   catalog row         : storage_id=10000000001 relation_oid=68235 limit=150000
   lookup by metapage id finds: 1
   lookup by live reloid finds: 0

The metapage tracks the rewrite exactly; relation_oid is left naming the
dropped transient relation. This is already the idiom in our own SQL —
pgcolumnar--1.0-alpha2--1.0-alpha3.sql joins WHERE st.storage_id = pgcolumnar.get_storage_id(rel) in three places.

Goal

pgcolumnar_written_stripe_row_limit() resolves the storage row through
storage_id and probes storage_pkey, instead of scanning storage by
relation_oid.

That gets all three properties at once:

• storage.relation_oid is orphaned by ALTER COLUMN TYPE, silently disabling the written-stripe-limit lookup #1211 disappears. The lookup stops depending on a value a rewrite
  invalidates. It is not "the staleness is fixed", it is "nothing reads the
  stale column".
• Every planned query scans pgcolumnar.storage twice; relation_oid has no index #1210 disappears. storage_pkey on storage_id already exists and is
  unique, so the whole-catalog scan — 2 x pages, twice per planned query —
  becomes an index probe. No index to add, so nothing to put in
  pgcolumnar--1.0-alpha5.sql and the alpha4->alpha5 script, and
  native_upgrade_converge.sh has nothing to hold identical.
• relation_oid becomes write-only. Whether it should then be dropped is a
  separate question and explicitly not in this change.

Non-goals

Dropping or repopulating relation_oid. Any other storage reader. The five
other storage scan sites from #1207.

Phases, and how each is proven

1. Establish the cost of the new path. The three callers
   (columnar_customscan.c:2064, :2439, :3141) hold an Oid in planner
   context, not a Relation, so the new path must open the relation and read
   its metapage. Proven by measuring heap_blks_hit and shared hit for a
   planned query before and after, at 500/1000/2000/3000 columnar tables —
   the same fixture @OffgridwithJD used in Every planned query scans pgcolumnar.storage twice; relation_oid has no index #1210, so the numbers are
   comparable to the ones already posted there.
2. The failing test first, both harnesses. A query planned against a table
   that has been through ALTER COLUMN TYPE must still see its written
   row_group_limit. That test fails on main today, for storage.relation_oid is orphaned by ALTER COLUMN TYPE, silently disabling the written-stripe-limit lookup #1211's reason, and
   it is the regression test the fix is owed. Proven by running it red before
   the fix.
3. The change. Proven by the test above going green, plus seq_scan on
   pgcolumnar.storage reaching 0 for a planned query, with the removal proof
   reverting to the relation_oid scan and going red.
4. Matrix. PG 15-19, ledger rows with a measured major set.

Risks and open questions, stated rather than resolved

• Opening a relation in the planner is the real risk, and it is unmeasured.
  The lock is already held by the range table entry, so NoLock is the
  standard idiom, but a metapage read is still a buffer access on a path that
  runs for every planned query over a columnar table. I expect it to be
  cheaper than a scan that is 2 x pages and grows with the catalog, but
  expecting is not measuring and phase 1 exists to settle it before any
  production code. If it comes back worse, this plan is wrong and Every planned query scans pgcolumnar.storage twice; relation_oid has no index #1210's
  index is the better answer after all.
• One caller (:2439) passes heapRelid, which may not be the columnar
  relation. Needs reading before it is assumed to behave like the other two.
• Whether any other reader of relation_oid appears later and reintroduces
  the staleness. Today there is exactly one reader and one writer, so this is
  cheap now and gets more expensive the longer it waits.

What is assumed vs decided

Decided, on the measurement above: the metapage is authoritative across a
rewrite and relation_oid is not.

Assumed, pending phase 1: that resolving through the metapage is cheaper
than the current scan for a planned query. Everything after phase 1 depends on
it.

@OffgridwithJD — you measured #1210 and proposed the index, so this is your
call as much as mine. If phase 1 says the metapage read is the more expensive
path, say so and I will build your index instead. I am starting on phases 1 and
2, which are measurement and a failing test, and neither commits us.

[jdatcmd]

Open question resolved: all four callers hold the columnar relation's OID

The plan flagged columnar_customscan.c:2439 as possibly not the columnar
relation, because the parameter is named heapRelid. It is not what the name
says. pgcolumnar_zonemap_survival(RelOptInfo *rel, Oid heapRelid) is called
from three places and every one passes the range table entry's relation:

2671   pgcolumnar_zonemap_survival(rel, relid)        <- pgcolumnar_refined_scan_cost(rel, relid, ...)
2797   pgcolumnar_zonemap_survival(rel, relid)        <- pgcolumnar_scan_io_run_cost(rel, relid)
3151   pgcolumnar_zonemap_survival(rel, rte->relid)

So all four call sites of pgcolumnar_written_stripe_row_limit() receive the
columnar table's OID, and the same resolution works for all of them. The
misleading name is worth fixing separately; it is not a behaviour difference.

Phase 1, the cost measurement that decides the plan, is next. Nothing assumed
from this beyond what is printed above.

[jdatcmd]

Phase 1 is done, and it does not decide what I expected it to decide

The plan said phase 1 would settle whether resolving through the metapage is cheaper than the current scan, and that if it came back worse, #1210's index is the better answer. It came back showing the cost question cannot separate them. Both remedies are O(1) where the current path is O(pages), so cost is not the discriminator. What separates them is correctness, and that is #1211's own subject.

The current path grows, and it grows exactly as 2 x pages

Blocks of pgcolumnar.storage touched per ONE planned query, counters reset immediately before the measured EXPLAIN and flushed immediately after, one backend per measurement, first plan discarded as cold:

  rows   pages  relation  blks total  2*pages  planning buffers
  1      1      FIRST     2                    shared hit=16
  1      1      LAST      2           2        shared hit=16
  50     1      FIRST     2                    shared hit=16
  50     1      LAST      2           2        shared hit=16
  200    2      FIRST     4                    shared hit=18
  200    2      LAST      4           4        shared hit=18
  1000   8      FIRST     4                    shared hit=19
  1000   8      LAST      16          16       shared hit=25
  3000   23     FIRST     4                    shared hit=19
  3000   23     LAST      46          46       shared hit=40

LAST blocks == 2 * pages on every row. Two scans per planned query, each reading the whole relation.

FIRST and LAST are the finding. A sequential scan stops at the first match, so the OLDEST columnar relation never pays for the catalog behind it and stays flat at 4 blocks. The NEWEST pays for all of it. In a database that keeps making tables, the tables people are actively querying are the expensive ones.

At 3000 columnar relations a planned query pays 40 planning buffers against a 16-buffer baseline, and it is unbounded.

Two measurements I had to throw away first, both recorded because they read as answers

seq_tup_read cannot see this work. systable_beginscan pushes the key into heapam, so only MATCHING tuples are returned and counted. It reads 2 whether the table holds one row or three thousand. A 200-row table cannot be scanned in 2 tuples, and that implausible value is the only thing that gave it away.

Planning over t0 measures the best case and reports it as the cost. My first fixture always planned over the first table created, whose storage row is at the front of the heap. It came back flat at 4 blocks to 3000 rows, which is TRUE and answers the question next door. The growth only appears when the relation's row is at the end.

What this means for the two remedies

  current                2 x pages, growing without bound
  metapage read          ~1 buffer per call, x2 calls, CONSTANT
  index on relation_oid  ~2-3 buffers per call, x2 calls, CONSTANT

Cost does not choose. Correctness does, and it is the thing #1211 is about: relation_oid is orphaned by ALTER COLUMN TYPE, so an index on it would make a stale lookup faster without making it right. The metapage is authoritative across a rewrite. That was already the DECIDED half of the plan and the measurement leaves it standing.

@OffgridwithJD — #1210 is yours and this is the number you were waiting on. I read it as: the performance case for your index is real, and it is subsumed by the correctness case for the metapage, because both are constant-cost and only one of them survives a rewrite. If you read it differently, say so; you filed the profile and I would rather argue it now than after either of us writes code.

Standing facts confirmed at source

  indexes on pgcolumnar.storage   storage_pkey, on (storage_id)
  relation_oid indexed            no
  readers of relation_oid         exactly one, columnar_metadata.c:3387
  writers                         exactly one, columnar_metadata.c:1979
  call sites reached per query    two

The probe was a scratch script, not a suite; it reports SKIPPED (ran no checks) and is not committed. If this becomes a change, the measurement becomes a real suite with arms in both harnesses, in that change.

[jdatcmd]

Fixed on main by #1250, verified on the merged tree

PgColumnarRetargetStorageRelation (@linuxhikerpm) points the storage row back
at the relation that survives the swap. Merged as 2f18b8dc.

Verified on merged main 2f18b8d, not on the branch:

  rewrite_storage_oid  PG17  rc=0  0 FAIL  (5 checks)
  rewrite_storage_oid  PG18  rc=0  0 FAIL  (5 checks)

    PASS  premise: the storage row points at the live table before the rewrite
    PASS  premise: the table holds every inserted row
    PASS  premise: the rewritten table still holds every row
    PASS  premise: a table that was not rewritten still points at itself
    PASS  a type rewrite leaves the storage row pointing at the live table

The last arm is this issue's subject. The two premises either side of it are
what make it mean something: the row points at the live table before the
rewrite, and an unrewritten table still points at itself — so a pass is not the
lookup being broken in a way that happens to match.

What this unblocks, and what it does not

#1210 noted that its own remedy must not be done first, because an index on
a column that is unique and stale makes a wrong answer arrive faster. The
staleness is now fixed, so that objection is discharged — though #1210's
preferred remedy has since become the metapage route, which needs no index at
all.

This issue is closed; #1210 is not. Its subject is the scan cost, which is
untouched by this.

One note for whoever reads this next

PgColumnarRetargetStorageRelation calls pgcolumnar_index_oid("storage_pkey")
rather than the size-aware pgcolumnar_scan_index_oid(rel, ...) introduced by
#1254. That is deliberate and matches the prevailing convention for this
catalog — counted on main, storage_pkey goes through pgcolumnar_index_oid
twice and through the size-aware form not at all. #1213 made row_group and
free_space size-aware on the compaction path and #1254 did options and
projection on the planner path; storage was in neither. If it should be
size-aware that is its own change across all three call sites with its own
measurements, not a consistency edit.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP