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

Attachment: signature.asc
Description: PGP signature

Reply via email to