fix: CREATE MATERIALIZED VIEW WITH DATA orphaned its storage row (#1275)

CHANGELOG.md

@@ -18,6 +18,32 @@ true until the next version shipped.
 
 ### Fixed
 
+- `CREATE MATERIALIZED VIEW ... USING pgcolumnar ... WITH DATA` no longer
+  orphans `pgcolumnar.storage.relation_oid` (#1275). The statement builds a
+  transient relation, fills it and swaps, exactly as a rewriting `ALTER` does,
+  so the storage row was written with the transient's OID and the swap left it
+  naming a relation that no longer existed. Measured: the row named a dropped
+  `16527` while the matview was `16523`, and a lookup by the live relation found
+  nothing.
+
+  `REFRESH MATERIALIZED VIEW` already repaired it, because that node type was in
+  the repair gate `ALTER`, `TRUNCATE` and `REFRESH` share. `CreateTableAsStmt`
+  was not, so a matview stayed broken between `CREATE` and its first `REFRESH`.
+
+  **Restricted to `objtype == OBJECT_MATVIEW`, and a control says why.**
+  `CREATE TABLE ... AS ... USING pgcolumnar` is the same parse node and does
+  **not** have the defect -- it fills the relation it created instead of
+  swapping a transient in -- so the remedy is narrower than the node type.
+  `test/rewrite_storage_oid.sh` asserts that as a premise, which refuses the
+  narrowing the day it stops being true.
+
+  The statement's own name is the only handle: `pgcolumnar_rewritten_relids` is
+  empty at that point, so the recorded-list route the `TRUNCATE` path uses is
+  not available.
+
+  `pgcolumnar.analyze()` stopped reading this column in #1276; the column itself
+  was still wrong for every other reader until this.
+
 - Planning a query over a columnar relation no longer sequentially scans
   `pgcolumnar.storage` (#1210). `pgcolumnar_written_stripe_row_limit` looked its
   row up by `relation_oid`, which has no index, so the scan walked the catalog

src/columnar_tableam.c

@@ -2897,7 +2897,7 @@ pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString,
 	 */
 	if (parsetree != NULL &&
 		(IsA(parsetree, AlterTableStmt) || IsA(parsetree, TruncateStmt) ||
-		 IsA(parsetree, RefreshMatViewStmt)))
+		 IsA(parsetree, RefreshMatViewStmt) || IsA(parsetree, CreateTableAsStmt)))
 	{
 		List	   *targets = NIL;
 		ListCell   *lc;
@@ -2939,7 +2939,62 @@ pgcolumnar_process_utility(PlannedStmt *pstmt, const char *queryString,
 			 * layout, not by any rule, and one added field in either struct turns
 			 * it into a wrong pointer with no diagnostic.
 			 */
-			if (IsA(parsetree, RefreshMatViewStmt))
+			if (IsA(parsetree, CreateTableAsStmt))
+			{
+				/*
+				 * CREATE MATERIALIZED VIEW ... WITH DATA (#1275). It builds a
+				 * transient, fills it and swaps, exactly as a rewriting ALTER
+				 * does, so the storage row is written with the transient's OID
+				 * and the swap leaves it naming a relation that no longer
+				 * exists. Measured: the row named a dropped 16527 while the
+				 * matview was 16523, and a lookup by the live relation found
+				 * nothing until the first REFRESH repaired it through the
+				 * RefreshMatViewStmt arm above.
+				 *
+				 * ITS OWN ARM RATHER THAN THE CAST BELOW, for the reason the
+				 * comment above already gives about the other two: they share a
+				 * field offset by coincidence of layout, and this node does not
+				 * share it at all -- its relation is reached through
+				 * ->into->rel.
+				 *
+				 * MATVIEWS ONLY, AND A CONTROL SAYS SO. `CREATE TABLE ... AS
+				 * ... USING pgcolumnar` is the same parse node and does NOT have
+				 * the defect -- measured, its storage row points at itself,
+				 * because it fills the relation it created instead of swapping
+				 * a transient in. test/rewrite_storage_oid.sh asserts that as a
+				 * premise, so the day it stops being true this narrowing is
+				 * refused rather than silently wrong.
+				 *
+				 * THE SET IS CLOSED AT TWO VALUES, so this partitions the node
+				 * type rather than guessing at it. parsenodes.h annotates
+				 * CreateTableAsStmt.objtype as OBJECT_TABLE or OBJECT_MATVIEW,
+				 * and nothing else -- read on the 15, 17 and 19 headers rather
+				 * than on one, since this ships across the matrix. SELECT INTO
+				 * is OBJECT_TABLE, so it is the control's case and not a third
+				 * one. Reported by @jdatcmd.
+				 *
+				 * NoLock, AND FOR A CREATE THE REASON IS NOT THE ONE STATED
+				 * ABOVE. That comment says the statement already holds
+				 * AccessExclusiveLock ON THE HIERARCHY, which was written for
+				 * TRUNCATE and for a type change -- statements that operate on
+				 * relations existing before they began. A CREATE has no
+				 * hierarchy and no pre-existing relation: the lock is held
+				 * because the creating transaction holds AccessExclusiveLock on
+				 * a relation it has just made, and this block runs after
+				 * standard_ProcessUtility, so the create has completed. Same
+				 * conclusion, different derivation, and the next reader would
+				 * otherwise inherit the stated reason rather than the true one.
+				 *
+				 * pgcolumnar_rewritten_relids is EMPTY here (measured), so the
+				 * recorded-list route the TRUNCATE path uses is not available
+				 * and the statement's own name is the only handle.
+				 */
+				CreateTableAsStmt *cts = (CreateTableAsStmt *) parsetree;
+
+				rv = (cts->objtype == OBJECT_MATVIEW && cts->into)
+					? cts->into->rel : NULL;
+			}
+			else if (IsA(parsetree, RefreshMatViewStmt))
 				rv = ((RefreshMatViewStmt *) parsetree)->relation;
 			else
 				rv = ((AlterTableStmt *) parsetree)->relation;

test/analyze_function.sh

@@ -944,33 +944,39 @@ check_text "premise: the capture helper reports a success when there is one" \
 check_text "pgcolumnar.analyze() still refuses a relation that has never been written" \
 	"$empty_state" "refused P0001"
 
-# AND THE BEHAVIOUR CHANGE IS ASSERTED, NOT LEFT TO BE DISCOVERED. A matview
-# created WITH DATA has an ORPHANED relation_oid until its first REFRESH
-# (#1275): the storage row points at a transient relation that no longer
-# exists. Keyed on relation_oid this found nothing and refused a matview holding
-# rows; through the metapage it resolves and succeeds.
-#
-# THIS IS NOT #1275 BEING FIXED. The orphan is still in the catalog and every
-# other reader of relation_oid still meets it. Only this caller stops hitting
-# it, and an arm that says so is the difference between a recorded change and a
-# reader concluding the orphan is gone.
+# A MATVIEW CREATED WITH DATA, AND THIS ARM HAS BEEN SUPERSEDED ONCE ALREADY.
+#
+# #1276 added it to record a behaviour change: a WITH DATA matview had an
+# ORPHANED relation_oid until its first REFRESH, so keyed on that column
+# analyze() refused a matview holding rows, and through the metapage it
+# resolved. Its premise asserted the orphan was still there -- and #1275 then
+# removed the orphan, so the premise went red. That is the premise doing its
+# job: it recorded a fact about the world and reddened when the world changed,
+# rather than passing quietly under an arm that no longer meant anything.
+#
+# WHAT IS LEFT TO GUARD, AND WHAT IS NOT. With the orphan gone, both routes
+# resolve, so this shape no longer distinguishes them and is NOT evidence for
+# #1276's fix -- the projection arms above are. What it still holds is that
+# analyze() reaches a freshly created matview at all, which was an outright
+# error before #1276, and that the two routes agree about WHICH storage, which
+# is #1275's guarantee cross-checked from a different file.
 q "DROP MATERIALIZED VIEW IF EXISTS af_mv;
    CREATE MATERIALIZED VIEW af_mv USING pgcolumnar AS SELECT a, b, c FROM af_proj;" >/dev/null
-mv_by_reloid="$(q "SELECT count(*) FROM pgcolumnar.storage
+mv_by_reloid="$(q "SELECT coalesce(min(storage_id)::text, 'none') FROM pgcolumnar.storage
 	WHERE relation_oid = 'af_mv'::regclass::oid;")"
-mv_by_meta="$(q "SELECT count(*) FROM pgcolumnar.storage
+mv_by_meta="$(q "SELECT coalesce(min(storage_id)::text, 'none') FROM pgcolumnar.storage
 	WHERE storage_id = pgcolumnar.get_storage_id('af_mv'::regclass);")"
 mv_state="$(q "SELECT af_try('af_mv'::regclass);")"
-echo "-- a WITH DATA matview: rows by relation_oid=$mv_by_reloid, by metapage id=$mv_by_meta, analyze() $mv_state"
+echo "-- a WITH DATA matview: by relation_oid=$mv_by_reloid, by metapage=$mv_by_meta, analyze() $mv_state"
 
-# THE ORPHAN MUST STILL BE THERE, or this arm is measuring a matview that was
-# never in the broken state and says nothing about the change.
-check_num "premise: the matview's relation_oid still finds no storage row" \
-	"$mv_by_reloid" "0"
-check_num "premise: and its metapage still finds exactly one" \
-	"$mv_by_meta" "1"
+# THE IDS, NOT THEIR COUNTS. Two rows counting 1 each can still be two
+# DIFFERENT storages, and that is the failure this arm would most want to see.
+check_text "premise: the matview's relation_oid finds a storage row" \
+	"$(if [ "$mv_by_reloid" = "none" ]; then echo none; else echo found; fi)" "found"
+check_text "the two routes agree which storage a WITH DATA matview owns" \
+	"$mv_by_meta" "$mv_by_reloid"
 
-check_text "pgcolumnar.analyze() now reaches a matview whose relation_oid is orphaned" \
+check_text "pgcolumnar.analyze() reaches a matview created WITH DATA" \
 	"$mv_state" "succeeded"
 
 

test/check_ledger.tsv

@@ -1772,8 +1772,14 @@ range_pruning	range_pruning	premise: the range type collates differently from it
 range_pruning	range_pruning	so does a containment probe	15;16;17;18;19	never	-
 range_pruning	range_pruning	the overlap qual is pushed down, where it used to be dropped	15;16;17;18;19	never	-
 range_pruning	range_pruning	the same for a containment probe far to its right	15;16;17;18;19	never	-
+rewrite_storage_oid	rewrite_storage_oid	a matview created WITH DATA leaves its storage row pointing at itself	15;16;17;18;19	never	-
 rewrite_storage_oid	rewrite_storage_oid	a type rewrite leaves the storage row pointing at the live table	15;16;17;18;19	2026-09-24	the rewrite hook does not retarget storage.relation_oid
+rewrite_storage_oid	rewrite_storage_oid	and REFRESH still leaves it pointing at itself	15;16;17;18;19	never	-
+rewrite_storage_oid	rewrite_storage_oid	premise: CREATE TABLE ... AS leaves its storage row pointing at itself	15;16;17;18;19	never	-
 rewrite_storage_oid	rewrite_storage_oid	premise: a table that was not rewritten still points at itself	15;16;17;18;19	never	-
+rewrite_storage_oid	rewrite_storage_oid	premise: the CREATE TABLE AS table holds them too	15;16;17;18;19	never	-
+rewrite_storage_oid	rewrite_storage_oid	premise: the matview holds the rows it was created with	15;16;17;18;19	never	-
+rewrite_storage_oid	rewrite_storage_oid	premise: the refreshed matview still holds every row	15;16;17;18;19	never	-
 rewrite_storage_oid	rewrite_storage_oid	premise: the rewritten table still holds every row	15;16;17;18;19	never	-
 rewrite_storage_oid	rewrite_storage_oid	premise: the storage row points at the live table before the rewrite	15;16;17;18;19	never	-
 rewrite_storage_oid	rewrite_storage_oid	premise: the table holds every inserted row	15;16;17;18;19	never	-

test/check_ledger_budget.txt

@@ -1177,4 +1177,4 @@ suites_not_covered 247
 # REBASED FROM 1bcfb92b ONTO 81ec3ddc, 32 commits, by the maintainer rather than
 # the author. The branch predates #1261, and three of its five CI reds were
 # fixed by the rebase alone rather than by any change here -- see the PR.
-checks_never_observed_red 1627
+checks_never_observed_red 1633

test/pytest/TESTS.md

@@ -6285,8 +6285,15 @@ THE ONE THING THIS HALF HAS TO DO THAT THE SHELL HALF DOES NOT is flush the stat
 
 Port of `rewrite_storage_oid.sh`. `ALTER COLUMN ... TYPE` writes `pgcolumnar.storage.relation_oid` as the transient relation and then drops it. The live table's OID no longer finds the row. The rows themselves stay readable.
 
+AND THE SAME DEFECT ON A SECOND STATEMENT (#1275). `CREATE MATERIALIZED VIEW ... AS` with data builds a transient, fills it and swaps, exactly as a rewriting `ALTER` does, so the storage row named a relation that no longer existed. Measured before the fix: the row named a dropped 16527 while the matview was 16523. `REFRESH MATERIALIZED VIEW` already repaired it, because that node type was in the repair gate; `CreateTableAsStmt` was not.
+
+THE `CREATE TABLE ... AS` ARM IS THE CONTROL THAT NARROWS THE FIX, and it is the reason the remedy is restricted to `objtype == OBJECT_MATVIEW` rather than applied to the parse node. Same node, no defect -- it fills the relation it created instead of swapping a transient in. Without that control the fix would reasonably have been written for `CreateTableAsStmt` as a whole, which is broader than anything measured asked for, and the premise now refuses the narrowing the day it stops being true.
+
+THE `REFRESH` ARM GUARDS THE PATH THAT ALREADY WORKED. It repaired this before the fix, through a different node type, so an arm there is what says the new arm did not displace it.
+
 ### Every test
 
 | test | what it holds |
 | --- | --- |
 | `test_rewrite_storage_oid` | the storage row matches the live regclass before the rewrite, the row count survives, an unrewritten table still matches itself, and the rewritten table's storage row matches the live regclass afterwards |
+| `test_a_matview_created_with_data_points_at_itself` | that the matview and the `CREATE TABLE AS` table both hold their rows, that `CREATE TABLE ... AS` points at itself (the control that narrows the fix), that a matview created `WITH DATA` points at itself, and that `REFRESH` still does |

test/pytest/expected_tests.txt

@@ -530,10 +530,17 @@ guard_tests 404
 # 1bcfb92b. Carrying that forward would have been a count measured on a tree
 # that no longer exists; 483 is measured on this one.
 # REBASED ONTO 5389b35a AFTER #1277 MERGED, AND THE NUMBER WAS COLLECTED RATHER
-# THAN ADDED UP. Both sides moved this key -- #1277 took it to 486 and this
+# THAN ADDED UP. Both sides moved this key -- #1277 took it to 486 and that
 # branch had 485 against the older base -- so git CONFLICTED, which is the loud
 # case this file's own history calls the lucky one. 486 + 1 is the arithmetic
-# and 487 is what `pytest --collect-only` over the 53 cluster files reports on
-# the merged tree. They agree here; the point is that the second one is evidence
-# and the first is a guess that happens to be right.
-cluster_tests 487
+# and 487 is what `pytest --collect-only` over the 53 cluster files reported on
+# the merged tree. They agreed; the point is that the second one is evidence and
+# the first is a guess that happens to be right.
+#
+# AND IT HAPPENED AGAIN IMMEDIATELY, WHICH IS THE POINT OF THE NOTE ABOVE.
+# #1278 and #1275 were both cut from 5389b35a and both moved this key to 487 for
+# DIFFERENT tests. #1278 merged first, so 487 is now main's value and #1275's
+# own test is not in it. Git conflicted again -- loudly, because the key moved
+# on both sides -- and the answer is neither branch's number. Collected on the
+# rebased tree, not added up.
+cluster_tests 488

test/pytest/test_analyze_function.py

@@ -851,29 +851,51 @@ def test_a_covering_projection_does_not_decide_which_columns_get_statistics(pgc_
         "pgcolumnar.analyze() still refuses a relation that has never been written",
     )
 
-    # AND THE BEHAVIOUR CHANGE IS ASSERTED, NOT LEFT TO BE DISCOVERED. A matview
-    # created WITH DATA has an orphaned relation_oid until its first REFRESH
-    # (#1275). Keyed on relation_oid this refused a matview holding rows;
-    # through the metapage it resolves. THAT IS NOT #1275 BEING FIXED -- the
-    # orphan is still in the catalog for every other reader.
+    # A MATVIEW CREATED WITH DATA, AND THIS ARM HAS BEEN SUPERSEDED ONCE ALREADY.
+    #
+    # #1276 added it to record a behaviour change: a WITH DATA matview had an
+    # orphaned relation_oid until its first REFRESH, so keyed on that column
+    # analyze() refused a matview holding rows and through the metapage it
+    # resolved. Its premise asserted the orphan was still there -- and #1275
+    # then removed the orphan, so the premise went red. That is the premise
+    # doing its job: it recorded a fact about the world and reddened when the
+    # world changed, rather than passing quietly under an arm that no longer
+    # meant anything.
+    #
+    # WITH THE ORPHAN GONE both routes resolve, so this shape no longer
+    # distinguishes them and is NOT evidence for #1276's fix -- the projection
+    # arms above are. What it still holds is that analyze() reaches a freshly
+    # created matview at all, which was an outright error before #1276, and
+    # that the two routes agree about WHICH storage.
     _exec(conn, "CREATE MATERIALIZED VIEW ap_mv USING pgcolumnar AS SELECT a, b, c FROM ap")
-    mv_by_reloid = int(
-        _one(conn, "SELECT count(*) FROM pgcolumnar.storage WHERE relation_oid = 'ap_mv'::regclass::oid")
+    by_reloid = _one(
+        conn,
+        "SELECT coalesce(min(storage_id)::text, 'none') FROM pgcolumnar.storage"
+        " WHERE relation_oid = 'ap_mv'::regclass::oid",
     )
-    mv_by_meta = int(
-        _one(
-            conn,
-            "SELECT count(*) FROM pgcolumnar.storage"
-            " WHERE storage_id = pgcolumnar.get_storage_id('ap_mv'::regclass)",
-        )
+    by_meta = _one(
+        conn,
+        "SELECT coalesce(min(storage_id)::text, 'none') FROM pgcolumnar.storage"
+        " WHERE storage_id = pgcolumnar.get_storage_id('ap_mv'::regclass)",
     )
     mv_state = _try_analyze(conn, "ap_mv")
-    print(f"-- a WITH DATA matview: by relation_oid={mv_by_reloid}, by metapage={mv_by_meta}, analyze() {mv_state}")
+    print(f"-- a WITH DATA matview: by relation_oid={by_reloid}, by metapage={by_meta}, analyze() {mv_state}")
 
-    expect.num(mv_by_reloid, 0, "premise: the matview's relation_oid still finds no storage row")
-    expect.num(mv_by_meta, 1, "premise: and its metapage still finds exactly one")
+    # THE IDS, NOT THEIR COUNTS. Two rows counting 1 each can still be two
+    # DIFFERENT storages, and that is the failure this arm would most want to
+    # see.
+    expect.text(
+        "none" if str(by_reloid) == "none" else "found",
+        "found",
+        "premise: the matview's relation_oid finds a storage row",
+    )
+    expect.text(
+        str(by_meta),
+        str(by_reloid),
+        "the two routes agree which storage a WITH DATA matview owns",
+    )
     expect.text(
         mv_state,
         "succeeded",
-        "pgcolumnar.analyze() now reaches a matview whose relation_oid is orphaned",
+        "pgcolumnar.analyze() reaches a matview created WITH DATA",
     )