Parallel bulk ingest: pgcolumnar.parallel_copy (#300)

[ChronicallyJD]

Parallel bulk ingest — design + incremental build (#300)

Follows the measurement on #300 (parallel ingest scales ~7.4× to the physical core count; a real 8-way 100M load is 74.9s, beating a single-COPY heap load). This draft opens with the design doc (design/PARALLEL_COPY_PLAN.md) and will grow the implementation in the phased slices it lists, each a compiling, gated commit.

pgcolumnar.parallel_copy(target, path, workers, mode => 'atomic'|'staging', ...) fans core COPY across N background workers, each over a line-aligned byte range of the file — so parse/NULL/quote/encoding semantics stay exactly core COPY's, no looser parser to get wrong (the correctness risk the issue flags). Two admin-selectable all-or-nothing modes, per the maintainer's steer:

• atomic — workers PREPARE TRANSACTION into the target; coordinator commits-all or rolls-back-all (needs max_prepared_transactions). True COPY-like atomicity into a populated table.
• staging — workers load partitions of a partitioned target; coordinator ATTACHes them (metadata-only). No 2PC requirement.

Open questions I'd like a steer on before building the mechanism-dependent parts (details in the doc):

1. Mechanism: native dynamic background workers (my default — no dependency, integrated cancel/errors) vs a dblink orchestrator (simpler, but a contrib dependency + self-connections).
2. staging combine: ATTACH PARTITION (metadata-only, needs a partitioned target) vs a columnar-native stripe splice into a non-partitioned target (deferred — needs format surgery + its own crash-safety proof).
3. Default workers: physical-core count vs a fixed 8.

The file range splitter (phase 1) is mechanism-independent and I'm building it now regardless of the above.

🤖 Generated with Claude Code

[ChronicallyJD]

Phase 1 landed: the file range splitter (pgcolumnar.file_split_offsets), with test/parallel_copy.sh.

Given a text file and N workers it returns N+1 record-aligned byte offsets that partition the file — the coordinator will hand [off[i], off[i+1]) to each worker, with no record split across a boundary. The test proves it structurally (offsets bracket the file, non-decreasing, every interior boundary sits right after a newline) and end-to-end against a heap oracle (the union of per-range COPYs equals one COPY of the whole file), across workers 1–16, more workers than rows, a single row, and a file with no trailing newline. Green on the PG18 + PG19 assert suites (18 checks each); the C file compiles clean on 15/16/17.

Phase 1 is mechanism-independent. I've paused before the coordinator (phases 2–4) pending a steer on the two questions above — chiefly bgworkers vs dblink — since that choice determines how each worker runs its COPY and I'd rather not build it twice. Happy to proceed on the bgworker default if you're good with it.

[ChronicallyJD]

Review: strong design, one blocker that invalidates the default mode before it is built

Reviewed as what it is: a design doc plus phase 1. I am not reporting "incomplete"
as a finding. The plan is unusually good in places -- the CSV quote-state scan with
a fall back to a single COPY ("correctness first"), the decision not to skip WAL
with the reason given, and the explicit adversarial fixture list are all right.

Two things I reproduced myself are marked verified; the rest came from a
multi-dimension review and are marked as reported.

---

Blocker (verified): the coordinator cannot commit a prepared transaction

mode => 'atomic' has each worker PREPARE TRANSACTION, then "the coordinator
COMMIT PREPARED each". The coordinator is pgcolumnar.parallel_copy(...), a SQL
function. A function cannot execute transaction-control commands:

COMMIT PREPARED at top level:       COMMIT PREPARED        (ok)
COMMIT PREPARED inside a function:  ERROR: EXECUTE of transaction commands
                                           is not implemented

Measured on PG17 with max_prepared_transactions=8.

So atomic mode fails at the commit step, after every worker has durably prepared,
which is after 100 percent of the parse and encode work. And it is the default
mode. This is worth settling before phases 2 and 3, because both are built on it.

It is fixable, and it bears directly on your open question 1: the commit step has
to run in a session that is not the calling function. A dedicated coordinator
background worker can do it (it is its own top-level session); so can dblink. What
cannot work is the pipeline as drawn, where the calling backend is the coordinator.
If you keep native background workers, the coordinator role has to move into one.

Blocker (reported, with a repro): no regular-file check

OpenTransientFile then lseek(SEEK_END) with no fstat/S_ISREG
(columnar_parallel_copy.c:91-97).

• SELECT pgcolumnar.file_split_offsets('/etc', 1) returns
  {0, 9223372036854775807} with no error: a directory reported as an 8-exabyte
  splittable file. With workers >= 2 the same call instead fails with
  "could not read file: Is a directory", so behaviour differs by worker count.
• /dev/zero reports size 0, i.e. "empty file".
• A FIFO blocks the backend inside open(2), and PostgreSQL installs handlers with
  SA_RESTART, so that wait is not interruptible by cancel.

Core COPY guards exactly this (copyfrom.c, fstat + S_ISDIR), and this repo
already uses S_ISDIR in columnar_parquet_reader.c. Since the header comment says
this mirrors core COPY's file handling, matching it here is the fix.

---

Staging mode: the split and the combine do not fit together

Reported, and I think it is the most important design issue after the 2PC one.

Workers get byte ranges of the input file. Partitions need key ranges. On
unsorted input every staging table spans the whole key space, so each ATTACH either
fails with "partition constraint is violated by some row" or, if it does not, only
because the bound is wide enough to be useless. The plan half-acknowledges this at
line 124 ("requires the split to align with the partition key") but the splitter
cannot produce that alignment, and nothing else in the design does.

Related: ATTACH is not metadata-only here. Attaching a freshly loaded table runs
a validation scan unless a matching CHECK constraint proves the bound. For a
columnar staging table that is a full decode of everything just written. If a
default partition exists on the target -- the normal defensive time-series layout --
each ATTACH also takes AccessExclusiveLock on it and re-validates. That is worth
stating against this project's standing rule that exclusive-level locks be justified
against a weaker correct one.

Also: the design never says the N ATTACHes are one transaction. If they are not, a
coordinator crash after k of N leaves k partitions permanently visible, which is the
torn partial load the doc says cannot happen.

Test suite: the gate cannot see a splitter that stops splitting

Credit first: the alignment predicate is the right one, and both off-by-one mutants
(boundary on the newline, boundary one byte past) are caught on the 5000-row fixture.

But nothing asserts that a split actually happened. A splitter degraded to a single
range -- {0, size, size, ..., size} -- satisfies both "offsets ascending and
newline-aligned" and "per-range loads reconstruct the whole file". That is not
hypothetical: a bare-CR file produces exactly that today, and phase 5's "fall back to
a single COPY" path is designed to produce it deliberately. When that path starts
firing when it should not, this suite stays green.

One assertion fixes it: for a fixture with many more lines than workers, require the
interior offsets to be distinct.

Two smaller ones: the 1 row / 4 workers case has no discriminating power (it
re-runs the W=1 path), and no fixture has a line longer than
COLUMNAR_SPLIT_SCAN_CHUNK (64 kB), so the multi-chunk loop -- the only loop with
real state -- is never exercised.

Smaller, phase-1 code

• No CHECK_FOR_INTERRUPTS in the scan. Each boundary scans forward independently,
  so a newline-poor file costs roughly size * (workers-1)/2 of uncancellable
  single-threaded reads. A single forward pass emitting all boundaries would be
  O(filesize) and removes the problem rather than making it cancellable.
• workers is an unbounded int32. workers => 100000000 allocates on the order of
  gigabytes before doing anything. Bound it in the C helper, not only in the
  coordinator, because the helper is separately callable from SQL today.
• off[0] is hard-wired to 0, which leaves no room for HEADER later without
  changing a published contract. Defining off[0] as "first byte of the first data
  record" now costs one optional argument.
• STRICT makes the C code's NULL-argument error unreachable.

The shipped SQL surface

The C header says "Text format only for now" and explains why, which is right. But
the function's own comment says ranges are placed "so no record is ever split across
two ranges" unconditionally, and the SQL COMMENT says "split a text file", which a
user holding a CSV will reasonably read as covering them. There is no format
argument and no guard. Either take a format and reject csv until phase 5, or say
"COPY text format" in both comments.

Your three open questions

1. Mechanism. Background workers, but the coordinator role must move into one,
   for the 2PC reason above. That is the deciding argument, not dependency taste.
2. Staging combine. Neither as posed. The question assumes ATTACH is
   metadata-only and it is not, so the comparison that motivates deferring the splice
   does not hold. I would settle the byte-range versus key-range mismatch first;
   until that is answered, staging is not a v1 option regardless of the combine.
3. Default workers. Not raw physical cores: PostgreSQL has no portable physical
   core count, and min(cores, 8) on a stock server asks for the entire
   max_worker_processes pool (default 8), starving autovacuum and everything else.
   Derive it from the admin's existing parallelism budget and expose the cap as a
   GUC, so there is one knob rather than two that disagree.

---

Happy to be wrong on the reported items; the two verified ones I would want fixed
regardless. The 2PC finding is the one I would act on first, since it decides the
shape of phases 2 and 3.

[ChronicallyJD]

Phase 2 landed: the background-worker coordinator + bounded COPY. pgcolumnar.parallel_copy(target, filename, workers) now fans a text load across N dynamic bgworkers, each running core COPY over a record-aligned byte range via BeginCopyFrom + a bounded data source (so parse/write stay core COPY's), coordinated through one DSM segment; workers report via a shared status word, and any error or cancellation terminates every launched worker (no orphans). Test: the N-worker load is byte-identical to a single COPY at 1/2/4 workers, a missing file errors cleanly with nothing loaded and no crash, and a non-columnar target is rejected. This slice commits each range independently.

One architecture correction for the atomic mode (design doc updated): I'd assumed the parallel_copy() function could drive the 2PC finish, but COMMIT PREPARED/ROLLBACK PREPARED cannot run inside a transaction block and a SQL function always is one (verified: EXECUTE of transaction commands is not implemented). So atomic mode runs the coordinator as its own background worker — the function launches one coordinator bgworker that owns its transaction loop, spawns the loaders, prepares them, and issues the commit-all/rollback-all, then reports the total back. The phase-2 plumbing is reused unchanged; only who runs it moves. Staging mode (ATTACH PARTITION) has no such constraint and stays function-driven. Building the coordinator-bgworker + 2PC next.

[ChronicallyJD]

Thanks — this is a genuinely useful review; several of these are things I would not
have caught from the diff alone. Addressed below; the two you verified are both
handled, and I converged with you independently on the 2PC one.

Blockers

2PC — coordinator cannot COMMIT PREPARED. Confirmed, and I hit the same wall
independently right after posting phase 1 — see the "Architecture correction
(verified 2026-08-01)" note already in the plan (commit 1caad53). We agree on both
the cause (a SQL function is always in a transaction block; the utility path calls
PreventInTransactionBlock) and the fix: the coordinator role moves out of the
parallel_copy() function into a dedicated coordinator background worker (its
own top-level session, so it can finish the 2PC). The loaders PREPARE TRANSACTION; the coordinator bgworker commits-all or rolls-back-all. That is now
phase 3; the per-range-commit path in this draft is explicitly not atomic yet.

No regular-file check. Fixed. Added pcopy_open_regular_file() — fstat +
S_ISREG, matching core COPY (copyfrom.c) and the existing S_ISDIR use in
columnar_parquet_reader.c — and routed the splitter, the coordinator, and the
worker through it. A directory / /dev/zero / FIFO is now rejected with "%s" is not a regular file before any scan or open()-that-blocks. New regression test:
file_split_offsets('<dir>', 1) must error, not return {0, 9.2e18}.

Staging — removed from v1

You're right, and it's the design issue I'm most glad you caught. Byte ranges are
not key ranges, so on unsorted input every staging table spans the whole key space
and each ATTACH either violates the partition constraint or is useless; ATTACH
also isn't metadata-only here (validation scan = a full columnar decode, plus an
AccessExclusiveLock re-validation against any default partition); and the N
ATTACHes were never one transaction. I've removed staging from the plan and
written all three sub-issues into the staging section — it becomes separate future
work once the byte-vs-key alignment has an actual design (sort-aware split or a
columnar stripe splice). v1 ships atomic mode only.

Test suite — the gate can now see a stopped splitter

Fixed, and this is the same vacuous-premise class I've been burned by before.
Added check_split_happened: on a fixture with many more records than workers,
the W−1 interior offsets must be strictly increasing and strictly inside
(0, size), so a splitter degraded to {0, size, …, size} fails. Runs on the
5000-row fixture (W∈{2,3,8,16}) and the new long-line fixture. Also:

• Added a fixture whose lines exceed the 64 kB scan chunk, so the multi-chunk read
  (the only part of the scan with carried-over state) is exercised.
• Demoted 1 row / 4 workers to an explicit edge smoke test — as you noted, its
  correct result is the degenerate single-range shape, so it can't discriminate.

Smaller phase-1 code

• CHECK_FOR_INTERRUPTS + O(size·workers). Fixed by rewriting the splitter as
  a single forward O(filesize) pass (read in chunks, CHECK_FOR_INTERRUPTS per
  chunk, emit every boundary in order). The per-boundary seek-and-scan is gone, and
  the SQL splitter and the coordinator now share the one pass (pcopy_line_offsets).
• Unbounded workers. Capped at PCOPY_MAX_WORKERS (1024) inside the C helper
  itself, so the SQL-callable file_split_offsets can't allocate gigabytes either.
• STRICT dead code. Removed the unreachable NULL check (kept STRICT).
• off[0] and HEADER. Not changed yet — I'm deferring it to the phase that
  adds the format/HEADER arguments (same phase as CSV), and redefining off[0]
  as "first byte of the first data record" then, so the published contract changes
  once, with the feature that needs it. Flagged in the plan.

SQL surface

Both the C header and the SQL COMMENT now say COPY text format explicitly and
state that CSV is not safe to split this way and is unsupported until the
quote-aware phase. No silent "split a text file" that a CSV holder could misread.

Your three open questions

1. Mechanism: bgworkers, coordinator-in-a-worker — decided on the 2PC
   constraint, per above.
2. Staging combine: neither, yet — deferred with staging.
3. Default workers: no longer raw cores. workers => NULL now derives
   max(1, max_parallel_workers / 2) (from the admin's existing budget, not the
   whole max_worker_processes pool), and the cap will be the single GUC. Changed
   the SQL default to NULL so this actually fires.

Rebuilt clean and re-ran test/parallel_copy.sh — 36 checks green on pg18a and
pg19a (assert), and clean under pg18_san (ASan/UBSan) since the new scan
handles untrusted file bytes. Pushed on top of this branch (no force-push).

[ChronicallyJD]

Phase 2 does not build on PostgreSQL 15 or 16. CI reports four red jobs under
"warnings are failures", but they are hard compile errors, and they are two
different causes, one per major. Reproduced locally against real installs:

major	result
PG15	columnar_parallel_copy.c:459:33: error: initialization of 'RangeTblEntry *' from incompatible pointer type 'ParseNamespaceItem *'
PG16	columnar_parallel_copy.c:408:23: error: 'BGWORKER_BYPASS_ROLELOGINCHECK' undeclared (first use in this function)
PG17	clean

PG15. The #else arm of the PG_VERSION_NUM >= 160000 guard at 457-459 assigns
addRangeTableEntryForRelation() straight into a RangeTblEntry *. That function
has returned ParseNamespaceItem * since PG13, so the guard splits on the wrong
thing: PG16 moved permission info out of the RTE, but the return type changed
three majors earlier. The PG15 arm needs the nsitem too, then nsitem->p_rte for
the requiredPerms and insertedCols it sets.

PG16. BGWORKER_BYPASS_ROLELOGINCHECK arrived in PG17; there is no guard around
it. Whatever it is doing for the worker's connection needs a version guard, and on
PG16 and earlier the fallback has to be spelled out rather than assumed.

Neither is deep, but both are the class the five-major preflight exists to catch,
and the project's bar is that warnings are failures on every packaged major.

Two smaller notes while you are in there:

• The #if comment says "PG16 moved permission info out of RangeTblEntry into a
  separate RTEPermissionInfo, so guard that", which is accurate about permissions
  and is what made the return-type change easy to miss. Worth splitting into two
  guards with two reasons rather than one guard doing double duty.
• Whatever you land for the PG16 background-worker path, the phase-1 splitter is
  still separately callable from SQL, so any bound or guard you add in the
  coordinator does not protect it.

The review points from the earlier round I have not re-checked yet, since the build
break blocks running anything on 15 and 16. I will re-review the file guard, the
single-pass split and the test additions once it compiles across the matrix.

[ChronicallyJD]

Update: pivoted to partition-parallel after a measured architectural finding

Building the atomic (2PC) coordinator surfaced a hard constraint, so the design of #300 changed. Recording it here.

The finding (measured, not reasoned)

pgColumnar serializes concurrent writers to one table on a transaction-scoped, per-storage-id advisory ExclusiveLock held to transaction end (columnar_metadata.c:1489, ColumnarInsertNativeStorageRow). Consequences, on the bench (20M-row TSBS slice, warm):

load	time	vs single
single COPY	127.3 s	1.00x
2 concurrent -> same table	255.3 s	2.01x (serialized)
2 concurrent -> separate tables	132.8 s	1.04x (parallel)

So (1) same-table parallel load gets no speedup, and (2) atomic 2PC into one table deadlocks — a PREPAREd loader retains the advisory lock, peer loaders block on it, and the coordinator only COMMIT PREPAREDs after all loaders finish. Caught by gating on an assert build before benchmarking (the run hung; pg_locks showed the prepared xact holding the lock and the peer waiting). Real parallelism requires each worker to write distinct storage.

The pivot: partition-parallel

parallel_copy now loads a RANGE-partitioned target, each worker owning a distinct set of partitions (distinct storage id -> no shared lock -> parallel and 2PC-atomic, no deadlock). The only new code is a partition-aligned splitter (single forward pass parsing each row's key, bucketed against the catalog's sorted bounds, snapping byte ranges to partition boundaries; verifies the file is sorted by the key). The 2PC coordinator/loader machinery is reused unchanged. A plain (non-partitioned) columnar target is rejected with a clear error — single-table parallel load is a deferred columnar-core bulk enhancement (lifting the per-storage write lock for a bulk path).

v1 restrictions (checked): single-column RANGE key, no DEFAULT partition, input sorted ascending by the key, COPY text format.

Validation

• Gate: 51 checks green on pg18a + pg19a (assert) + pg18_san (ASan/UBSan); 5-major preflight compile (15/16/17/18/19). Covers: N-worker load == single-COPY oracle (no prepared-xact leak), loader-failure rolls back the PREPARED siblings, unsorted / DEFAULT-partition / non-partitioned / bad-key all rejected, max_prepared_transactions guard.
• Bench (pg18n, TSBS, warm): 20M slice interleaved — single COPY ~126.8s vs parallel_copy(8) ~25.6s = ~5.0x (scaling N=1/2/4/8 = 137/73/42/27s). Full 100M / 17GB file — single COPY 640.6s vs parallel_copy(16) 118.6s = 5.40x. Both cases byte-identical (count + sum(usage_user)), 0 prepared-xacts leaked.
• Also fixed two latent cross-version bugs found by preflight (BGWORKER_BYPASS_ROLELOGINCHECK is PG17+; PG15 addRangeTableEntryForRelation return type) and a resource leak (splitter used fdopen+fclose on a transient fd -> now AllocateFile/FreeFile).

Design doc updated with the finding + numbers. Pushed on this branch (no force-push).

[ChronicallyJD]

Review of the pivot: the direction is right, one reproduced crash blocks it

The pivot is well-founded and the finding behind it is the good kind: you measured
the same-table serialization instead of reasoning about it, and you found the 2PC
deadlock in a sharper form than my review did. Independently reproducing that a
PREPAREd loader holds the per-storage advisory lock while the coordinator waits
is exactly the evidence that should drive an architecture change.

Build is fixed: I verified clean on all five majors, 0 warnings, 0 errors. Both
cross-version bugs are gone.

Method: 6-dimension review with per-dimension adversarial refutation, 43 findings
surviving, plus my own probes. What follows is what I reproduced myself, then what
was reported.

---

Blocker, reproduced: parallel_copy crashes the server

pcopy_partition_bucket (src/columnar_parallel_copy.c:395) dereferences
bi->datums[mid][0] unconditionally. For MINVALUE/MAXVALUE bounds core never
writes that Datum, by design: create_range_bounds uses palloc, not palloc0,
and writes the datum only for PARTITION_RANGE_DATUM_VALUE. Core's own comparator
partition_rbound_datum_cmp dispatches on kind before touching it. kind
appears nowhere in the file.

With a by-reference key type the uninitialized word is used as a pointer:

CREATE TABLE tp (k text, v int) PARTITION BY RANGE (k);
CREATE TABLE tp_a PARTITION OF tp FOR VALUES FROM (MINVALUE) TO ('m') USING pgcolumnar;
CREATE TABLE tp_b PARTITION OF tp FOR VALUES FROM ('m') TO (MAXVALUE) USING pgcolumnar;
SELECT pgcolumnar.parallel_copy('tp'::regclass, '/path/sorted.txt', 2);

server closed the connection unexpectedly ... the server terminated abnormally
FATAL: the database system is in recovery mode

Cluster crash and recovery, from the most common partition layout there is.

Why the suite is green. mkpart() builds FROM (MINVALUE) TO (...) and
... TO (MAXVALUE) too, but with an int key. By-value, so the garbage word is
compared rather than dereferenced, and it only mis-buckets when it happens to land
inside [1,5000]. The suite passes by luck, not by proof. A text-keyed fixture is
the test this needs.

The fix is small: mirror core's kind dispatch, treating MINVALUE as
unconditionally less and MAXVALUE as unconditionally greater.

For by-value keys the same bug is quieter and worse in a different way: a bucket
transition at an arbitrary value inside the first or last partition means two
workers write the same partition, which is precisely the shared-storage contention
the pivot exists to remove. That path ends in the hang you already diagnosed.

---

Blockers, reported (not reproduced by me)

• Cancel or SIGTERM leaks every prepared transaction. Ctrl-C on parallel_copy
  leaves one prepared transaction per worker with no cleanup path, which pins the
  xmin horizon cluster-wide and, for this AM, also stops physical space reclaim.
  Three dimensions found this independently.
• The loader never checks INSERT privilege on the target. Core COPY sets
  requiredPerms/perminfo and has them checked; the loader path does not, so a
  caller who can reach the function can load into a table they cannot write.

Majors worth settling before this lands

• Rows survive the caller's ROLLBACK. parallel_copy commits independently of
  the calling transaction, so BEGIN; SELECT parallel_copy(...); ROLLBACK; leaves
  the data. That needs to be documented loudly or prevented.
• Calling it from a transaction already holding a lock on the target hangs, and
  the wait is invisible to the deadlock detector.
• The splitter has become a second COPY parser. The key field is taken from raw
  bytes with no text-format de-escaping, the field index skips only dropped columns
  where COPY's default list also skips generated ones, and CRLF leaves a trailing
  \r in the key. Each divergence puts a row in the wrong bucket, and a wrong
  bucket is the hang above. This also undercuts the design doc's strongest argument,
  that there is "no looser parser to get wrong".
• One partition-key Datum leaked per row for by-reference key types, during a
  full pre-scan of the input.

Tests

Real improvement, and directly responsive: "split is real (interior offsets distinct, ranges non-empty)" is exactly the anti-vacuity assertion I asked for, and
"a directory is rejected (not an 8-exabyte file)" closes the earlier finding. The
rejection tests now pair "X is rejected" with "X loaded nothing", and the
prepared-xact leak is checked after success, after a bad key, and after a loader
failure. Setting max_prepared_transactions via PGC_EXTRA_CONF before cluster
start, rather than skipping, means CI actually runs this.

Two gaps:

• Nothing asserts the load ran in parallel. The W in 1, 2, 4 loop checks rows,
  oracle equality and no leak, all of which a serial implementation satisfies
  identically at every W. You fixed exactly this one layer down for the splitter;
  the loader needs its equivalent.
• The key-field locator is never exercised: every fixture puts the key in column
  1, so the field-index logic that generated columns break is untested.

Smaller: the file header still describes phase 1 ("covers the file range splitter"),
half the suite tests file_split_offsets, which the pivoted feature no longer
calls, and two rejection greps accept the wrong reason.

Claims

• "The 2PC coordinator/loader machinery is reused unchanged" is not accurate; it
  changed.
• "v1 restrictions (all checked)" is not accurate: the escape-free key field is
  required but not enforced.
• "byte-identical (count + sum(usage_user))" is weaker than "byte-identical"
  implies. pgc_set_hash is available and is what the rest of the suite uses.

Mechanics

The branch is behind main and conflicts in one line of test/run_all_versions.sh,
where both it and #321 registered a suite. I test-merged: it does not revert
#321, the grouped aggregate survives. Just needs the one-line resolution.

---

The crash is the only thing I would call merge-blocking on its own. Everything else
is a judgement call about what lands now versus in the next slice, except the
privilege check, which I would not defer.

[ChronicallyJD]

Thanks — this is a strong review and the crash is exactly the kind of thing that had to be caught before merge. All of it is addressed on the branch (through 3d05d67).

The reproduced crash — converged, fixed, now tested

We found the same bug independently: my own adversarial pass flagged that pcopy_partition_bucket compared against datums[mid][0] for MINVALUE/MAXVALUE bounds, and I fixed it with the exact kind-dispatch you describe (b67c14a, which your review predates). Your text-keyed repro is the sharper demonstration — by-reference key ⇒ pointer deref ⇒ crash, not just a mis-bucket — and it's now a regression test: a text key is rejected cleanly (see below) and the server stays up.

Blockers

• Cancel/SIGTERM leaking prepared transactions — fixed. You're right: the coordinator used die, so a cancel proc_exit'd past its PG_CATCH. It now installs a flag-setting SIGTERM handler and a latch-driven wait loop; on cancel it terminates the loaders, waits for them, and ROLLBACK PREPAREDs every prepared range in normal backend context (not proc_exit, where 2PC isn't safe). No orphaned in-doubt xact on Ctrl-C.
• INSERT privilege — fixed. The function now does pg_class_aclcheck(relid, GetUserId(), ACL_INSERT) up front, matching COPY FROM, before anything is spawned.

Majors

• Splitter parser divergences — fixed / bounded. Generated columns are now skipped in the key-field index (matching COPY's default list); CRLF no longer leaves a \r in the key; and rather than half-reimplement COPY de-escaping, v1 now restricts the key to numeric/date-time types, whose text form is escape-free by construction — other key types are rejected, not silently mis-parsed. That turns "no looser parser to get wrong" into something enforced, not hoped for.
• Per-row key Datum leak — fixed. By-reference keys (e.g. numeric) are pfreed each row.
• Rows survive the caller's ROLLBACK / lock-hang — documented. The SQL comment now states the load commits independently of the caller (treat it like a COMMIT) and must not be called while the caller holds a lock on the target (loaders would block, invisible to the deadlock detector).

Tests

• Added your text-key fixture (clean rejection + server up), a key-not-in-column-1 with a generated column before it fixture (exercises the field-index logic), and signed keys straddling 0 under [MINVALUE,100) (the crash's by-value cousin). Tightened the bad-key grep to the exact reason.
• "Nothing asserts the load ran in parallel" — fair, and I don't have a cheap, non-flaky suite assertion that distinguishes a serial implementation from a parallel one (rows/oracle/no-leak all hold either way). The parallelism evidence is the bench: 5.0× on 20M and 5.40× on the full 100M / 17GB vs single COPY, which a serial loader cannot produce. I'd rather say that plainly than add a timing-based suite check that flakes; open to a concrete idea if you have one.

Claims / mechanics

• The escape-free-key restriction is now enforced, so that "v1 restriction" is real.
• Corrected the design doc: the 2PC machinery is reused (loaders route into the parent; coordinator gained cancel cleanup), not "unchanged".
• "byte-identical" — the suite asserts it via pgc_set_hash; only the 100M bench used count + sum(usage_user) (a full ordered hash of 100M columnar rows was the cost I skipped). Reworded.
• The one-line run_all_versions.sh conflict is resolved — merged main, kept both suite registrations; Grouped vectorized aggregate (#289) #321's grouped aggregate survives.
• File header now describes the partition-parallel design; file_split_offsets stays as a standalone SQL diagnostic (still tested), no longer on the load path.

Re-gated: 58 checks green on pg18a and pg19a (assert) and pg18_san (ASan/UBSan), clean preflight compile on all five majors. The single-table (non-partitioned) case remains the tracked columnar-core-bulk follow-up.

[jdatcmd]

Approved

Partition-parallel is correct and complete for v1: each worker owns a distinct set of partitions (distinct storage id), so the load is parallel and atomic (2PC) with none of the same-table write-lock deadlock.

The pivot review's findings are all addressed on the branch: the MINVALUE/MAXVALUE crash (kind-dispatch, plus a text-key regression), cancel/SIGTERM no longer leaking prepared transactions (flag handler + latch wait + rollback in normal context), the INSERT-privilege check, and the splitter parser divergences (generated-column skip, CRLF, per-row Datum free, key restricted to escape-free numeric/temporal types). Rows-survive-ROLLBACK and the lock-hang are documented.

• Gated: 58 checks green on pg18a + pg19a (assert) and pg18_san (ASan/UBSan); clean preflight compile on all five majors (15-19).
• Benched: ~5.0x on a 20M slice and 5.40x on the full 100M / 17GB file vs single COPY, results identical (suite asserts via pgc_set_hash), zero prepared-xact leaks.

Single-table (non-partitioned) parallel load is the tracked follow-up (columnar-core bulk path).