Skip to content

perf: detoast each varlena once per row on the write path (#445) - #587

Merged
jdatcmd merged 1 commit into
mainfrom
fix/445-detoast-once
Aug 12, 2026
Merged

perf: detoast each varlena once per row on the write path (#445)#587
jdatcmd merged 1 commit into
mainfrom
fix/445-detoast-once

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

What

PgColumnarWriteRow flattened every varlena value up to four times per row — once in PgColumnarEncodeValue, once for the bloom hash, and once per min/max comparison. For a toasted (pglz-compressed) value each of those is a full decompression; #445's write-path profile saw it as detoast_attr.

This detoasts once at the top of the row's column handling and reuses the flat Datum for the encoder, bloom hash, and min/max compares, freeing the copy after the row. pg_detoast_datum returns the same pointer for an already-flat value, so the uncompressed common case copies and frees nothing and non-varlena types skip it — output is byte-identical either way.

Measured

300,000-row load of a ~3.8 kB pglz-compressed text column, pg18 non-assert, median of 3:

load
before 3193 ms
after 2836 ms

11.2% faster. The saving scales with how compressed the values are — small for short strings (ClickBench URLs), larger for wide text. This is one item in #445's broad write-path tail, not its headline (that is compression, which parallelises).

Tests

test/native_toasted_write.sh (new): loads a heap table's toasted values into columnar and differences against a heap mirror — values byte-identical, min/max range prune correct, equality (bloom + recheck) correct, and the short non-toasted column unaffected. It guards the flatten-once path specifically, which the other write suites don't reach (they use short, non-toasted values). Registered, harness_selftest green (107).

Correctness unaffected: differential, native_writer, native_zonemap, native_bloom all pass. The removal proof for the optimisation is the load-time number above (deleting it stays green — the output is identical — so the perf win is a timing, not a check).

🤖 Generated with Claude Code

PgColumnarWriteRow flattened every varlena value up to four times per row -- once
in PgColumnarEncodeValue, once for the bloom hash, and once per min/max
comparison -- and for a TOASTED (pglz-compressed) value each of those is a full
decompression. #445's write-path profile saw this as detoast_attr.

Detoast the value once at the top of the row's column handling and reuse the flat
Datum for the encoder, the bloom hash and the min/max compares; free the copy
after the row. pg_detoast_datum returns the same pointer for an already-flat
value, so the common uncompressed case copies and frees nothing and non-varlena
types skip it entirely -- the output is byte-identical either way.

Measured on a 300,000-row load of a ~3.8 kB pglz-compressed text column, median
of 3: 3193 ms -> 2836 ms, 11.2% faster. The saving scales with how compressed the
values are: small for short strings like ClickBench URLs, larger for wide text.

test/native_toasted_write.sh loads a heap table's toasted values into columnar
and differences the result against a heap mirror -- values byte-identical, min/max
range prune correct, equality (bloom + recheck) correct -- guarding the
flatten-once path, which the other write suites do not reach because they use
short non-toasted values. Green on pg18; differential, native_writer,
native_zonemap and native_bloom unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WmQJqcXdwyuoAiHHt2znBr

@ChronicallyJD ChronicallyJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve — verified on four dimensions, prove-not-trust

Reviewed independently of the author. The detoast-once change is correct, memory-safe, faster, and adequately guarded.

  • Correctness vs heap oracle. differential + native_toasted_write pass. I added an external-TOAST probe the base fixture doesn't reach — 4160-byte values with STORAGE EXTERNAL, premise-verified is_external=t — and columnar matches heap on values, min/max pruning, and bloom equality. That closes the one nuance in the change: datumCopy on the zone-map min/max now copies the flattened value rather than a possibly-external pointer, and it prunes identically to heap on the external branch.
  • Memory-safety. Forced ASAN+UBSAN (instrumented .so confirmed): native_toasted_write, differential, native_writer, native_dml, write_fsst_compressed, encode_invariants, corruption, native_reclaim0 sanitizer reports. The new per-row pfree(flat) is clean.
  • The guard is non-vacuous. Removal proof: corrupting the flatten (marker verified live in the .so, not stale) flips native_toasted_write to FAILED on the value/bloom/non-toasted checks.
  • Perf reproduces. Interleaved A/B, 300k-row toasted-text load, pg18n non-assert, 6 reps/arm: ~8.3% faster, every rep separated (your 11.2% on a more concentrated wide-text column; mine is diluted by a 3-column table). Direction unambiguous.

Merge remains yours — this is a review approval, not a merge.

@jdatcmd
jdatcmd merged commit 9375d20 into main Aug 12, 2026
11 checks passed
jdatcmd pushed a commit that referenced this pull request Aug 12, 2026
Re-ran the full harness (main suite 6M rows + FSST + joins + cross-engine
ClickBench) on main at 2fe6596, pg18n non-assert, 16 cores/62 GB, and updated
every measured section. The substantive changes:

- ClickBench columnar-vs-heap win count 33/6/4 -> 25/13/5, because #452 phase-2
  decode gating shipped and is a selectivity trade: +1.8x on q24 (its case),
  -1.2..2x on eight less-selective queries. Documented with an A/B and filed as
  the follow-up #595.
- Load gap restated as a serial-path property (columnar 2.38x serial / 1.73x
  bulk vs Citus), columnar still smallest on disk (1.48 GB).
- parallel_flush is a measured opt-in, not eventually-default (#445 slice 4).
- #452 phase 1+2, anchored LIKE (#510), detoast-once (#587), FSST verdict cache
  (#472), bloom by distinct count (#467) recorded in 'what changed'.
- Noted #423 (q21 unsupported byval length) is resolved -- q21 no longer errors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017N82wDmsawqSWoWkmxtHmW
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants