Re: ZSTD TOAST compression, and an extensible compression method encoding

Hi Nikhil,

Thanks for the updated patch set! Looking at patch 4/4, I noticed a minor
issue in zstd_compress_datum() regarding the check for incompressible data:

/* data is incompressible so just free the memory and return NULL */
if (len > (size_t) valsize)
{
pfree(tmp);
return NULL;
}
Should this condition be >= instead of >? if (len >= (size_t) valsize)
Reasons:
If len == valsize, the compressed payload alone is already the same size as
the uncompressed data. Once VARHDRSZ_COMPRESSED_LONG (9 bytes) is added,
the compressed datum is strictly larger than the original. There is no
compression benefit here.
If len == valsize, zstd_compress_datum() currently proceeds to call
SET_VARSIZE_COMPRESSED() and returns tmp, only for the caller
toast_compress_datum() to immediately reject it via if (VARSIZE(tmp) <
valsize - 2) and pfree(tmp). Freeing it early and returning NULL saves
redundant operations.
This is also consistent with lz4_compress_datum() in the same file:

/*
* If the compressed size is greater than or equal to the raw data size,
* then data is incompressible so just free the memory and return NULL.
*/
if (len >= valsize)
{
pfree(tmp);
return NULL;
}