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 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 formatsbest regards,
Marc Mamin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 -ZSo 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..9Specify 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 -ZSo 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..9Specify 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 formatsWhat 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 -ZSo 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..9Specify 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 formatsWhat 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 the case.
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 the case.
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
Attachments:
20150725_dump_compress.patchtext/x-patch; charset=US-ASCII; name=20150725_dump_compress.patchDownload
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 912fc2f..10d52c8 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -549,8 +549,12 @@ cfopen(const char *path, const char *mode, int compression)
#ifdef HAVE_LIBZ
char mode_compression[32];
- snprintf(mode_compression, sizeof(mode_compression), "%s%d",
- mode, compression);
+ if (compression == Z_DEFAULT_COMPRESSION)
+ snprintf(mode_compression, sizeof(mode_compression), "%s",
+ mode);
+ else
+ snprintf(mode_compression, sizeof(mode_compression), "%s%d",
+ mode, compression);
fp->compressedfp = gzopen(path, mode_compression);
fp->uncompressedfp = NULL;
if (fp->compressedfp == NULL)
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 the case.
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 the case.
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
Attachments:
pg_dump_compression_fix.patchtext/x-diff; name=pg_dump_compression_fix.patchDownload
diff --git a/src/bin/pg_dump/compress_io.c b/src/bin/pg_dump/compress_io.c
index 912fc2f..6e1469b 100644
--- a/src/bin/pg_dump/compress_io.c
+++ b/src/bin/pg_dump/compress_io.c
@@ -547,11 +547,21 @@ cfopen(const char *path, const char *mode, int compression)
if (compression != 0)
{
#ifdef HAVE_LIBZ
- char mode_compression[32];
+ if (compression != Z_DEFAULT_COMPRESSION)
+ {
+ /* user has specified a compression level, so tell zlib to use it */
+ char mode_compression[32];
+
+ snprintf(mode_compression, sizeof(mode_compression), "%s%d",
+ mode, compression);
+ fp->compressedfp = gzopen(path, mode_compression);
+ }
+ else
+ {
+ /* don't specify a level, just use the zlib default */
+ fp->compressedfp = gzopen(path, mode);
+ }
- snprintf(mode_compression, sizeof(mode_compression), "%s%d",
- mode, compression);
- fp->compressedfp = gzopen(path, mode_compression);
fp->uncompressedfp = NULL;
if (fp->compressedfp == NULL)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 0e036b8..b24b8bd 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -485,6 +485,11 @@ main(int argc, char **argv)
case 'Z': /* Compression Level */
compressLevel = atoi(optarg);
+ if (compressLevel < 0 || compressLevel > 9)
+ {
+ write_msg(NULL, "compression level must be in range 0..9\n");
+ exit_nicely(1);
+ }
break;
case 0:
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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 }
regards,
Marc
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers