Re: Add LZ4 compression in pg_dump
Michael Paquier <michael@paquier.xyz>
Attachments
- dump-header-compress.patch (text/x-diff) patch
On Wed, Jan 25, 2023 at 12:00:20PM -0600, Justin Pryzby wrote:
> While looking at this, I realized that commit 5e73a6048 introduced a
> regression:
>
> @@ -3740,19 +3762,24 @@ ReadHead(ArchiveHandle *AH)
>
> - if (AH->compression != 0)
> - pg_log_warning("archive is compressed, but this installation does not support compression -- no data will be available");
> + if (AH->compression_spec.algorithm == PG_COMPRESSION_GZIP)
> + pg_fatal("archive is compressed, but this installation does not support compression");
>
> Before, it was possible to restore non-data chunks of a dump file, even
> if the current build didn't support its compression. But that's now
> impossible - and it makes the code we're discussing in RestoreArchive()
> unreachable.
Right. The impacts the possibility of looking at the header data,
which is useful with pg_restore -l for example. On a dump that's been
compressed, pg_restore <= 15 would always print the TOC entries with
or without compression support. On HEAD, this code prevents the
header lookup. All *nix or BSD platforms should have support for
zlib, I hope.. Still that could be an issue on Windows, and this
would prevent folks to check the contents of the dumps after saving it
on a WIN32 host, so let's undo that.
So, I have been testing the attached with four sets of binaries from
15/HEAD and with[out] zlib support, and this brings HEAD back to the
pre-15 state (header information able to show up, still failure when
attempting to restore the dump's data without zlib).
> I don't think we can currently test for that, since it requires creating a dump
> using a build --with compression and then trying to restore using a build
> --without compression.
Right, the location of the data is in the header, and I don't see how
you would be able to do that without two sets of binaries at hand, but
our tests run under the assumption that you have only one. Well,
that's not entirely true as well, as you could create a TAP test like
pg_upgrade that relies on a environment variable pointing to a second
set of binaries. That's not worth the complication involved, IMO.
> The coverage report disagrees with me, though...
> https://coverage.postgresql.org/src/bin/pg_dump/pg_backup_archiver.c.gcov.html#3901
Isn't that one of the tests like compression_gzip_plain?
Thoughts?
--
Michael
Commits
-
Advance input pointer when LZ4 compressing data
- 1a05c1d25299 16.0 landed
-
Null-terminate the output buffer of LZ4Stream_gets
- 3c18d90f8907 16.0 landed
-
Rework code defining default compression for dir/custom formats in pg_dump
- bedc1f0564d1 16.0 landed
-
pg_dump: Use only LZ4 frame format for compression
- 0070b66fef21 16.0 landed
-
Minor comment improvements for compress_lz4
- d0160ca11e31 16.0 landed
-
Unify buffer sizes in pg_dump compression API
- f081a48f9a91 16.0 landed
-
Improve type handling in pg_dump's compress file API
- d3b57755e60c 16.0 landed
-
Improve wording in pg_dump compression docs
- 6095069b40d7 16.0 landed
-
Fix condition in pg_dump TAP test
- 34ce11437497 16.0 landed
-
Add LZ4 compression to pg_dump
- 0da243fed087 16.0 landed
-
Introduce a generic pg_dump compression API
- e9960732a961 16.0 landed
-
Prepare pg_dump internals for additional compression methods
- 03d02f54a640 16.0 landed
-
Fix behavior with pg_restore -l and compressed dumps
- 783d8abc3b63 16.0 landed
-
Add 250c8ee07ed to git-blame-ignore-revs
- ff23b592ad66 16.0 cited
-
Provide test coverage in pg_dump for default behaviors with compression
- a7885c9bb22d 16.0 landed
-
Switch pg_dump to use compression specifications
- 5e73a6048849 16.0 landed
-
Refactor code parsing compression option values (-Z/--compress)
- d18655cc037a 16.0 landed
-
meson: Add some missing env settings for tests of pg_dump and pg_verifybackup
- 00ae5d6f588e 16.0 landed
-
Extend TAP tests of pg_dump to test for compression with gzip
- 98fe74218d97 15.0 landed
-
Clean up some dead code in pg_dump with tar format and gzip compression
- 8ac4c25a05d1 15.0 landed
-
Add TAP test in pg_dump with --format=tar and --compress
- edcedcc2c7bb 15.0 landed
-
Replace BASE_BACKUP COMPRESSION_LEVEL option with COMPRESSION_DETAIL.
- ffd53659c46a 15.0 cited
-
Refactor the pg_dump zlib code from pg_backup_custom.c to a separate file,
- bf9aa490db24 9.1.0 cited