Обсуждение: pg_dump -Fd and compression level
Hello, After our last upgrade, we've noticed a 10-20% size increase of our dump size. This comes from our backup scripts were pg_dump was called without setting -Z So it seems, that this fix did modify the default compression to use: http://michael.otacoo.com/postgresql-2/pg_dump-directory-format-compression/ not sure if this is expected or if this commit accidently changed the default compression, setting it too low. moreover, the doc is somewhat unclear here as it mentions all formats but the directory one: -Z 0..9 --compress=0..9 Specify the compression level to use. Zero means no compression. For the custom archive format, this specifies compressionof individual table-data segments, and the default is to compress at a moderate level. For plain text output,setting a nonzero compression level causes the entire output file to be compressed, as though it had been fed throughgzip; but the default is not to compress. The tar archive format currently does not support compression at all. shouldn't that be changed to - For the custom archive format + For the directory and custom archive formats best regards, Marc Mamin
On 07/24/2015 02:52 AM, Marc Mamin wrote: > Hello, > > After our last upgrade, we've noticed a 10-20% size increase of our dump size. > This comes from our backup scripts were pg_dump was called without setting -Z > > So it seems, that this fix did modify the default compression to use: > http://michael.otacoo.com/postgresql-2/pg_dump-directory-format-compression/ > > not sure if this is expected or if this commit accidently changed the default compression, setting it too low. > > moreover, the doc is somewhat unclear here as it mentions all formats but the directory one: > > -Z 0..9 > --compress=0..9 > > Specify the compression level to use. Zero means no compression. > For the custom archive format, this specifies compression of individual > table-data segments, and the default is to compress at a moderate level. > For plain text output, setting a nonzero compression level causes the entire > output file to be compressed, as though it had been fed through gzip; > but the default is not to compress. > The tar archive format currently does not support compression at all. > > shouldn't that be changed to > > - For the custom archive format > + For the directory and custom archive formats > > What did you upgrade from/to? cheers andrew
>> After our last upgrade, we've noticed a 10-20% size increase of our dump size. >> This comes from our backup scripts were pg_dump was called without setting -Z >> >> So it seems, that this fix did modify the default compression to use: >> http://michael.otacoo.com/postgresql-2/pg_dump-directory-format-compression/ >> >> not sure if this is expected or if this commit accidently changed the default compression, setting it too low. >> >> moreover, the doc is somewhat unclear here as it mentions all formats but the directory one: >> >> -Z 0..9 >> --compress=0..9 >> >> Specify the compression level to use. Zero means no compression. >> For the custom archive format, this specifies compression of individual >> table-data segments, and the default is to compress at a moderate level. >> For plain text output, setting a nonzero compression level causes the entire >> output file to be compressed, as though it had been fed through gzip; >> but the default is not to compress. >> The tar archive format currently does not support compression at all. >> >> shouldn't that be changed to >> >> - For the custom archive format >> + For the directory and custom archive formats >> >> > >What did you upgrade from/to? 9.3.6 to 9.3.9 this is bound to this 9.3.7 fix: "In pg_dump, fix failure to honor -Z compression level option together with -Fd (Michael Paquier)" I understand that the modification is wishfull, but the change has nevertheless a non negligable impact. This had increased our backup repository of about 1TB within a few weeks if we hadn't noticed it. regards, Marc mamin
On 07/24/2015 05:22 PM, Marc Mamin wrote: >>> After our last upgrade, we've noticed a 10-20% size increase of our dump size. >>> This comes from our backup scripts were pg_dump was called without setting -Z >>> >>> So it seems, that this fix did modify the default compression to use: >>> http://michael.otacoo.com/postgresql-2/pg_dump-directory-format-compression/ >>> >>> not sure if this is expected or if this commit accidently changed the default compression, setting it too low. >>> >>> moreover, the doc is somewhat unclear here as it mentions all formats but the directory one: >>> >>> -Z 0..9 >>> --compress=0..9 >>> >>> Specify the compression level to use. Zero means no compression. >>> For the custom archive format, this specifies compression of individual >>> table-data segments, and the default is to compress at a moderate level. >>> For plain text output, setting a nonzero compression level causes the entire >>> output file to be compressed, as though it had been fed through gzip; >>> but the default is not to compress. >>> The tar archive format currently does not support compression at all. >>> >>> shouldn't that be changed to >>> >>> - For the custom archive format >>> + For the directory and custom archive formats >>> >>> >> What did you upgrade from/to? > 9.3.6 to 9.3.9 > > this is bound to this 9.3.7 fix: > "In pg_dump, fix failure to honor -Z compression level option together with -Fd (Michael Paquier)" > > I understand that the modification is wishfull, but the change has nevertheless a non negligable impact. > This had increased our backup repository of about 1TB within a few weeks if we hadn't noticed it. > > Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 changed from using the default compression for libz to using the compression set in pg_dump options, which defaults to 0. This actually seems like the right thing to do, but it certainly should have been called out much more forcefully in release notes, and arguably should not have been changed in stable releases. Not sure what we do about it now. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911
> changed from using the default compression for libz to using the
> compression set in pg_dump options, which defaults to 0. This actually
> seems like the right thing to do, but it certainly should have been
> called out much more forcefully in release notes, and arguably should
> not have been changed in stable releases. Not sure what we do about it now.
I don't think we realized that we were changing the default behavior.
Still, it's clearly a bug fix, so I'm disinclined to revert it now.
regards, tom lane
>Andrew Dunstan <andrew@dunslane.net> writes: >> Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >> changed from using the default compression for libz to using the >> compression set in pg_dump options, which defaults to 0. This actually >> seems like the right thing to do, but it certainly should have been >> called out much more forcefully in release notes, and arguably should >> not have been changed in stable releases. Not sure what we do about it now. really 0? wouldn't that mean no compression at all? >I don't think we realized that we were changing the default behavior. >Still, it's clearly a bug fix, so I'm disinclined to revert it now. > > regards, tom lane Sure, but at least the doc should be modified as it suggests that a higher compression is used: "the default is to compress at a moderate level" => "the default is to compress at the lowest level" And maybe a quick notice for planet Postgres to highlight the modification of the default behavior. (I' haven't a blog myself ...) IMHO a slightly higher default would help spare some hardware out there... regards, Marc Mamin
On 07/25/2015 02:34 AM, Marc Mamin wrote: > >> Andrew Dunstan <andrew@dunslane.net> writes: >>> Hmm. Yeah. It looks like commit a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >>> changed from using the default compression for libz to using the >>> compression set in pg_dump options, which defaults to 0. This actually >>> seems like the right thing to do, but it certainly should have been >>> called out much more forcefully in release notes, and arguably should >>> not have been changed in stable releases. Not sure what we do about it now. > really 0? wouldn't that mean no compression at all? No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in the default case. And it is definitely compressing some. So I'm now puzzled by what you're seeing. cheers andrew
On 07/25/2015 03:20 AM, Andrew Dunstan wrote: > > On 07/25/2015 02:34 AM, Marc Mamin wrote: >> >>> Andrew Dunstan <andrew@dunslane.net> writes: >>>> Hmm. Yeah. It looks like commit >>>> a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >>>> changed from using the default compression for libz to using the >>>> compression set in pg_dump options, which defaults to 0. This actually >>>> seems like the right thing to do, but it certainly should have been >>>> called out much more forcefully in release notes, and arguably should >>>> not have been changed in stable releases. Not sure what we do about >>>> it now. >> really 0? wouldn't that mean no compression at all? > > No, that's not right either. The default should be > Z_DEFAULT_COMPRESSION, so we shouldn't actually see a difference in > the default case. And it is definitely compressing some. So I'm now > puzzled by what you're seeing. > > OK, I have got this worked out. I'll have a bug fix shortly. cheers andrew
On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan <andrew@dunslane.net> wrote: > > On 07/25/2015 03:20 AM, Andrew Dunstan wrote: >> >> >> On 07/25/2015 02:34 AM, Marc Mamin wrote: >>> >>> >>>> Andrew Dunstan <andrew@dunslane.net> writes: >>>>> >>>>> Hmm. Yeah. It looks like commit >>>>> a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >>>>> changed from using the default compression for libz to using the >>>>> compression set in pg_dump options, which defaults to 0. This actually >>>>> seems like the right thing to do, but it certainly should have been >>>>> called out much more forcefully in release notes, and arguably should >>>>> not have been changed in stable releases. Not sure what we do about it >>>>> now. >>> >>> really 0? wouldn't that mean no compression at all? >> >> >> No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, >> so we shouldn't actually see a difference in the default case. And it is >> definitely compressing some. So I'm now puzzled by what you're seeing. >> >> > > > OK, I have got this worked out. I'll have a bug fix shortly. So you are basically planning to switch to Z_BEST_SPEED instead of Z_DEFAULT_COMPRESSION where needed for the default code path? -- Michael
>On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >> >> On 07/25/2015 03:20 AM, Andrew Dunstan wrote: >>> >>> >>> On 07/25/2015 02:34 AM, Marc Mamin wrote: >>>> >>>> >>>>> Andrew Dunstan <andrew@dunslane.net> writes: >>>>>> >>>>>> Hmm. Yeah. It looks like commit >>>>>> a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >>>>>> changed from using the default compression for libz to using the >>>>>> compression set in pg_dump options, which defaults to 0. This actually >>>>>> seems like the right thing to do, but it certainly should have been >>>>>> called out much more forcefully in release notes, and arguably should >>>>>> not have been changed in stable releases. Not sure what we do about it >>>>>> now. >>>> >>>> really 0? wouldn't that mean no compression at all? >>> >>> >>> No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, >>> so we shouldn't actually see a difference in the default case. And it is >>> definitely compressing some. So I'm now puzzled by what you're seeing. >>> >>> >> >> >> OK, I have got this worked out. I'll have a bug fix shortly. > >So you are basically planning to switch to Z_BEST_SPEED instead of >Z_DEFAULT_COMPRESSION where needed for the default code path? >-- >Michael It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be thecase. From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in -Z1. this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. e.g. pg_dump -Z-2 is like pg_dump -Z2 Marc
On Sat, Jul 25, 2015 at 11:09 PM, Marc Mamin <M.Mamin@intershop.de> wrote: > >>On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >>> >>> On 07/25/2015 03:20 AM, Andrew Dunstan wrote: >>>> >>>> >>>> On 07/25/2015 02:34 AM, Marc Mamin wrote: >>>>> >>>>> >>>>>> Andrew Dunstan <andrew@dunslane.net> writes: >>>>>>> >>>>>>> Hmm. Yeah. It looks like commit >>>>>>> a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >>>>>>> changed from using the default compression for libz to using the >>>>>>> compression set in pg_dump options, which defaults to 0. This actually >>>>>>> seems like the right thing to do, but it certainly should have been >>>>>>> called out much more forcefully in release notes, and arguably should >>>>>>> not have been changed in stable releases. Not sure what we do about it >>>>>>> now. >>>>> >>>>> really 0? wouldn't that mean no compression at all? >>>> >>>> >>>> No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, >>>> so we shouldn't actually see a difference in the default case. And it is >>>> definitely compressing some. So I'm now puzzled by what you're seeing. >>>> >>>> >>> >>> >>> OK, I have got this worked out. I'll have a bug fix shortly. >> >>So you are basically planning to switch to Z_BEST_SPEED instead of >>Z_DEFAULT_COMPRESSION where needed for the default code path? > > It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be thecase. > > From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in -Z1.this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. I think I understand what is happening... With a quick test using the default compression level in directory mode, gzopen is called with w-1. By decrypting the docs of zlib (http://www.zlib.net/manual.html), section File Access Functions, it seems to me that calling gzopen like that will cause the file to not be compressed at all, which is actually why you are seeing an increase in your dump files, while we should call it with a compression mode of 6 actually based on what Z_DEFAULT_COMPRESSION means for zlib now, or simply without a compression level specified such as the default is used by zlib. Prior to a7ad5cf0, what we did was actually gzopen with always "w" or "wb" that caused the default compression level of 6 to be used. Hence I think that we should definitely use "w" or "wb" without specifying a level number when the compression level is Z_DEFAULT_COMPRESSION, and "wbN" (0<=N<=9) when a compression level is given by the user. A patch is attached for this purpose. Marc, does it work for you? > e.g. pg_dump -Z-2 is like pg_dump -Z2 The compression needs to be included in range [0,9]. -- Michael
Вложения
>On Sat, Jul 25, 2015 at 11:09 PM, Marc Mamin <M.Mamin@intershop.de> wrote: >> >>>On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >>>> >>>> On 07/25/2015 03:20 AM, Andrew Dunstan wrote: >>>>> >>>>> >>>>> On 07/25/2015 02:34 AM, Marc Mamin wrote: >>>>>> >>>>>> >>>>>>> Andrew Dunstan <andrew@dunslane.net> writes: >>>>>>>> >>>>>>>> Hmm. Yeah. It looks like commit >>>>>>>> a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >>>>>>>> changed from using the default compression for libz to using the >>>>>>>> compression set in pg_dump options, which defaults to 0. This actually >>>>>>>> seems like the right thing to do, but it certainly should have been >>>>>>>> called out much more forcefully in release notes, and arguably should >>>>>>>> not have been changed in stable releases. Not sure what we do about it >>>>>>>> now. >>>>>> >>>>>> really 0? wouldn't that mean no compression at all? >>>>> >>>>> >>>>> No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, >>>>> so we shouldn't actually see a difference in the default case. And it is >>>>> definitely compressing some. So I'm now puzzled by what you're seeing. >>>>> >>>>> >>>> >>>> >>>> OK, I have got this worked out. I'll have a bug fix shortly. >>> >>>So you are basically planning to switch to Z_BEST_SPEED instead of >>>Z_DEFAULT_COMPRESSION where needed for the default code path? >> >> It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be thecase. >> >> From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in-Z1. this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. > >I think I understand what is happening... With a quick test using the >default compression level in directory mode, gzopen is called with >w-1. By decrypting the docs of zlib (http://www.zlib.net/manual.html), >section File Access Functions, it seems to me that calling gzopen like >that will cause the file to not be compressed at all, which is >actually why you are seeing an increase in your dump files, But it does compress. Seems to be the same as -Z1 >while we >should call it with a compression mode of 6 actually based on what >Z_DEFAULT_COMPRESSION means for zlib now, or simply without a >compression level specified such as the default is used by zlib. Prior >to a7ad5cf0, what we did was actually gzopen with always "w" or "wb" >that caused the default compression level of 6 to be used. Hence I >think that we should definitely use "w" or "wb" without specifying a >level number when the compression level is Z_DEFAULT_COMPRESSION, and >"wbN" (0<=N<=9) when a compression level is given by the user. A patch >is attached for this purpose. Marc, does it work for you? I'm on vacation and won't be able to test patches for the 2 next weeks. sorry > >> e.g. pg_dump -Z-2 is like pg_dump -Z2 > >The compression needs to be included in range [0,9]. Yes, but pg_dump is not such strict. e.g. pg_dump -Zx is accepted and leads to no compression These 2 calls result in the same compression, which is how I came to the idea that the minus sign of Z_DEFAULT_COMPRESSION might get lost on the way: pg_dump -Z9 ... pg_dump -Z-9 ... Marc >-- >Michael >
On 07/25/2015 10:50 AM, Michael Paquier wrote: > On Sat, Jul 25, 2015 at 11:09 PM, Marc Mamin <M.Mamin@intershop.de> wrote: >>> On Sat, Jul 25, 2015 at 9:56 PM, Andrew Dunstan <andrew@dunslane.net> wrote: >>>> On 07/25/2015 03:20 AM, Andrew Dunstan wrote: >>>>> >>>>> On 07/25/2015 02:34 AM, Marc Mamin wrote: >>>>>> >>>>>>> Andrew Dunstan <andrew@dunslane.net> writes: >>>>>>>> Hmm. Yeah. It looks like commit >>>>>>>> a7ad5cf0cfcfab8418000d652fa4f0c6ad6c8911 >>>>>>>> changed from using the default compression for libz to using the >>>>>>>> compression set in pg_dump options, which defaults to 0. This actually >>>>>>>> seems like the right thing to do, but it certainly should have been >>>>>>>> called out much more forcefully in release notes, and arguably should >>>>>>>> not have been changed in stable releases. Not sure what we do about it >>>>>>>> now. >>>>>> really 0? wouldn't that mean no compression at all? >>>>> >>>>> No, that's not right either. The default should be Z_DEFAULT_COMPRESSION, >>>>> so we shouldn't actually see a difference in the default case. And it is >>>>> definitely compressing some. So I'm now puzzled by what you're seeing. >>>>> >>>>> >>>> >>>> OK, I have got this worked out. I'll have a bug fix shortly. >>> So you are basically planning to switch to Z_BEST_SPEED instead of >>> Z_DEFAULT_COMPRESSION where needed for the default code path? >> It looks like Z_DEFAULT_COMPRESSION was the used value before, and from the source code I guess this should still be thecase. >> >> From a quick testing, it now behaves as if the minus sign from Z_DEFAULT_COMPRESSION is lost on the way, resulting in-Z1. this might indicate that Z_DEFAULT_COMPRESSION is parsed like the command line argument. gzopen only allows a digit, so -1 results in the - being ignored and 1 being the compression value. It's a pity gzopen is so liberal about accepting modes with values it ignores, or we'd have noticed this ages ago. > I think I understand what is happening... With a quick test using the > default compression level in directory mode, gzopen is called with > w-1. By decrypting the docs of zlib (http://www.zlib.net/manual.html), > section File Access Functions, it seems to me that calling gzopen like > that will cause the file to not be compressed at all, which is > actually why you are seeing an increase in your dump files, while we > should call it with a compression mode of 6 actually based on what > Z_DEFAULT_COMPRESSION means for zlib now, or simply without a > compression level specified such as the default is used by zlib. Prior > to a7ad5cf0, what we did was actually gzopen with always "w" or "wb" > that caused the default compression level of 6 to be used. Hence I > think that we should definitely use "w" or "wb" without specifying a > level number when the compression level is Z_DEFAULT_COMPRESSION, and > "wbN" (0<=N<=9) when a compression level is given by the user. A patch > is attached for this purpose. Marc, does it work for you? > >> e.g. pg_dump -Z-2 is like pg_dump -Z2 > The compression needs to be included in range [0,9]. I propose to tighten pg_dump's rules so that only 0..9 are accepted as arguments for -Z, and in compress_io.c:cfopen(), if compression is equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value to the mode, thus using the zlib default. cheers andrew
On 07/25/2015 01:52 PM, I wrote: > > > > I propose to tighten pg_dump's rules so that only 0..9 are accepted as > arguments for -Z, and in compress_io.c:cfopen(), if compression is > equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value > to the mode, thus using the zlib default. > > As per attached patch. Comments? cheers andrew
Вложения
Andrew Dunstan <andrew@dunslane.net> writes:
> On 07/25/2015 01:52 PM, I wrote:
>> I propose to tighten pg_dump's rules so that only 0..9 are accepted as
>> arguments for -Z, and in compress_io.c:cfopen(), if compression is
>> equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value
>> to the mode, thus using the zlib default.
> As per attached patch.
> Comments?
Looks OK to me. Thanks for fixing it!
regards, tom lane
> >> >> I propose to tighten pg_dump's rules so that only 0..9 are accepted as >> arguments for -Z, and in compress_io.c:cfopen(), if compression is >> equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value >> to the mode, thus using the zlib default. >> >> > > >As per attached patch. > >Comments? It seems that the first test on the compression in pg_backup_tar.c is now obsolete. It didn't make much sense anyway. 211 if (AH->compression < 0 || AH->compression > 9) 212 AH->compression = Z_DEFAULT_COMPRESSION; 213 214 /* Don't compress into tar files unless asked to do so */ 215 if (AH->compression == Z_DEFAULT_COMPRESSION)216 AH->compression = 0; 217 218 /* 219 * We don't support compressionbecause reading the files back is not 220 * possible since gzdopen uses buffered IO which totally screwsfile 221 * positioning. 222 */ 223 if (AH->compression != 0) 224 exit_horribly(modulename,225 "compression is not supported by tar archive format\n"); 226 } regards,Marc > >cheers > >andrew
On 07/25/2015 03:07 PM, Marc Mamin wrote: >>> I propose to tighten pg_dump's rules so that only 0..9 are accepted as >>> arguments for -Z, and in compress_io.c:cfopen(), if compression is >>> equal to Z_DEFAULT_COMPRESSION, not add any explicit compression value >>> to the mode, thus using the zlib default. >>> >>> >> >> As per attached patch. >> >> Comments? > > It seems that the first test on the compression in pg_backup_tar.c is now obsolete. > It didn't make much sense anyway. > > > > 211 if (AH->compression < 0 || AH->compression > 9) > 212 AH->compression = Z_DEFAULT_COMPRESSION; > 213 > 214 /* Don't compress into tar files unless asked to do so */ > 215 if (AH->compression == Z_DEFAULT_COMPRESSION) > 216 AH->compression = 0; > 217 > 218 /* > 219 * We don't support compression because reading the files back is not > 220 * possible since gzdopen uses buffered IO which totally screws file > 221 * positioning. > 222 */ > 223 if (AH->compression != 0) > 224 exit_horribly(modulename, > 225 "compression is not supported by tar archive format\n"); > 226 } > > In fact, the first two tests look unnecessary. Neither condition should be possible now. cheers andrew
>>> >>> As per attached patch. >>> >>> Comments? >> >> It seems that the first test on the compression in pg_backup_tar.c is now obsolete. >> It didn't make much sense anyway. >> >> >> >> 211 if (AH->compression < 0 || AH->compression > 9) >> 212 AH->compression = Z_DEFAULT_COMPRESSION; >> 213 >> 214 /* Don't compress into tar files unless asked to do so */ >> 215 if (AH->compression == Z_DEFAULT_COMPRESSION) >> 216 AH->compression = 0; >> 217 >> 218 /* >> 219 * We don't support compression because reading the files back is not >> 220 * possible since gzdopen uses buffered IO which totally screws file >> 221 * positioning. >> 222 */ >> 223 if (AH->compression != 0) >> 224 exit_horribly(modulename, >> 225 "compression is not supported by tar archive format\n"); >> 226 } >> >> > >In fact, the first two tests look unnecessary. Neither condition should >be possible now. > Hello, Isn't the second test still required if you call pg_dump -Ft without setting -Z0 explicitly ? (=> AH->compression == Z_DEFAULT_COMPRESSION) There still are a few suspicious places in pg_backup_tar.c that refer to the compression although not supported (except for blob ?) (C programming is beyond my capabilities, I can roughly read simple code ... ) regards, Marc Mamin
On 07/27/2015 03:52 AM, Marc Mamin wrote: >>>> As per attached patch. >>>> >>>> Comments? >>> It seems that the first test on the compression in pg_backup_tar.c is now obsolete. >>> It didn't make much sense anyway. >>> >>> >>> >>> 211 if (AH->compression < 0 || AH->compression > 9) >>> 212 AH->compression = Z_DEFAULT_COMPRESSION; >>> 213 >>> 214 /* Don't compress into tar files unless asked to do so */ >>> 215 if (AH->compression == Z_DEFAULT_COMPRESSION) >>> 216 AH->compression = 0; >>> 217 >>> 218 /* >>> 219 * We don't support compression because reading the files back is not >>> 220 * possible since gzdopen uses buffered IO which totally screws file >>> 221 * positioning. >>> 222 */ >>> 223 if (AH->compression != 0) >>> 224 exit_horribly(modulename, >>> 225 "compression is not supported by tar archive format\n"); >>> 226 } >>> >>> >> In fact, the first two tests look unnecessary. Neither condition should >> be possible now. >> > Hello, > > Isn't the second test still required if you call pg_dump -Ft without setting -Z0 explicitly ? > (=> AH->compression == Z_DEFAULT_COMPRESSION) No. Z_DEFAULT_COMPRESSION is only set for directory and custom archive types. See pg_dump.c at lines 578-592. cheers andrew