import_parquet segfaults on a partitioned columnar table: the columnar check is absent, so #1261 does not reach it

[OffgridwithJD]

What

pgcolumnar.import_parquet() on a partitioned table with relam = pgcolumnar segfaults the backend and restarts the cluster. This is not
#1259's class and #1261 does not fix it: import_parquet never asks whether its
target is columnar at all.

  LOG:  server process (PID 3328813) was terminated by signal 11: Segmentation fault
  DETAIL:  Failed process was running: SELECT pgcolumnar.import_parquet('pp'::regclass, '/tmp/v2.parquet');
  LOG:  all server processes terminated; reinitializing
  LOG:  database system was not properly shut down; automatic recovery in progress

Signal 11, not an assert — so a production build is affected

#1259's family is an Assert that a non-assert build compiles out, leaving a
wrong answer. This one is a segfault, and I measured both:

  /usr/local/pg17a   PostgreSQL 17.6   --enable-cassert       signal 11
  /usr/local/pg18n   PostgreSQL 18     cassert flags: 0       signal 11

Neither run produced a failed Assert line — traps before = 0, traps after = 0. The crash is in ordinary code.

Reproduction, with both controls

  PREMISE file bytes=[757]
  PREMISE parent relkind=[p]  relfilenode=[0]
  CONTROL alive before = [up]

  import_parquet(PARENT, valid file)   rc=2   cluster DOWN, signal 11

  LEAF control       import_parquet('pp1', same file)   rc=0   imported 50 rows
  SIBLING control    import_arrow('pp',  valid file)    rc=1
                     ERROR: relation "pp" is not a columnar table

The leaf control says the call works and the file is good. The sibling
control is the sharper one: import_arrow on the same parent with the same
shape refuses cleanly, so the refusal is something this code could have done and
does not.

  CREATE TABLE src (a int, b text) USING pgcolumnar;
  INSERT INTO src SELECT g,'x'||g FROM generate_series(1,50) g;
  CREATE TABLE pp (a int, b text) PARTITION BY RANGE (a) USING pgcolumnar;
  CREATE TABLE pp1 PARTITION OF pp FOR VALUES FROM (0) TO (100);
  SELECT pgcolumnar.export_parquet('src'::regclass, '/tmp/v.parquet');
  SELECT pgcolumnar.import_parquet('pp'::regclass, '/tmp/v.parquet');   -- SIGSEGV

Cause: the check is absent, not wrong

  grep -c PgColumnarIsColumnarRelation src/columnar_parquet_reader.c   ->  0

pgcolumnar_import_parquet checks pg_read_server_files, then ACL_INSERT,
then RLS, then resolves the path, then table_open(relid, RowExclusiveLock) and
proceeds. It never asks whether the relation is columnar, so it is not one of
the 31 call sites #1261 changed and the predicate fix cannot reach it.

Swept across the tree, it is the only file in that position:

  file                        entrypoints  getarg_oid(0)  predicate uses
  columnar_arrow.c                 2            2              2
  columnar_parallel_copy.c         2            1              1
  columnar_parallel_export.c       1            1              2
  columnar_parquet.c               1            1              1
  columnar_parquet_reader.c        5            1              0     <-
  columnar_projection.c            4            4              5
  columnar_vacuum.c               14           11             10
  columnar_visibilitymap.c         2            2              2

columnar_vacuum.c at 11 against 10 is a count and not a proof, so it is worth
a look rather than a claim.

Who can reach it

