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