Re: Different compression methods for FPI

On Sun, Jun 20, 2021 at 11:15:08PM +0500, Andrey Borodin wrote:
> I have some small questions.
> 
> 1.
> +			report_invalid_record(record, "image at %X/%X compressed with %s not supported, block %d",
> +								  (uint32) (record->ReadRecPtr >> 32),
> +								  (uint32) record->ReadRecPtr,
> +								  "lz4",
> +								  block_id);
> Can we point out to user that the problem is in the build?

What about the following error then?  Say:
"image at %X/%X compressed with LZ4 not supported by build, block
%d".

> Also, maybe %s can be inlined to lz4 in this case.

I think that's a remnant of the zstd part of the patch set, where I
wanted to have only one translatable message.  Sure, I can align lz4
with the message.

> 2.
> > const char *method = "???";
> Maybe we can use something like "unknown" for unknown compression
> methods? Or is it too long string for waldump output?

A couple of extra bytes for pg_waldump will not matter much.  Using
"unknown" is fine by me.

> 3. Can we exclude lz4 from config if it's not supported by the build?

Enforcing the absence of this value at GUC level is enough IMO:
+static const struct config_enum_entry wal_compression_options[] = {
+   {"pglz", WAL_COMPRESSION_PGLZ, false},
+#ifdef USE_LZ4
+   {"lz4", WAL_COMPRESSION_LZ4, false},
+#endif
[...]
+typedef enum WalCompression
+{
+   WAL_COMPRESSION_NONE = 0,
+   WAL_COMPRESSION_PGLZ,
+   WAL_COMPRESSION_LZ4
+} WalCompression;

It is not possible to set the GUC, still it is listed in the enum that
allows us to track it.  That's the same thing as
default_toast_compression with its ToastCompressionId and its
default_toast_compression_options.
--
Michael