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
signature.asc
Description: PGP signature