On Sunday, October 25th, 2026 at 9:21 PM, Manu <[email protected]> wrote:
> Hi Greg, Hi Manu, Thanks for the depth here. All four diffs are in v12, which I'll post as its own message; replies inline. > 1. lz4 without zstd does not compile. lz4_dict_get() is only under > #ifdef USE_LZ4 and calls dict_fetch(), which is static and inside > #ifdef USE_ZSTD; the call is at toast_dictionary.c:537. Fixed as you had it: dict_fetch() now sits under USE_ZSTD || USE_LZ4 and only the zstd invalidation-callback registration stays under USE_ZSTD. I build all three configurations now (zstd+lz4, lz4-only, zstd-only), and each passes make check (243). cfbot never caught this because it builds with both; I should have built the single-codec configs myself. > 2. With zstd but without lz4, compression_zstd_dict fails: its SET > COMPRESSION lz4 gets "compression method lz4 not supported". The same > diff uses pglz there. Taken; the test uses pglz for the switch-away-from-zstd_dict case, which does not depend on lz4 being built. > 3. 0005 swallows errors, a cancel included. [...] A real SIGINT on entry > to it: ANALYZE completed and only logged "... was skipped: canceling > statement due to user request". A swallowed cancel and the half-done dict_install() is fixed. v12 runs the evaluation in an internal subtransaction and re-throws, exactly as your diff does: BeginInternalSubTransaction, the work, then either ReleaseCurrentSubTransaction on success or RollbackAndReleaseCurrent on error; and it re-throws when edata->sqlerrcode is ERRCODE_QUERY_CANCELED or an interrupt is still pending, since ProcessInterrupts() clears QueryCancelPending before raising. A failed step now rolls back its TOAST scan and buffer pins instead of leaking, and a mid-dict_install() error no longer leaves the old dictionary non-current with no replacement. Under cassert+ASan a SIGINT gives the normal ERROR, a damaged chunk gives one WARNING and no leak, and adoption still works in plain ANALYZE, in BEGIN/COMMIT, BEGIN/ROLLBACK, a rolled-back savepoint, and VACUUM (ANALYZE). > 4. The dictionary caches hold one entry. Every switch between > dictionaries fetches the dictionary again, and zstd digests it again. > [...] one column retrained without a rewrite, which evolution now does > on its own [...] 258,163 (1,946 fetches for 2,000 values). Right, and evolution makes this the common case, not a corner one. v12 takes your 4-entry LRU keyed by OID for both directions, picking the victim after dict_fetch() since the fetch can run the invalidation callback and empty the cache. The instruction counts match what you saw: the two-dictionary scan drops from ~265k to ~28k per value, and the retrain-without-rewrite case from ~258k to ~28k. I kept your 4; it is arbitrary but it covers a scan that alternates between an old and a new dictionary with headroom, and the per-backend memory is bounded. > 5. Evolution without drift: [...] At 6,000 one evaluation calls > ZSTD_createCDict() 12,005 times [...] The fourth diff replaces > compressed_size() with compressed_total(), one call per dictionary. Taken. compressed_total() digests each dictionary once and loops the held-out values inside, so an evaluation now builds two CDicts instead of one per value. Same totals, same decision, and the held-out test stops dominating the ANALYZE it rides on. > 6. On the ~1kB numbers: a 1 kB value alone in its row stays below > TOAST_TUPLE_THRESHOLD (heapam.c:2266), so it is stored uncompressed with > every method [...]. At 3 kB the dictionary takes lz4 from 1.94 to 2.50 > on docs, 3.58 to 4.76 on json. You're right, a ~1 kB value never reaches the compressor, so the "+80% at 1 kB" number was measuring nothing on the TOAST path; it came from a standalone CLI run, not from values actually stored. The dictionary helps once a value crosses TOAST_TUPLE_THRESHOLD. > Corrupt dictionary values error cleanly under ASan, and substr() matches > the raw text at 56 offset/length pairs on UTF8 and SQL_ASCII. Matches my runs too, corrupt values raise the data-corrupted error with no sanitizer report, and substr() over the offset/length grid matches the raw text on both encodings. Thanks again; the subtransaction and the cache fixes in particular are ones the series needed. v12 follows. best. -greg