Not an ordinary user. import_parquet requires superuser or
pg_read_server_files, plus INSERT on the target:

  if (!has_privs_of_role(GetUserId(), ROLE_PG_READ_SERVER_FILES))
      ereport(ERROR, ... "must be superuser or a member of the
                          pg_read_server_files role to read a server file");

So the population is roles a deployment has deliberately granted file-reading to
— narrower than #1259's "grant USAGE on the schema", and still a role that is
not superuser taking the cluster down.

Found how

Following up my own caveat on #1259 that survived meant "did not abort" rather
than "correct". The earlier 22-probe recorded import_parquet as surviving —
because the file did not exist, so it failed at the open before reaching the
crash. It only appears when the path resolves, which is why a sweep that reuses
one fixture misses it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP

[jdatcmd]

Reproduced independently on main at 00d3529c, which already carries #1261.
Confirming this is outside that class rather than a survivor of it.

  premise: parent relkind=[p] relfilenode=[0] rows=[50]
  premise: the parquet file exists = [yes]      <- exported from the LEAF first
  premise: the arrow   file exists = [yes]

  CONTROL import_parquet on the LEAF     alive=1    sig=[]           out=[50]
  SIBLING import_arrow  on the PARENT    alive=1    sig=[]           refused
  SUBJECT import_parquet on the PARENT   alive=NO   sig=[signal 11]

The leaf control returns 50, so the call works. The sibling refuses the
identical shape. Only import_parquet on the parent takes the cluster down.

The sweep, re-derived here

I resolved every regclass-first SQL entry point from its AS 'MODULE_PATHNAME', '<symbol>' clause, mapped each symbol to the file defining it, and counted
predicate uses per file:

  columnar_arrow.c              2   export_arrow, import_arrow
  columnar_parallel_copy.c      1   parallel_copy
  columnar_parallel_export.c    2   parallel_export_parquet
  columnar_parquet.c            1   export_parquet
  columnar_parquet_reader.c     0   import_parquet          <-- ZERO
  columnar_projection.c         5   add_projection, drop_projection, ...
  columnar_vacuum.c            10   cluster, compact, recluster, ...
  columnar_visibilitymap.c      2   vm_is_visible, vm_selftest

Agrees with @OffgridwithJD: one file, one entry point, no predicate. It was
never one of #1259's 31 call sites, so #1261 could not have reached it.

On the behaviour question, and I think it is narrower than it looks

Refuse, matching the sibling. The shape is already written twice in
columnar_arrow.c:

	if (!PgColumnarIsColumnarRelation(relid))
	{
		table_close(rel, RowExclusiveLock);
		ereport(ERROR,
				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
				 errmsg("relation \"%s\" is not a columnar table", ...

Fanning out to leaves is a feature, not this fix. parallel_export_parquet
fans out on export, where the parent's contents are well defined. Import is
not symmetric: routing rows to partitions is tuple routing, which is a new
behaviour with its own questions — what happens to a row matching no partition,
whether the import is atomic across leaves, what import_parquet should return.
None of that belongs in a segfault fix, and shipping it here would make the
crash fix unreviewable.

The narrow argument for refusing: export_parquet refuses a parent,
import_arrow refuses a parent, and import_parquet is the fourth corner of
that square. Making it the one that behaves differently would need a reason, and
the reason would be the feature above.

Taking this

I will take the fix, since #1259 is mine and keeping the crash class together is
worth more than the context @OffgridwithJD has on the reproduction — which is
now written down here anyway.

The arm has to build the file first. Their 22-entry sweep recorded this as
survived rc=1 because export had been refused on the parent, so the import
failed at the open before reaching the crash. A fixture shared across entry
points hides exactly this, and a test that skips the export step would pass
against the unfixed build.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XiFn3HteTXnGdRiA2xDP2n

[OffgridwithJD]

Closing my own loose end: columnar_vacuum.c's 11-vs-10 is not a gap

The table in this issue flagged columnar_vacuum.c at 11 PG_GETARG_OID(0)
against 10 predicate uses, and I called it "a count and not a proof". Resolved:
there is no unguarded entry point there. The count was an artifact of
counting per function instead of per call chain.

What the difference actually was

  entry points in the file                 14
  taking a regclass first argument         10 by my test, 14 in the SQL
  of those, no predicate in the body        5

Every one of the five is accounted for, and none is a gap:

Four delegate, through two levels:

  pgcolumnar_cluster           -> cluster_verb    -> cluster_verb_validate
  pgcolumnar_cluster_hilbert   -> cluster_verb    -> cluster_verb_validate
  pgcolumnar_recluster         -> recluster_verb  -> cluster_verb_validate
  pgcolumnar_recluster_hilbert -> recluster_verb  -> cluster_verb_validate

cluster_verb itself has 0 predicate uses, which is why a per-function scan
reports it bare; cluster_verb_validate has 1. They also never call
PG_GETARG_OID(0) — they pass fcinfo down — so a scan keyed on that argument
misses them as regclass entry points entirely. That is the same
count-is-not-a-property shape as reading pronargs for vacuum_sorted.

This is also why cluster crashed before #1261 and survives after, despite
never naming the predicate itself.

One needs no predicate at all:

  pgcolumnar_require_caller_select(PG_FUNCTION_ARGS)
  {
      Oid relid = PG_GETARG_OID(0);
      AclResult ac = pg_class_aclcheck(relid, GetOuterUserId(), ACL_SELECT);
      if (ac != ACLCHECK_OK)
          aclcheck_error(ac, OBJECT_TABLE, get_rel_name(relid));
      PG_RETURN_VOID();
  }

It performs an ACL check and returns. It never opens storage, so there is
nothing for a storage predicate to protect — which is also why it appears as
survived in this issue's nine for a reason that is sound rather than vacuous.

So import_parquet really was the only one

The sweep's conclusion stands and is now stronger:
src/columnar_parquet_reader.c was the sole file with a regclass entry point
reaching storage without ever asking the question, and #1264 closed it.

Recording the negative because I raised the number in public and a count left
hanging reads as an open defect.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MpajdQbkVJ9ey1XyYHcikP