feat: round-trip flush_one_column through a dsm segment, serial (#445 slice 2)

[ChronicallyJD]

#445 slice 2: round-trip flush_one_column through a dsm segment (serial, no workers)

Second slice of the in-COPY parallelism design (#588), on top of slice 1 (#589). Still serial in the backend; output is byte-identical. No workers yet — the whole job of this slice is to prove the per-column input and result can cross a dsm segment byte-identically, so slice 3 can move the call to a worker with the serialization already settled.

Depends on #589 (flush_one_column). Branch is stacked on it; will rebase to main once #589 merges.

What changed

The per-column flush loop now, for each column:

1. serializes that column's flush_one_column input — its per-chunk-group buffers (existsStream, valueStream, hashBuf, valueCount, sum, hasMinMax, and the min/max Datums via datumSerialize) — into a dsm_created segment behind a uint32 length prefix;
2. deserializes it back into a reconstructed per-column chunkGroups (every buffer copied out of the dsm into palloc'd memory), and runs flush_one_column on the round-tripped input;
3. serializes the result (chunk bytes, descriptor, block codec, zone-map rows incl. the numeric sum via datumSerialize, bloom filter) into a second segment, deserializes it, and assembles into the stripe exactly as slice 1 did;
4. dsm_detaches both segments.

flush_one_column's slice-1 signature is untouched: the read side reconstructs a minimal chunkGroups whose columns array is sized (columnIndex+1) with only [columnIndex] populated (the only entry the function reads). Four static helpers (serialize_column_input/deserialize_column_input/serialize_column_result/deserialize_column_result) carry a fixed little wire format; datumSerialize/datumRestore handle the min/max and numeric-sum Datums.

Why it's safe

• Copy-out before detach. Every buffer and Datum is copied from the dsm into palloc'd memory (initStringInfo+append for streams; palloc+memcpy for descriptor/min/max/filter; datumRestore for Datums), so nothing references the segment after dsm_detach. Lifetimes match slice 1 (flush context).
• Self-checking format. Each deserializer asserts the cursor consumed exactly the payload length (Assert(cursor == base + 4 + payloadLen)).

Verification (prove-not-trust)

• Byte-identical vs heap oracle: differential.sh PASSES.
• Zone-map / min-max / bloom serialization specifically: native_zonemap, native_bloom, write_minmax_fastpath PASS — these exercise the parts of the wire format most likely to drift.
• Write path across types/codecs: native_writer, native_dml, write_fsst_compressed, encode_invariants, encode_effort, corruption, native_reclaim, native_roundtrip PASS.
• Two toolchains: forced ASAN+UBSAN (pg18_san) — 0 sanitizer reports on every suite (the dsm round-trip + copy-outs are the memory surface); and assert (pg18a) — no TRAP/Assert/crash. -Wshadow=compatible-local / -Werror clean.

Next

Slice 3: dispatch flush_one_column across min(natts, N) background workers reading the slice-2 dsm, backend collects in column order, degrading to the serial path when worker slots are unavailable (never a wrong row count). Slice 4: GUC + measure vs the ~17% ceiling, then the default flip.

🤖 Generated with Claude Code

https://claude.ai/code/session_017N82wDmsawqSWoWkmxtHmW

[jdatcmd]

Correctness is solid — but this regresses the write path ~26% on the merge target

Verified the byte-identical claim rather than trusting it, and it holds: same 400k-row text-heavy load on this branch and clean main is identical in size, content hash, encoding-descriptor hash, and chunk bytes (3915776 / 64e2fe13… both). differential, native_writer, native_zonemap, native_bloom, native_dml, native_roundtrip, write_fsst_compressed, encode_effort all pass. The serialization is careful — datumSerialize for min/max, deserialize copies out of the dsm into palloc'd memory, length prefixes throughout. As a proof that the input and result cross a dsm byte-identically, slice 2 does its job.

The blocker is performance on main, which the suites don't measure. This slice does dsm_create + dsm_detach twice per column per flush (columnar_write_state.c :968, :982, :1377-8, inside the per-column loop), and segment creation is expensive. Measured, 41-column / 500k-row load, pg18 non-assert, median of 3:

	load
main	1061 ms
this branch	1333 ms

~26% slower, with no benefit yet (still serial), and it scales with column count — a 105-column ClickBench table does 2×105 dsm_create/flush. Merging this to main as a standalone slice means every writer pays that until slice 3 lands. That is the exact opposite of #445's direction for anyone loading in that window.

What I'd change before this lands on main

Any one of:

1. Reuse one dsm segment, not per-column. Create a single segment per flush (or, better, a persistent per-writer segment grown as needed) and round-trip every column through it. That removes the dsm_create churn that is the whole cost here — and slice 3's workers will want a stable segment to attach to anyway, so this is the shape that slice actually needs, not throwaway scaffolding.
2. Gate it off by default behind the GUC/table-option slice 4 introduces, so main keeps the serial in-backend path until the full pipeline is proven a net win.
3. Land it with slice 3, so main never carries the serialize-without-parallelism cost alone.

I'd take (1): it is the least scaffolding-thrown-away and it is what the worker slice needs regardless. The extraction and wire format below it are right; it's the per-column dsm_create that shouldn't reach main. Happy to re-review promptly on any of the three.

[ChronicallyJD]

You're right, and thank you for A/B-ing the write path — I verified byte-identity and memory-safety but did not measure load time, which is my miss: a perf A/B is exactly the discipline I hold my own findings to, and I didn't apply it to a regression I introduced. The per-column dsm_create/dsm_detach (2×natts per flush) is real cost for zero benefit while it's still serial.

One thing that strengthens the case, which you may not have seen yet: #591 (slice 3) inherits this. Its OFF branch is verbatim slice 2 — I confirmed reading the else in pgcolumnar_flush_row_group: with pgcolumnar.parallel_flush off (the default), it runs the same per-column round-trip. So the ~26% doesn't just affect slice 2 standalone; it's the default path of slice 3 too until something changes.

That points past "make the round-trip cheaper" to "the serial path shouldn't round-trip at all." The serialization only earns its cost when there's a worker to hand the bytes to; in-backend-serial, the right path is a direct flush_one_column call — which is exactly merged main (slice 1) today, zero overhead.

So my recommendation, which is your option (3) sharpened:

• Serial/OFF = direct flush_one_column (= merged main, no dsm) → no regression on main, ever.
• Parallel/ON = one input dsm + workers — already how slice 3 builds it (a single per-flush segment, not per-column; your option (1)'s stable segment is what the worker slice already uses).
• Fold slice 2's serialize/deserialize helpers into slice 3, where the workers are their only consumer, and supersede this PR (feat: round-trip flush_one_column through a dsm segment, serial (#445 slice 2) #590). The extraction and wire format you validated ride in unchanged; only the "round-trip serially in the backend" scaffold is dropped.
• I'll add the perf A/B to slice 3's gate: OFF == main (no regression) and ON faster (the actual win), alongside the byte-identical differential.

Your option (1) (one segment, keep slice 2 standalone) also works and is less restructuring, but it leaves a small serialize/deserialize cost on the serial path and doesn't fix #591's default. Since it's your design, your call — I'll re-verify with perf either way. If you're good with the fold, I'll close #590, revert slice 3's OFF branch to the direct path, and re-run the gate with the A/B.

[ChronicallyJD]

Superseded by #591 — closing

Per the review discussion here and on #591: the serial dsm round-trip this PR introduced should not be a shipping path (it's ~22-26% overhead when no worker reads the bytes, scaling with column count). The resolution landed in #591: the serial / non-dispatched flush calls flush_one_column directly, and the dsm crossing lives only on the worker path. The serialize/deserialize helpers and wire format you validated here ride into #591 unchanged — they're the workers' only consumer now.

So this standalone slice is retired rather than revised: there's no serial-round-trip path left to merge. Measured after the fix, the default (parallel_flush off) is back to main's write-path time (wide 41-col: 2.19 s == 2.19 s), and the parallel path is −11%. Closing; the work continues in #591.