fix: name storage_pkey on both storage_id scans (#1237)

CHANGELOG.md

@@ -336,6 +336,47 @@ true until the next version shipped.
 
 ### Fixed
 
+- Planning a columnar query sequentially scanned `pgcolumnar.storage` on its own
+  primary key (#1237).
+
+  Two readers key on `storage_id` and both passed `InvalidOid`, so both swept the
+  catalog sequentially on the one column `storage_pkey` is a UNIQUE btree over:
+
+  | reader | phase | reached from |
+  | --- | --- | --- |
+  | `PgColumnarGetSortedInfo` | planning | `pgcolumnar_sorted_pathkeys` |
+  | `PgColumnarCheckNativeFormatVersion` | execution, once per relation scanned | `columnar_reader.c:644` |
+
+  Unlike #1210 nothing had to be built and no decision about #1211 was involved:
+  the index exists and the key is exact.
+
+  ```
+    unfixed   count(*) idx=0 seq=1    join idx=0 seq=5
+    fixed     count(*) idx=1 seq=0    join idx=4 seq=1
+  ```
+
+  The four converted scans are `GetSortedInfo` twice at planning and
+  `CheckNativeFormatVersion` twice at execution. **THE RESIDUE IS
+  `pgcolumnar_written_stripe_row_limit`, ONCE PER COLUMNAR RELATION THAT REACHES
+  IT** (#1210, #1211), which keys on `relation_oid` and has no index to name.
+  One here because this query's qual is on one side of the join only;
+  @OffgridwithJD measures two where both sides reach it. `seq` becoming 1 is a
+  property of the query, not of the change.
+
+  WHICH IS WHY THE JOIN ARM ASSERTS `idx_scan >= 2` RATHER THAN `seq_scan == 0`.
+  Asserting zero would fail for a defect this change does not fix, and asserting
+  one would pin a number #1210 is expected to move.
+
+  The two shapes are separate arms because they reach different code. A no-qual
+  count never reaches the row-group-limit lookup, so its only storage access is
+  the format-version one and `seq_scan == 0` reads cleanly for that site alone.
+
+  HOW IT WAS FOUND: two instruments disagreeing, and both being right. An `elog`
+  inside `pgcolumnar_written_stripe_row_limit` counted 2 scans for a join;
+  `pg_stat_all_tables.seq_scan` on the catalog counted 4. An elog at one
+  function counts arrivals at that function; a counter on the relation counts
+  scans from any caller. The gap was the second reader.
+
 - Eleven suites built with their own `make`, so neither the build stamp (#536)
   nor the cross-major object check (#1219) protected them (#1220). They now route
   through `pgc_build_and_install`.

src/columnar_metadata.c

@@ -2315,6 +2315,7 @@ PgColumnarGetSortedInfo(uint64 storageId, int64 *firstGroup, int64 *lastGroup,
 	ScanKeyData key[1];
 	SysScanDesc scan;
 	HeapTuple	tuple;
+	Oid			storIdx;
 
 	*firstGroup = -1;
 	*lastGroup = -1;
@@ -2325,7 +2326,23 @@ PgColumnarGetSortedInfo(uint64 storageId, int64 *firstGroup, int64 *lastGroup,
 	tupdesc = RelationGetDescr(rel);
 	ScanKeyInit(&key[0], Anum_native_storage_storage_id, BTEqualStrategyNumber,
 				F_INT8EQ, Int64GetDatum((int64) storageId));
-	scan = systable_beginscan(rel, InvalidOid, false, NULL, 1, key);
+
+	/*
+	 * NAME THE INDEX (#1237). The key is storage_id, which storage_pkey is a
+	 * UNIQUE btree on, so this was a sequential scan of the catalog on the one
+	 * column it is indexed by -- 2 * pages per call, growing with the number of
+	 * columnar relations in the database.
+	 *
+	 * It matters because this is on the PLANNING path, not a maintenance one:
+	 * pgcolumnar_sorted_pathkeys reaches it at columnar_customscan.c:1542. The
+	 * comment above says "used by recluster's self-gate", which was true when
+	 * it was written and is no longer the whole truth.
+	 *
+	 * Unlike #1210's lookup on relation_oid, nothing has to be built and no
+	 * decision about #1211 is involved: the index exists and the key is exact.
+	 */
+	storIdx = pgcolumnar_index_oid("storage_pkey");
+	scan = systable_beginscan(rel, storIdx, OidIsValid(storIdx), NULL, 1, key);
 	tuple = systable_getnext(scan);
 	if (HeapTupleIsValid(tuple))
 	{
@@ -2392,11 +2409,25 @@ PgColumnarCheckNativeFormatVersion(uint64 storageId, const char *relName)
 	HeapTuple	tuple;
 	bool		found = false;
 	int32		formatVersion = 0;
+	Oid			storIdx;
 
 	ScanKeyInit(&key[0], Anum_native_storage_storage_id, BTEqualStrategyNumber,
 				F_INT8EQ, Int64GetDatum((int64) storageId));
 	/* NULL snapshot -> catalog snapshot, same as the other read-side scans. */
-	scan = systable_beginscan(rel, InvalidOid, false, NULL, 1, key);
+	/*
+	 * NAME THE INDEX (#1237), for the same reason as PgColumnarGetSortedInfo
+	 * above: storage_pkey is a UNIQUE btree on storage_id. This one is NOT on
+	 * the planning path -- measured by @OffgridwithJD, fmtver=0 in all five
+	 * planned shapes -- but it is reached ONCE PER COLUMNAR RELATION SCANNED at
+	 * execution, from columnar_reader.c:644, and a `count(*)` that pays nothing
+	 * at planning still pays this.
+	 *
+	 * Whether that cost MATTERS against actually reading the relation's data is
+	 * not claimed here and has not been measured. The scan is wrong either way:
+	 * it is sequential on an indexed primary key.
+	 */
+	storIdx = pgcolumnar_index_oid("storage_pkey");
+	scan = systable_beginscan(rel, storIdx, OidIsValid(storIdx), NULL, 1, key);
 
 	tuple = systable_getnext(scan);
 	if (HeapTupleIsValid(tuple))

test/catalog_plan_index.sh

@@ -73,4 +73,66 @@ check_num "planning probed pgcolumnar.projection through projection_pkey" \
 check_num "planning did not sequentially scan pgcolumnar.projection" \
 	"$prj_seq" "0"
 
+# ---- and pgcolumnar.storage, through storage_pkey (#1237) -------------------
+#
+# Two readers key on storage_id and both passed InvalidOid, so both scanned the
+# catalog sequentially on the one column storage_pkey is a UNIQUE btree over:
+#
+#     PgColumnarGetSortedInfo           PLANNING, via pgcolumnar_sorted_pathkeys
+#     PgColumnarCheckNativeFormatVersion EXECUTION, once per relation scanned
+#
+# THE TWO SHAPES ARE DIFFERENT ARMS BECAUSE THEY REACH DIFFERENT CODE, and a
+# single shape cannot tell them apart. Measured on the unfixed tree, scans of
+# pgcolumnar.storage per planned query by shape:
+#
+#     count(*), no qual                  0 at planning, 1 at execution
+#     qual on a plain column             1
+#     qual on a column with a projection 4
+#     two columnar relations, one qual   4   <- 2 limit lookups + 2 sorted lookups
+#
+# A `count(*)` never reaches the row-group-limit lookup, so the ONLY storage
+# access it makes is the format-version one. That makes seq_scan == 0 a clean
+# reading for that site and nothing else.
+#
+# THE JOIN IS MEASURED ON idx_scan RATHER THAN seq_scan, deliberately. Its two
+# remaining sequential scans come from pgcolumnar_written_stripe_row_limit,
+# which keys on relation_oid and has NO index to name -- that is #1210 and
+# #1211 and is not this change. Asserting seq_scan == 0 there would fail for a
+# defect this change does not claim to fix, and asserting seq_scan == 2 would
+# pin a number that #1210 is expected to move.
+q "CREATE TABLE plan_cat_j (id int) USING pgcolumnar;
+   INSERT INTO plan_cat_j SELECT g FROM generate_series(1,800) g;" >/dev/null
+
+storage_stat() {	# -> "idx_scan seq_scan"
+	q "SELECT coalesce(idx_scan,0)::text || ' ' || coalesce(seq_scan,0)::text
+		FROM pg_stat_all_tables
+		WHERE schemaname = 'pgcolumnar' AND relname = 'storage';"
+}
+
+q "SELECT pg_stat_reset();" >/dev/null
+q "SELECT count(*) FROM plan_cat;" >/dev/null
+q "SELECT pg_stat_force_next_flush();" >/dev/null
+st="$(storage_stat)"
+st_idx="${st%% *}"
+st_seq="${st##* }"
+echo "-- storage after count(*)  idx_scan=$st_idx seq_scan=$st_seq"
+
+check_num "premise: a no-qual count over a columnar table touched storage at all" \
+	"$(if [ "$((st_idx + st_seq))" -ge 1 ]; then echo 1; else echo 0; fi)" "1"
+check_num "a no-qual count did not sequentially scan pgcolumnar.storage" \
+	"$st_seq" "0"
+
+q "SELECT pg_stat_reset();" >/dev/null
+q "SELECT count(*) FROM plan_cat a JOIN plan_cat_j b ON a.id = b.id WHERE a.id > 0;" >/dev/null
+q "SELECT pg_stat_force_next_flush();" >/dev/null
+sj="$(storage_stat)"
+sj_idx="${sj%% *}"
+sj_seq="${sj##* }"
+echo "-- storage after a two-relation join  idx_scan=$sj_idx seq_scan=$sj_seq"
+
+check_num "premise: the join reached storage more than once" \
+	"$(if [ "$((sj_idx + sj_seq))" -ge 2 ]; then echo 1; else echo 0; fi)" "1"
+check_num "planning a join probed pgcolumnar.storage through storage_pkey" \
+	"$(if [ "$sj_idx" -ge 2 ]; then echo 1; else echo 0; fi)" "1"
+
 pgc_summary

test/check_ledger.tsv

@@ -20,11 +20,15 @@ capability_sweep	capability_sweep	premise: the shim makes pyarrow unimportable	1
 capability_sweep	capability_sweep	premise: the sweep is not in the population it sweeps	15;16;17;18;19	2026-09-23	the self-exclusion removed from pgc_cap_population, so the sweep enters its own population
 capability_sweep	capability_sweep	premise: the two derivations of the population name the same suites	15;16;17;18;19	never	-
 capability_sweep	capability_sweep	premise: while python itself still runs under the shim	15;16;17;18;19	never	-
+catalog_plan_index	catalog_plan_index	a no-qual count did not sequentially scan pgcolumnar.storage	15;16;17;18;19	2026-09-23	both storage_id scans reverted to InvalidOid, so the catalog is swept sequentially on its own primary key
+catalog_plan_index	catalog_plan_index	planning a join probed pgcolumnar.storage through storage_pkey	15;16;17;18;19	2026-09-23	both storage_id scans reverted to InvalidOid, so the catalog is swept sequentially on its own primary key
 catalog_plan_index	catalog_plan_index	planning did not sequentially scan pgcolumnar.options	15;16;17;18;19	2026-09-22	InvalidOid on the planner options and projection scans
 catalog_plan_index	catalog_plan_index	planning did not sequentially scan pgcolumnar.projection	15;16;17;18;19	2026-09-22	InvalidOid on the planner options and projection scans
 catalog_plan_index	catalog_plan_index	planning probed pgcolumnar.options through options_pkey	15;16;17;18;19	2026-09-22	InvalidOid on the planner options and projection scans
 catalog_plan_index	catalog_plan_index	planning probed pgcolumnar.projection through projection_pkey	15;16;17;18;19	2026-09-22	InvalidOid on the planner options and projection scans
+catalog_plan_index	catalog_plan_index	premise: a no-qual count over a columnar table touched storage at all	15;16;17;18;19	never	-
 catalog_plan_index	catalog_plan_index	premise: the filtered scan returned every row	15;16;17;18;19	never	-
+catalog_plan_index	catalog_plan_index	premise: the join reached storage more than once	15;16;17;18;19	never	-
 catalog_plan_index	catalog_plan_index	premise: the measured table holds its rows	15;16;17;18;19	never	-
 differential	differential	agg avg	15;16;17;18;19	never	-
 differential	differential	agg count	15;16;17;18;19	never	-

test/check_ledger_budget.txt

@@ -546,4 +546,52 @@ suites_not_covered 249
 # this shows; they are unattacked, and they need mutations aimed at plan shape,
 # the NULL handling and the baseline join rather than at the filter's verdict.
 # Saying which is the next reader's job, not an entry in this file.
-checks_never_observed_red 1519
+# 1519 -> 1521 for #1237's four new catalog_plan_index checks. Re-derived by
+# COUNTING on this tree:
+#
+#     awk -F'\t' '$5=="never"' test/check_ledger.tsv | wc -l   ->  1521
+#
+# with the premise beside it, because a pattern matching nothing counts 0 and
+# reads like a clean answer: 1606 rows total, 85 not `never`, 1521 + 85 == 1606.
+#
+# FOUR ROWS AND THE CENSUS MOVES BY TWO. Two carry a measured last-red from one
+# mutation, both `storage_id` scans reverted to `InvalidOid`:
+#
+#     a no-qual count did not sequentially scan pgcolumnar.storage   got [1] want [0]
+#     planning a join probed pgcolumnar.storage through storage_pkey got [0] want [1]
+#
+# The two that stay `never` are fixture premises: that a no-qual count reached
+# the catalog at all, and that the join reached it more than once. They exist so
+# the two arms above cannot pass over a measurement that never happened.
+#
+# RED BEFORE GREEN, WITH SOURCE AND BINARY IN STEP EACH TIME:
+#
+#     unfixed   count(*) idx=0 seq=1   join idx=0 seq=5    8 passed + 2 failed
+#     fixed     count(*) idx=1 seq=0   join idx=4 seq=1   10 passed + 0 failed
+#
+# and the pytest twin separately, because the twins' regimes diverge: unfixed
+# rc=1 with `storage after count(*) idx_scan=0 seq_scan=1`, fixed rc=0 with 10
+# of 10. The twin aborts at its first failure where the shell suite runs on, so
+# it reports 8 checks against the shell's 10 -- a divergence in the FAILURE
+# output, not in the names.
+#
+# THE MAJOR SET WAS DERIVED FROM FIVE RUNS against DEFAULT_CONFIGS read out of
+# run_all_versions.sh: rc=0 and 10 passed + 0 failed on pg15, pg16, pg17, pgsql
+# (the runner's PG18) and pg19.
+#
+# FIVE INSTRUMENT FAILURES BEFORE THE FIRST MEASUREMENT, none of them about the
+# change, and one defect behind all five. Each time the run produced no verdict
+# and each time a grep written for the expected answer hid the cause:
+#
+#     a FATAL matching none of PASS|FAIL|accounting, read as "ran, printed nothing"
+#     a raw `make install`, which does NOT write the source stamp -- only
+#       pgc_build_and_install does -- so the freshness gate refused (#1230)
+#     a harness build as `postgres` into a tree a root-run `make` had left
+#       root-owned, dying on Permission denied writing a .d file
+#     a grep for `storage` matching the TEST FILE'S OWN COMMENTS echoed back
+#
+# `head -12` on the log settled it in one go. READ THE TAIL, THEN FILTER ONCE
+# YOU KNOW WHAT IS THERE.
+#
+# suites_not_covered does NOT move: catalog_plan_index is already covered.
+checks_never_observed_red 1521

test/pytest/test_catalog_plan_index.py

@@ -101,3 +101,78 @@ def test_catalog_plan_index(pgc_cluster, pgc_conn, expect):
         0,
         "planning did not sequentially scan pgcolumnar.projection",
     )
+
+    # ---- and pgcolumnar.storage, through storage_pkey (#1237) --------------
+    #
+    # Two readers key on storage_id and both passed InvalidOid, so both scanned
+    # the catalog sequentially on the one column storage_pkey is a UNIQUE btree
+    # over: PgColumnarGetSortedInfo at PLANNING (via pgcolumnar_sorted_pathkeys)
+    # and PgColumnarCheckNativeFormatVersion once per relation scanned at
+    # EXECUTION.
+    #
+    # TWO SHAPES BECAUSE THEY REACH DIFFERENT CODE. A no-qual count never
+    # reaches the row-group-limit lookup, so its only storage access is the
+    # format-version one and seq_scan == 0 reads cleanly. The join is measured
+    # on idx_scan instead: its remaining sequential scan comes from
+    # pgcolumnar_written_stripe_row_limit, which keys on relation_oid and has
+    # no index to name -- that is #1210 and #1211, not this change. Measured
+    # unfixed, join idx=0 seq=5; fixed, idx=4 seq=1, and the 1 is that lookup.
+    #
+    # Its own cursor and its own reader connection, like the arms above: the
+    # session that wrote the rows must not be the session being measured.
+    with pgc_conn.cursor() as cur:
+        cur.execute("CREATE TABLE planner_join (n bigint) USING pgcolumnar")
+        cur.execute(
+            f"INSERT INTO planner_join SELECT g FROM generate_series(1,{ROWS}) g"
+        )
+        cur.execute("SELECT pg_stat_reset()")
+
+    reader = psycopg.connect(pgc_cluster.dsn(), autocommit=True)
+    try:
+        with reader.cursor() as cur:
+            cur.execute(f'SET search_path TO "{schema}", public')
+            cur.execute("SELECT count(*) FROM planner_opts")
+            cur.execute("SELECT pg_stat_force_next_flush()")
+    finally:
+        reader.close()
+
+    st_idx, st_seq = _stats(pgc_conn, "storage")
+    print(f"-- storage after count(*) idx_scan={st_idx} seq_scan={st_seq}")
+    expect.at_least(
+        st_idx + st_seq,
+        1,
+        "premise: a no-qual count over a columnar table touched storage at all",
+    )
+    expect.num(
+        st_seq,
+        0,
+        "a no-qual count did not sequentially scan pgcolumnar.storage",
+    )
+
+    with pgc_conn.cursor() as cur:
+        cur.execute("SELECT pg_stat_reset()")
+
+    reader = psycopg.connect(pgc_cluster.dsn(), autocommit=True)
+    try:
+        with reader.cursor() as cur:
+            cur.execute(f'SET search_path TO "{schema}", public')
+            cur.execute(
+                "SELECT count(*) FROM planner_opts a "
+                "JOIN planner_join b ON a.n = b.n WHERE a.n >= 1"
+            )
+            cur.execute("SELECT pg_stat_force_next_flush()")
+    finally:
+        reader.close()
+
+    sj_idx, sj_seq = _stats(pgc_conn, "storage")
+    print(f"-- storage after a two-relation join idx_scan={sj_idx} seq_scan={sj_seq}")
+    expect.at_least(
+        sj_idx + sj_seq,
+        2,
+        "premise: the join reached storage more than once",
+    )
+    expect.at_least(
+        sj_idx,
+        2,
+        "planning a join probed pgcolumnar.storage through storage_pkey",
+    )