test: stats() and sort_status() refuse a heap table for privilege before reading its kind (#1657 follow-up)

test/check_ledger.tsv

@@ -7103,7 +7103,9 @@ sort_status	sort_status	unsorted table reports every row appended	15;16;17;18;19
 sort_status	sort_status	unsorted table reports no sorted groups	15;16;17;18;19	2026-10-20	sort_status counts every group as sorted when the table has no mark
 sort_status	sort_status	unsorted table reports no sorted rows	15;16;17;18;19	2026-10-20	sort_status counts every group as sorted when the table has no mark
 sort_status_privilege	sort_status_privilege	a role merely GRANTed select can read it too	15;16;17;18;19	2026-10-20	sort_status back to invoker rights, so the owner is false-denied on the catalogs (#608)
+sort_status_privilege	sort_status_privilege	a role with no privilege on a heap table is refused for privilege, not told it is not columnar	15;16;17;18;19	2026-10-22	pgcolumnar--1.0-alpha5.sql: sort_status() reads the storage id before the caller check (the two lines swapped)
 sort_status_privilege	sort_status_privilege	a role with no privilege on the table is refused	15;16;17;18;19	2026-10-20	sort_status back to invoker rights, so the owner is false-denied on the catalogs (#608);sort_status no longer checks the caller's SELECT privilege (require_caller_select removed)
+sort_status_privilege	sort_status_privilege	a superuser asking about a heap table is told it is not columnar	15;16;17;18;19	never	-
 sort_status_privilege	sort_status_privilege	a superuser still reads sort status	15;16;17;18;19	never	-
 sort_status_privilege	sort_status_privilege	and the refusal names the TABLE, not a catalog table it never asked about	15;16;17;18;19	2026-10-20	sort_status back to invoker rights, so the owner is false-denied on the catalogs (#608);sort_status no longer checks the caller's SELECT privilege (require_caller_select removed)
 sort_status_privilege	sort_status_privilege	premise: none of these roles is a superuser	15;16;17;18;19	never	-
@@ -7199,6 +7201,21 @@ statement_self_visibility	statement_self_visibility	and it read only the rows th
 statement_self_visibility	statement_self_visibility	and the table holds them once, as heap does	15;16;17;18;19	2026-10-15	a serial scan re-reads its row-group list when it reaches the end, so it picks up groups its own statement flushed (src/columnar_reader.c pgcolumnar_next_group_index)
 statement_self_visibility	statement_self_visibility	in one transaction, an INSERT, an UPDATE of every row and an INSERT SELECT run to completion	15;16;17;18;19	2026-10-15	a serial scan re-reads its row-group list when it reaches the end, so it picks up groups its own statement flushed (src/columnar_reader.c pgcolumnar_next_group_index)
 statement_self_visibility	statement_self_visibility	the INSERT flushed row groups while its scan was still reading	15;16;17;18;19	2026-10-15	a serial scan re-reads its row-group list when it reaches the end, so it picks up groups its own statement flushed (src/columnar_reader.c pgcolumnar_next_group_index)
+stats_privilege	stats_privilege	a role merely GRANTed select can read them too	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	a role with no privilege on a heap table is refused for privilege, not told it is not columnar	15;16;17;18;19	2026-10-22	pgcolumnar--1.0-alpha5.sql: stats() reads the storage id before the caller check (the two lines swapped)
+stats_privilege	stats_privilege	a role with no privilege on the table is refused	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	a superuser asking about a heap table is told it is not columnar	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	a superuser still reads stats	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	and the refusal names the TABLE, not a catalog table it never asked about	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: none of these roles is a superuser	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: t_stowner really owns the table	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: the catalog tables are NOT readable by these roles, so a GRANT is not the fix	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: the granted role can open a session	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: the no-privilege role can open a session	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: the no-privilege role cannot read it by ordinary SQL	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: the owner can open a session	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	premise: the owner can read its own table by ordinary SQL	15;16;17;18;19	never	-
+stats_privilege	stats_privilege	the OWNER of the table can read its stats	15;16;17;18;19	never	-
 storage_row_repair	storage_row_repair	REFRESH MATERIALIZED VIEW repairs the view's row	15;16;17;18;19	2026-10-12	PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row
 storage_row_repair	storage_row_repair	a same-type ALTER COLUMN ... USING repairs the table's row	15;16;17;18;19	2026-10-12	PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row
 storage_row_repair	storage_row_repair	after both repairs the notes' query finds nothing	15;16;17;18;19	2026-10-12	PgColumnarRetargetStorageRelation returns at once, so no repair repoints a storage row;the release notes' query is edited from NOT EXISTS to EXISTS

test/check_ledger_budget.txt

@@ -15,7 +15,7 @@
 #
 #   Adding a check to a suite that is already covered does not move it, which is
 #   what makes it safe to bound.
-suites_not_covered 28
+suites_not_covered 27
 #
 # checks_never_observed_red -- A CENSUS. NOT a ceiling, and it must not become
 #   one.
@@ -7039,4 +7039,21 @@ suites_not_covered 28
 #     never         4545 - 25 = 4520
 #     dated         3418 + 25 = 3443
 #     4520 + 3443 == 7963
-checks_never_observed_red 4520
+# #1657 follow-up, stats() and sort_status() refuse before they read the storage id
+# (OffgridwithJD): 4520 -> 4535, and suites_not_covered 28 -> 27. get_storage_id
+# checks SELECT with GetUserId(), which inside these SECURITY DEFINER functions is the
+# definer, so it passes for anyone and says "not a columnar table" for a heap table.
+# Only the explicit caller check refuses, and only while it runs first. With the two
+# lines swapped ("sidfirst"), both privilege suites stayed 13/13 in shell and twin on
+# main, because on their columnar table the caller check still refused afterwards.
+# Each suite gains a heap table no test role may read: refusing it for privilege is
+# dated by sidfirst, red by the same name in both twins, and the superuser control
+# ("not a columnar table") arrives never. sort_status_privilege already had its rows
+# and gains these 2. stats_privilege had none, so it enters whole with its 15 checks
+# from five-major runs (all green on 15-19, shell and twin alike): 1 dated, 14 never.
+#
+#     ledger rows   7963 + 17 = 7980
+#     never         4520 + 15 = 4535
+#     dated         3443 + 2 = 3445
+#     4535 + 3445 == 7980
+checks_never_observed_red 4535

test/pytest/TESTS.md

@@ -3931,6 +3931,8 @@ property this suite tests, so `SET ROLE` would assert it away. Each role connect
 | `test_the_premises_each_role_is_what_the_suite_assumes` | each premise run BY the role it is about |
 | `test_who_may_read_the_stats` | owner, GRANTed reader and superuser all succeed |
 | `test_a_role_with_no_privilege_is_refused` | the bar: 42501 **and** the table named |
+| `test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read_by_stats` | a heap table the caller may not read is refused for privilege, not called non-columnar: the caller check runs before the storage id is read |
+| `test_a_superuser_is_told_a_heap_table_is_not_columnar_by_stats` | control: a caller who may read it is told "not a columnar table" |
 
 ### A helper that turned a driver detail into a product claim
 
@@ -8975,6 +8977,8 @@ A call made as a role reads the first column of its first row, or its failure as
 | `test_a_role_with_no_privilege_is_refused_its_sort_status` | a role with no privilege on the table is refused |
 | `test_the_refusal_names_the_table` | and the refusal names the TABLE, not a catalog table it never asked about |
 | `test_a_superuser_still_reads_the_sort_status` | a superuser still reads sort status |
+| `test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read` | a role with no privilege on a heap table is refused for privilege, not told it is not columnar |
+| `test_a_superuser_is_told_a_heap_table_is_not_columnar` | a superuser asking about a heap table is told it is not columnar |
 ## 210. test_bloom_lazy.py: a scan reads the bloom filters of the groups and columns it examines
 
 The pytest twin of `test/bloom_lazy.sh` (ported for #1341), every check by the shell's name. Two tables differ only in how many groups a one-group equality scan skips (6 and 30), so the bloom blocks they fetch, read from `pg_stat_get_blocks_fetched` on `pgcolumnar.bloom`, must not grow with the skipped groups (#310); on a wide table where no group is excluded, six equality predicates must fetch more than one (#314). The answers are compared with a heap mirror, with and without bloom filters.

test/pytest/expected_tests.txt

@@ -1186,6 +1186,8 @@ guard_tests 839
 # 22 checks in 22 tests, by collection over the cluster files.
 # cluster_tests 2670 -> 2677 for #1637's test_catalog_filter_reads.py, the twin of catalog_filter_reads.sh:
 # 7 tests, one per check, by collection over the cluster files.
+# cluster_tests 4227 -> 4231 for the #1657 follow-up: a heap-table refusal and its control in
+# test_stats_privilege.py and test_sort_status_privilege.py, by collection over the cluster files.
 # cluster_tests 4176 -> 4200 for #1339's hilbert batch: test_hilbert_cluster.py's S1 and (a2) tests split
 # into one check per test (S1 8 -> 24, (a2) 1 -> 7) and two daemon tests on a decayed zorder table, by collection.
 # cluster_tests 2748 -> 2753 for #1706's five timestamp-flag tests in test_native_parquet_schema.py, by collection.
@@ -1208,4 +1210,4 @@ guard_tests 839
 # 54 tests, one per check, by collection over the cluster files.
 # cluster_tests 4225 -> 4226 for #1753's arm 5 in test_parallel_flush_optin.py (folded into #1746):
 # one structural test, by collection over the cluster files.
-cluster_tests 4227
+cluster_tests 4231

test/pytest/test_sort_status_privilege.py

@@ -61,6 +61,8 @@ def ss(pgc_cluster):
             conn.execute("ALTER TABLE ss_t OWNER TO t_ssowner")
             conn.execute("REVOKE ALL ON ss_t FROM PUBLIC")
             conn.execute("GRANT SELECT ON ss_t TO t_sssel")
+            conn.execute("CREATE TABLE ss_h (id int)")   # a heap table, readable by no test role
+            conn.execute("REVOKE ALL ON ss_h FROM PUBLIC")
             owner = conn.execute("SELECT pg_get_userbyid(relowner) FROM pg_class WHERE relname='ss_t'").fetchone()[0]
             nosuper = conn.execute("SELECT bool_and(NOT rolsuper) FROM pg_roles WHERE rolname LIKE 't_ss%'").fetchone()[0]
             superuser = conn.execute("SELECT count(*) FROM pgcolumnar.sort_status('ss_t')").fetchone()[0]
@@ -80,6 +82,10 @@ def ss(pgc_cluster):
             "none refused": _refused(none_status, "permission denied for table"),
             "none names table": _refused(none_status, "ss_t"),
             "super status": superuser,
+            "none heap": _refused(_as(dsn, "t_ssnone", "SELECT count(*) FROM pgcolumnar.sort_status('ss_h')"),
+                                  "permission denied for table ss_h"),
+            "super heap": _refused(_as(dsn, "postgres", "SELECT count(*) FROM pgcolumnar.sort_status('ss_h')"),
+                                   'relation "ss_h" is not a columnar table'),
         }
     finally:
         with psycopg.connect(pgc_cluster.dsn(), autocommit=True) as a:
@@ -143,3 +149,14 @@ def test_the_refusal_names_the_table(ss, expect):
 
 def test_a_superuser_still_reads_the_sort_status(ss, expect):
     expect.num(ss["super status"], 1, "a superuser still reads sort status")
+
+
+def test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read(ss, expect):
+    """get_storage_id checks the DEFINER inside this SECURITY DEFINER function, so only
+    the explicit caller check refuses, and only while it runs first (#1657 review)."""
+    expect.num(ss["none heap"], 1,
+               "a role with no privilege on a heap table is refused for privilege, not told it is not columnar")
+
+
+def test_a_superuser_is_told_a_heap_table_is_not_columnar(ss, expect):
+    expect.num(ss["super heap"], 1, "a superuser asking about a heap table is told it is not columnar")

test/pytest/test_stats_privilege.py

@@ -75,6 +75,9 @@ def _fixture(cur):
     cur.execute(f"ALTER TABLE st_t OWNER TO {OWNER}")
     cur.execute("REVOKE ALL ON st_t FROM PUBLIC")
     cur.execute(f"GRANT SELECT ON st_t TO {SEL}")
+    cur.execute("DROP TABLE IF EXISTS st_h")
+    cur.execute("CREATE TABLE st_h (id int)")   # a heap table, readable by no test role
+    cur.execute("REVOKE ALL ON st_h FROM PUBLIC")
     return schema
 
 
@@ -170,3 +173,28 @@ def test_a_role_with_no_privilege_is_refused(pgc_cluster, pgc_conn, expect):
     expect.contains(
         str(err), "st_t",
         "and the refusal names the TABLE, not a catalog table it never asked about")
+
+
+def _says(err, text):
+    """-> 1 when the statement failed with `text` in its message, else 0, as the shell's grep -c."""
+    return 1 if err is not None and text in str(err) else 0
+
+
+def test_a_heap_table_is_refused_for_privilege_before_its_kind_is_read_by_stats(pgc_cluster, pgc_conn, expect):
+    """get_storage_id checks SELECT with GetUserId(), the DEFINER here, so it passes for
+    anyone and then says "not a columnar table". Only the explicit caller check refuses,
+    and only while it runs first: with the two swapped (#1657 review) every arm above
+    stays green, because on st_t the caller check still refuses afterwards."""
+    with pgc_conn.cursor() as cur:
+        schema = _fixture(cur)
+    _, err = _as(pgc_cluster, NONE, "SELECT count(*) FROM pgcolumnar.stats('st_h')", schema)
+    expect.num(_says(err, "permission denied for table st_h"), 1,
+               "a role with no privilege on a heap table is refused for privilege, not told it is not columnar")
+
+
+def test_a_superuser_is_told_a_heap_table_is_not_columnar_by_stats(pgc_cluster, pgc_conn, expect):
+    with pgc_conn.cursor() as cur:
+        schema = _fixture(cur)
+    _, err = _as(pgc_cluster, "postgres", "SELECT count(*) FROM pgcolumnar.stats('st_h')", schema)
+    expect.num(_says(err, 'relation "st_h" is not a columnar table'), 1,
+               "a superuser asking about a heap table is told it is not columnar")

test/sort_status_privilege.sh

@@ -42,6 +42,8 @@ psql_run "SELECT pgcolumnar.vacuum_sorted('ss_t','ts');"   -- give it a sorted r
 psql_run "ALTER TABLE ss_t OWNER TO t_ssowner;"
 psql_run "REVOKE ALL ON ss_t FROM PUBLIC;"
 psql_run "GRANT SELECT ON ss_t TO t_sssel;"
+psql_run "CREATE TABLE ss_h (id int);"   # a heap table, readable by no test role
+psql_run "REVOKE ALL ON ss_h FROM PUBLIC;"
 
 as() { env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U "$1" \
 	-d "$PGC_DB" -At -v ON_ERROR_STOP=0 -c "$2" 2>&1 | head -1; }
@@ -75,6 +77,16 @@ check "a role with no privilege on the table is refused" \
 check "and the refusal names the TABLE, not a catalog table it never asked about" \
 	"$(as t_ssnone "SELECT count(*) FROM pgcolumnar.sort_status('ss_t');" | grep -c 'ss_t')" "1"
 
+# ---- the order: privilege before anything else ------------------------------
+#
+# As in stats_privilege.sh: get_storage_id checks the DEFINER, so only the explicit
+# caller check refuses, and only while it runs first ("sidfirst" swaps them and
+# every check above stays green). A heap table must be refused for privilege.
+check "a role with no privilege on a heap table is refused for privilege, not told it is not columnar" \
+	"$(as t_ssnone "SELECT count(*) FROM pgcolumnar.sort_status('ss_h');" | grep -c 'permission denied for table ss_h')" "1"
+check "a superuser asking about a heap table is told it is not columnar" \
+	"$(as postgres "SELECT count(*) FROM pgcolumnar.sort_status('ss_h');" | grep -c 'relation "ss_h" is not a columnar table')" "1"
+
 # ---- the superuser path still works ----------------------------------------
 check_num "a superuser still reads sort status" \
 	"$(q "SELECT count(*) FROM pgcolumnar.sort_status('ss_t');")" "1"

test/stats_privilege.sh

@@ -51,6 +51,8 @@ psql_run "INSERT INTO st_t SELECT g, 'v'||g FROM generate_series(1,$ROWS) g;"
 psql_run "ALTER TABLE st_t OWNER TO t_stowner;"
 psql_run "REVOKE ALL ON st_t FROM PUBLIC;"
 psql_run "GRANT SELECT ON st_t TO t_stsel;"
+psql_run "CREATE TABLE st_h (id int);"   # a heap table, readable by no test role
+psql_run "REVOKE ALL ON st_h FROM PUBLIC;"
 
 as() { env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$PGC_PORT" -U "$1" \
 	-d "$PGC_DB" -At -v ON_ERROR_STOP=0 -c "$2" 2>&1 | head -1; }
@@ -101,6 +103,20 @@ check "a role with no privilege on the table is refused" \
 check "and the refusal names the TABLE, not a catalog table it never asked about" \
 	"$(as t_stnone "SELECT count(*) FROM pgcolumnar.stats('st_t');" | grep -c 'st_t')" "1"
 
+# ---- the order: privilege before anything else ------------------------------
+#
+# get_storage_id checks SELECT with GetUserId(), which inside this SECURITY DEFINER
+# function is the definer, so it passes for anyone and then says "not a columnar
+# table" for a heap table. Only the explicit caller check refuses, and only while
+# it runs FIRST: with the two lines swapped (#1657 review, "sidfirst") every check
+# above stays green, because on st_t the caller check still refuses afterwards.
+# A heap table the caller may not read must be refused for privilege; the control
+# shows the same call does say "not a columnar table" to a caller who may read it.
+check "a role with no privilege on a heap table is refused for privilege, not told it is not columnar" \
+	"$(as t_stnone "SELECT count(*) FROM pgcolumnar.stats('st_h');" | grep -c 'permission denied for table st_h')" "1"
+check "a superuser asking about a heap table is told it is not columnar" \
+	"$(as postgres "SELECT count(*) FROM pgcolumnar.stats('st_h');" | grep -c 'relation "st_h" is not a columnar table')" "1"
+
 # ---- the superuser path still works ----------------------------------------
 check_num "a superuser still reads stats" "$(q "SELECT count(*) FROM pgcolumnar.stats('st_t');")" "1"