Thank you for review comments. >There are still numerous formatting changes required, e.g. spaces around >"=" and correct formatting of comments. And "git diff --check" still has >a few whitespace problems. I won't point these out one by one, but maybe >you should run pgindent
I will do this. >Could you look into his suggestions of other places to do the >allocation, please? I will get back to you on this >Wouldn't it be better to set > bkpb->block_compression = compress_backup_block; >once earlier instead of setting it that way once and setting it to >BACKUP_BLOCK_COMPRESSION_OFF in two other places Yes. If you're using VARATT_IS_COMPRESSED() to detect compression, don't you need SET_VARSIZE_COMPRESSED() in CompressBackupBlock? pglz_compress() does it for you, but the other two algorithms don't. Yes we need SET_VARSIZE_COMPRESSED. It is present in wrappers around snappy and LZ4 namely pg_snappy_compress and pg_LZ4_compress. >But now that you've added bkpb.block_compression, you should be able to >avoid VARATT_IS_COMPRESSED() altogether, unless I'm missing something. >What do you think? You are right. It can be removed. Thank you, On Fri, Jul 4, 2014 at 9:35 PM, Abhijit Menon-Sen <a...@2ndquadrant.com> wrote: > At 2014-07-04 21:02:33 +0530, a...@2ndquadrant.com wrote: > > > > > +/* > > > + */ > > > +static const struct config_enum_entry > backup_block_compression_options[] = { > > Oh, I forgot to mention that the configuration setting changes are also > pending. I think we had a working consensus to use full_page_compression > as the name of the GUC. As I understand it, that'll accept an algorithm > name as an argument while we're still experimenting, but eventually once > we select an algorithm, it'll become just a boolean (and then we don't > need to put algorithm information into BkpBlock any more either). > > -- Abhijit >