On Wed, Dec 17, 2025 at 04:11:38PM +0100, Peter Eisentraut wrote: > On 16.12.25 11:51, Dharin Shah wrote: > > - Zstd only applies to external TOAST, not inline compression. The 2-bit > > limit in va_tcinfo stays as-is for inline data, where pglz/lz4 work fine > > anyway. Zstd's wins show up on larger values. > > This is a very complicated patch. To motivate it, you should show some > detailed performance measurements that show these wins.
Yes, this is expected for any patch posted. Zstd is an improved version of lz4, acting as a sort of industry standard these days, and any byte sequences I have looked at points that zstd leads kind of always to a better compression ratio for less or equivalent CPU cost compared to LZ4. Not saying that numbers are not required, they are. But I strongly suspect numbers among these lines. FWIW, it's not a complicated patch, it is a large mechanical patch that enforces a bunch of TOAST code paths to do what it wants. If we are going to do something about that and agree on something, I think that we should just use a new vartag_external for this matter (spoiler: I think we should use a new vartag_external value), but keep the toast structure at 16 bytes all the time, leaving alone the extra bit in the existing varatt_external structure so as there is no impact for heap relations if zstd is used, as long as the TOAST value is 32 bits. The patch introduces a new vartag_external with VARTAG_ONDISK_EXTENDED, so while it leads to a better compatibility, it also means that all zstd entries have to pay an extra amount of space in the main relation as an effect of a different default_toast_compression. The difficulty is not in the implementation, it would be on agreeing on what folks would be OK with in terms if vartag and varatt structures, and that's one of the oldest areas of the PG code, that has complications and assumptions of its own. The test implementation looks wrong to me. Why is there any need for an extra test module test_toast_ext? You could just reuse the same structure as compression_lz4.sql, but adapted to zstd. That's why a split with compression.sql has been done in 74a3fc36f314, FYI. You should also aim at splitting the patch more to make it easier to review: one of the sticky point of this area of the code is to untie completely DefaultCompressionId, its GUC and the TOAST code. The GUC default_toast_compression accepts by design only 4 values. This needs to go further, and should be refactored as a piece of its own. And also, I would prefer if the 32-bit value issue is tackled first, but that's a digression here, for a different thread. :) -- Michael
signature.asc
Description: PGP signature
