Re: Add LZ4 compression in pg_dump

On 5/17/23 10:59, Tomas Vondra wrote: > On 5/17/23 08:18, Michael Paquier wrote: >> On Tue, May 09, 2023 at 02:54:31PM +0200, Tomas Vondra wrote: >>> Yeah. Thanks for the report, should have been found during review. >> >> Tomas, are you planning to do something by the end of this week for >>

Re: Add LZ4 compression in pg_dump

On 5/17/23 08:18, Michael Paquier wrote: > On Tue, May 09, 2023 at 02:54:31PM +0200, Tomas Vondra wrote: >> Yeah. Thanks for the report, should have been found during review. > > Tomas, are you planning to do something by the end of this week for > beta1? Or do you need some help of any kind?

Re: Add LZ4 compression in pg_dump

On Tue, May 09, 2023 at 02:54:31PM +0200, Tomas Vondra wrote: > Yeah. Thanks for the report, should have been found during review. Tomas, are you planning to do something by the end of this week for beta1? Or do you need some help of any kind? -- Michael signature.asc Description: PGP

Re: Add LZ4 compression in pg_dump

On Tue, May 09, 2023 at 02:12:44PM +, gkokola...@pm.me wrote: > Thank you both for looking. A small consolation is that now there are > tests for this case. +1, noticing that was pure luck ;) Worth noting that the patch posted in [1] has these tests, not the version posted in [2]. +

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Tuesday, May 9th, 2023 at 2:54 PM, Tomas Vondra wrote: > > > On 5/9/23 00:10, Michael Paquier wrote: > > > On Mon, May 08, 2023 at 08:00:39PM +0200, Tomas Vondra wrote: > > > > > The LZ4Stream_write() forgot to move the pointer to the next chunk, so

Re: Add LZ4 compression in pg_dump

On 5/9/23 00:10, Michael Paquier wrote: > On Mon, May 08, 2023 at 08:00:39PM +0200, Tomas Vondra wrote: >> The LZ4Stream_write() forgot to move the pointer to the next chunk, so >> it was happily decompressing the initial chunk over and over. A bit >> embarrassing oversight :-( >> >> The custom

Re: Add LZ4 compression in pg_dump

On Mon, May 08, 2023 at 08:00:39PM +0200, Tomas Vondra wrote: > The LZ4Stream_write() forgot to move the pointer to the next chunk, so > it was happily decompressing the initial chunk over and over. A bit > embarrassing oversight :-( > > The custom format calls WriteDataToArchiveLZ4(), which was

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Monday, May 8th, 2023 at 8:20 PM, Tomas Vondra wrote: > > > > > On 5/8/23 18:19, gkokola...@pm.me wrote: > > > --- Original Message --- > > On Monday, May 8th, 2023 at 3:16 AM, Tom Lane t...@sss.pgh.pa.us wrote: > > > > > I wrote: > > > >

Re: Add LZ4 compression in pg_dump

On 5/8/23 18:19, gkokola...@pm.me wrote: > > > > > > --- Original Message --- > On Monday, May 8th, 2023 at 3:16 AM, Tom Lane wrote: > > >> >> >> I wrote: >> >>> Michael Paquier mich...@paquier.xyz writes: >>> While testing this patch, I have triggered an error pointing out

Re: Add LZ4 compression in pg_dump

On 5/8/23 03:16, Tom Lane wrote: > I wrote: >> Michael Paquier writes: >>> While testing this patch, I have triggered an error pointing out that >>> the decompression path of LZ4 is broken for table data. I can >>> reproduce that with a dump of the regression database, as of: >>> make

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Monday, May 8th, 2023 at 3:16 AM, Tom Lane wrote: > > > I wrote: > > > Michael Paquier mich...@paquier.xyz writes: > > > > > While testing this patch, I have triggered an error pointing out that > > > the decompression path of LZ4 is broken for table

Re: Add LZ4 compression in pg_dump

On 5/7/23 17:01, gkokola...@pm.me wrote: > > > > On Sat, May 6, 2023 at 04:51, Michael Paquier wrote: >> On Fri, May 05, 2023 at 02:13:28PM +, gkokola...@pm.me wrote: >> > Good point. I thought about it before submitting the

Re: Add LZ4 compression in pg_dump

I wrote: > Michael Paquier writes: >> While testing this patch, I have triggered an error pointing out that >> the decompression path of LZ4 is broken for table data. I can >> reproduce that with a dump of the regression database, as of: >> make installcheck >> pg_dump --format=d --file=dump_lz4

Re: Add LZ4 compression in pg_dump

On Sun, May 07, 2023 at 09:09:25PM -0400, Tom Lane wrote: > Ugh. Reproduced here ... so we need an open item for this. Yep. Already added. -- Michael signature.asc Description: PGP signature

Re: Add LZ4 compression in pg_dump

Michael Paquier writes: > While testing this patch, I have triggered an error pointing out that > the decompression path of LZ4 is broken for table data. I can > reproduce that with a dump of the regression database, as of: > make installcheck > pg_dump --format=d --file=dump_lz4 --compress=lz4

Re: Add LZ4 compression in pg_dump

On Fri, May 05, 2023 at 02:13:28PM +, gkokola...@pm.me wrote: > Good point. I thought about it before submitting the patch. I > concluded that given the complexity and operations involved in > LZ4Stream_read_internal() and the rest of t he pg_dump/pg_restore > code, the memset() call will be

Re: Add LZ4 compression in pg_dump

On Sun, May 07, 2023 at 03:01:52PM +, gkokola...@pm.me wrote: > Thank you but I am not certain I know what that means. Can you please explain? It means that this thread has been added to the following list: https://wiki.postgresql.org/wiki/PostgreSQL_16_Open_Items#Open_Issues

Re: Add LZ4 compression in pg_dump

On Sat, May 6, 2023 at 04:51, Michael Paquier <[mich...@paquier.xyz](mailto:On Sat, May 6, 2023 at 04:51, Michael Paquier < wrote: > On Fri, May 05, 2023 at 02:13:28PM +, gkokola...@pm.me wrote: >> Good point. I thought about it before submitting the patch. I >> concluded that given the

Re: Add LZ4 compression in pg_dump

On Fri, May 05, 2023 at 02:13:28PM +, gkokola...@pm.me wrote: > Good point. I thought about it before submitting the patch. I > concluded that given the complexity and operations involved in > LZ4Stream_read_internal() and the rest of t he pg_dump/pg_restore > code, the memset() call will be

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Friday, May 5th, 2023 at 3:23 PM, Andrew Dunstan wrote: > On 2023-05-05 Fr 06:02, gkokola...@pm.me wrote: > >> --- Original Message --- >> On Friday, May 5th, 2023 at 8:00 AM, Alexander Lakhin >> [](mailto:exclus...@gmail.com) >> wrote: >> >>>

Re: Add LZ4 compression in pg_dump

On 2023-05-05 Fr 06:02, gkokola...@pm.me wrote: --- Original Message --- On Friday, May 5th, 2023 at 8:00 AM, Alexander Lakhin wrote: 23.03.2023 20:10, Tomas Vondra wrote: So pushed all three parts, after updating the commit messages a bit. This leaves the empty-data issue

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Friday, May 5th, 2023 at 8:00 AM, Alexander Lakhin wrote: > > > 23.03.2023 20:10, Tomas Vondra wrote: > > > So pushed all three parts, after updating the commit messages a bit. > > > > This leaves the empty-data issue (which we have a fix for) and

Re: Add LZ4 compression in pg_dump

23.03.2023 20:10, Tomas Vondra wrote: So pushed all three parts, after updating the commit messages a bit. This leaves the empty-data issue (which we have a fix for) and the switch to LZ4F. And then the zstd part. I'm sorry that I haven't noticed/checked that before, but when trying to

Re: Add LZ4 compression in pg_dump

On Wed, Apr 26, 2023 at 08:50:46AM +, gkokola...@pm.me wrote: > For what is worth, I think this would be the best approach. +1 Thanks. I have gone with that, then! -- Michael signature.asc Description: PGP signature

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Tuesday, April 25th, 2023 at 8:02 AM, Michael Paquier wrote: > > > On Wed, Apr 12, 2023 at 07:53:53PM -0500, Justin Pryzby wrote: > > > I doubt it - in the !HAVE_LIBZ case, it's currently an "if" statement > > with nothing but a comment, which isn't

Re: Add LZ4 compression in pg_dump

On Wed, Apr 12, 2023 at 07:53:53PM -0500, Justin Pryzby wrote: > I doubt it - in the !HAVE_LIBZ case, it's currently an "if" statement > with nothing but a comment, which isn't a problem. > > I think the only issue with an empty "if" is when you have no braces, > like: > > if (...) > #if

Re: Add LZ4 compression in pg_dump

On Thu, Apr 13, 2023 at 09:37:06AM +0900, Michael Paquier wrote: > > If you don't insist on calling parse(NONE), the only change is to remove > > the empty #else, which was my original patch. > > Removing the empty else has as problem to create an empty if block, > which could be itself a cause

Re: Add LZ4 compression in pg_dump

On Wed, Apr 12, 2023 at 05:52:40PM -0500, Justin Pryzby wrote: > I don't think you need to call parse_compress_specification(NONE). > As you wrote it, if zlib is unavailable, there's no parse(NONE) call, > even for directory and custom formats. And there's no parse(NONE) call > for plan format

Re: Add LZ4 compression in pg_dump

On Thu, Apr 13, 2023 at 07:23:48AM +0900, Michael Paquier wrote: > On Tue, Apr 11, 2023 at 08:19:59PM -0500, Justin Pryzby wrote: > > On Wed, Apr 12, 2023 at 10:07:08AM +0900, Michael Paquier wrote: > >> Yes, this comment gives no value as it stands. I would be tempted to > >> follow the

Re: Add LZ4 compression in pg_dump

On Tue, Apr 11, 2023 at 08:19:59PM -0500, Justin Pryzby wrote: > On Wed, Apr 12, 2023 at 10:07:08AM +0900, Michael Paquier wrote: >> Yes, this comment gives no value as it stands. I would be tempted to >> follow the suggestion to group the whole code block in a single ifdef, >> including the

Re: Add LZ4 compression in pg_dump

On Wed, Apr 12, 2023 at 10:07:08AM +0900, Michael Paquier wrote: > On Tue, Apr 11, 2023 at 07:41:11PM -0500, Justin Pryzby wrote: > > Maybe I would write it as: "if zlib is unavailable, default to no > > compression". But I think that's best done in the leading comment, and > > not inside an

Re: Add LZ4 compression in pg_dump

On Tue, Apr 11, 2023 at 07:41:11PM -0500, Justin Pryzby wrote: > Maybe I would write it as: "if zlib is unavailable, default to no > compression". But I think that's best done in the leading comment, and > not inside an empty preprocessor #else. > > I was hoping Michael would comment on this.

Re: Add LZ4 compression in pg_dump

On Mon, Feb 27, 2023 at 02:33:04PM +, gkokola...@pm.me wrote: > > > - Finally, the "Nothing to do in the default case" comment comes from > > > Michael's commit 5e73a6048: > > > > > > + /* > > > + * Custom and directory formats are compressed by default with gzip when > > > + * available, not

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

> On 6 Apr 2023, at 18:39, Justin Pryzby wrote: > *If* you wanted to do something to fix this, you could create a key > called files_to_remove_after_loading, and run unlink on those files > rather than running a shell command. Or maybe just remove the file > unconditionally at the start of the

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

On Tue, Mar 14, 2023 at 12:16:16AM +0100, Tomas Vondra wrote: > On 3/9/23 19:00, Tomas Vondra wrote: > > On 3/9/23 01:30, Michael Paquier wrote: > >> On Thu, Mar 09, 2023 at 12:39:08AM +0100, Tomas Vondra wrote: > >>> IMO we should fix that. We have a bunch of buildfarm members running on > >>>

Re: Add LZ4 compression in pg_dump

On 3/31/23 11:19, gkokola...@pm.me wrote: > >> ... >> >> >> I think the patch is fine, but I'm wondering if the renames shouldn't go >> a bit further. It removes references to LZ4File struct, but there's a >> bunch of functions with LZ4File_ prefix. Why not to simply use LZ4_ >> prefix? We don't

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Wednesday, March 29th, 2023 at 12:02 AM, Tomas Vondra wrote: > > > On 3/28/23 18:07, gkokola...@pm.me wrote: > > > --- Original Message --- > > On Friday, March 24th, 2023 at 10:30 AM, gkokola...@pm.me gkokola...@pm.me > > wrote: > > > > >

Re: Add LZ4 compression in pg_dump

On 3/28/23 00:34, gkokola...@pm.me wrote: > > ... > >> Got it. In that case I agree it's fine to do that in a single commit. > > For what is worth, I think that this patch should get a +1 and get in. It > solves the empty writes problem and includes a test to a previous untested > case. >

Re: Add LZ4 compression in pg_dump

On 3/28/23 18:07, gkokola...@pm.me wrote: > > --- Original Message --- > On Friday, March 24th, 2023 at 10:30 AM, gkokola...@pm.me > wrote: > >> >> --- Original Message --- >> On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra >> tomas.von...@enterprisedb.com wrote: >> >>>

Re: Add LZ4 compression in pg_dump

On Tue, Mar 28, 2023 at 06:40:03PM +0200, Tomas Vondra wrote: > On 3/28/23 18:07, gkokola...@pm.me wrote: > > --- Original Message --- > > On Friday, March 24th, 2023 at 10:30 AM, gkokola...@pm.me > > wrote: > > > >> --- Original Message --- > >> On Thursday, March 23rd, 2023 at

Re: Add LZ4 compression in pg_dump

On 3/28/23 18:07, gkokola...@pm.me wrote: > > > > > > --- Original Message --- > On Friday, March 24th, 2023 at 10:30 AM, gkokola...@pm.me > wrote: > >> >> --- Original Message --- >> On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra >>

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Friday, March 24th, 2023 at 10:30 AM, gkokola...@pm.me wrote: > > --- Original Message --- > On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra > tomas.von...@enterprisedb.com wrote: > > > This leaves the empty-data issue (which we have a

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Thursday, March 16th, 2023 at 11:30 PM, Tomas Vondra wrote: > > > > > On 3/16/23 01:20, Justin Pryzby wrote: > > > On Mon, Mar 13, 2023 at 10:47:12PM +0100, Tomas Vondra wrote: > > > > > > > > Thanks. I don't want to annoy you too much, but

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Thursday, March 23rd, 2023 at 6:10 PM, Tomas Vondra wrote: > > So pushed all three parts, after updating the commit messages a bit. Thank you very much. > > This leaves the empty-data issue (which we have a fix for) and the > switch to LZ4F. And

Re: Add LZ4 compression in pg_dump

Hi, I looked at this again, and I realized I misunderstood the bit about errno in LZ4File_open_write a bit. I now see it simply just brings the function in line with Gzip_open_write(), so that the callers can just do pg_fatal("%m"). I still think the special "errno" handling in this one place

Re: Add LZ4 compression in pg_dump

On Fri, Mar 17, 2023 at 03:43:58PM +, gkokola...@pm.me wrote: > From a174cdff4ec8aad59f5bcc7e8d52218a14fe56fc Mon Sep 17 00:00:00 2001 > From: Georgios Kokolatos > Date: Fri, 17 Mar 2023 14:45:58 + > Subject: [PATCH v3 1/3] Improve type handling in pg_dump's compress file API > -int >

Re: Add LZ4 compression in pg_dump

Hi, I was preparing to get the 3 cleanup patches pushed, so I updated/reworded the commit messages a bit (attached, please check). But I noticed the commit message for 0001 said: In passing save the appropriate errno in LZ4File_open_write in case that the caller is not using the API's

Re: Add LZ4 compression in pg_dump

On 3/17/23 16:43, gkokola...@pm.me wrote: >> >> ... >> >> I agree it's cleaner the way you did it. >> >> I was thinking that with each compression function handling error >> internally, the callers would not need to do that. But I haven't >> realized there's logic to detect ENOSPC and so on, and

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Thursday, March 16th, 2023 at 10:20 PM, Tomas Vondra wrote: > > > > > On 3/16/23 18:04, gkokola...@pm.me wrote: > > > --- Original Message --- > > On Tuesday, March 14th, 2023 at 4:32 PM, Tomas Vondra > > tomas.von...@enterprisedb.com

Re: Add LZ4 compression in pg_dump

On 3/16/23 23:58, Justin Pryzby wrote: > On Thu, Mar 16, 2023 at 11:30:50PM +0100, Tomas Vondra wrote: >> On 3/16/23 01:20, Justin Pryzby wrote: >>> But try reading the diff while looking for the cause of a bug. It's the >>> difference between reading 50, two-line changes, and reading a hunk

Re: Add LZ4 compression in pg_dump

On Thu, Mar 16, 2023 at 11:30:50PM +0100, Tomas Vondra wrote: > On 3/16/23 01:20, Justin Pryzby wrote: > > But try reading the diff while looking for the cause of a bug. It's the > > difference between reading 50, two-line changes, and reading a hunk that > > replaces 100 lines with a different

Re: Add LZ4 compression in pg_dump

On 3/16/23 01:20, Justin Pryzby wrote: > On Mon, Mar 13, 2023 at 10:47:12PM +0100, Tomas Vondra wrote: >>> Rearrange functions to their original order allowing a cleaner diff to the >>> prior code; >> >> OK. I wasn't very enthusiastic about this initially, but after thinking >> about it a bit I

Re: Add LZ4 compression in pg_dump

On 3/16/23 18:04, gkokola...@pm.me wrote: > > --- Original Message --- > On Tuesday, March 14th, 2023 at 4:32 PM, Tomas Vondra > wrote: >> >> On 3/14/23 16:18, gkokola...@pm.me wrote: >> >>> ...> Would you mind me trying to come with a patch to address your points? >> >> >> That'd be

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Tuesday, March 14th, 2023 at 4:32 PM, Tomas Vondra wrote: > > > > > On 3/14/23 16:18, gkokola...@pm.me wrote: > > > ...> Would you mind me trying to come with a patch to address your points? > > > That'd be great, thanks. Please keep it split into

Re: Add LZ4 compression in pg_dump

On Mon, Mar 13, 2023 at 10:47:12PM +0100, Tomas Vondra wrote: > > Rearrange functions to their original order allowing a cleaner diff to the > > prior code; > > OK. I wasn't very enthusiastic about this initially, but after thinking > about it a bit I think it's meaningful to make diffs clearer.

Re: Add LZ4 compression in pg_dump

On 3/14/23 12:07, gkokola...@pm.me wrote: > > > --- Original Message --- > On Monday, March 13th, 2023 at 10:47 PM, Tomas Vondra > wrote: > > > >> >>> Change pg_fatal() to an assertion+comment; >> >> >> Yeah, that's reasonable. I'd even ditch the assert/comment, TBH. We >> could add

Re: Add LZ4 compression in pg_dump

On 3/14/23 16:18, gkokola...@pm.me wrote: > ...> Would you mind me trying to come with a patch to address your points? > That'd be great, thanks. Please keep it split into smaller patches - two might work, with one patch for "cosmetic" changes and the other tweaking the API error-handling

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Monday, March 13th, 2023 at 9:21 PM, Tomas Vondra wrote: > > > > > On 3/11/23 11:50, gkokola...@pm.me wrote: > > > --- Original Message --- > > On Saturday, March 11th, 2023 at 7:00 AM, Alexander Lakhin > > exclus...@gmail.com wrote: > > > > >

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

On 3/14/23 11:34, Christoph Berg wrote: > Re: Tomas Vondra >> and I don't think there's a good place to inject the 'rm' so I ended up >> adding a 'cleanup_cmd' right after 'compress_cmd'. But it seems a bit >> strange / hacky. Maybe there's a better way? > > Does the file need to be removed at

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Monday, March 13th, 2023 at 10:47 PM, Tomas Vondra wrote: > > > Change pg_fatal() to an assertion+comment; > > > Yeah, that's reasonable. I'd even ditch the assert/comment, TBH. We > could add such protections against "impossible" stuff to a zillion

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

Re: Tomas Vondra > and I don't think there's a good place to inject the 'rm' so I ended up > adding a 'cleanup_cmd' right after 'compress_cmd'. But it seems a bit > strange / hacky. Maybe there's a better way? Does the file need to be removed at all? Could we leave it there and have "make clean"

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

On 3/9/23 19:00, Tomas Vondra wrote: > > > On 3/9/23 01:30, Michael Paquier wrote: >> On Thu, Mar 09, 2023 at 12:39:08AM +0100, Tomas Vondra wrote: >>> IMO we should fix that. We have a bunch of buildfarm members running on >>> Ubuntu 18.04 (or older) - it's true none of them seems to be

Re: Add LZ4 compression in pg_dump

Hi Justin, Thanks for the patch. On 3/8/23 02:45, Justin Pryzby wrote: > On Wed, Mar 01, 2023 at 04:52:49PM +0100, Tomas Vondra wrote: >> Thanks. That seems correct to me, but I find it somewhat confusing, >> because we now have >> >> DeflateCompressorInit vs. InitCompressorGzip >> >>

Re: Add LZ4 compression in pg_dump

On 3/11/23 11:50, gkokola...@pm.me wrote: > --- Original Message --- > On Saturday, March 11th, 2023 at 7:00 AM, Alexander Lakhin > wrote: > >> Hello, >> 23.02.2023 23:24, Tomas Vondra wrote: >> >>> On 2/23/23 16:26, Tomas Vondra wrote: >>> Thanks for v30 with the updated commit

Re: Add LZ4 compression in pg_dump

On 3/12/23 11:07, Peter Eisentraut wrote: > On 11.03.23 07:00, Alexander Lakhin wrote: >> Hello, >> 23.02.2023 23:24, Tomas Vondra wrote: >>> On 2/23/23 16:26, Tomas Vondra wrote: Thanks for v30 with the updated commit messages. I've pushed 0001 after fixing a comment typo and

Re: Add LZ4 compression in pg_dump

On 11.03.23 07:00, Alexander Lakhin wrote: Hello, 23.02.2023 23:24, Tomas Vondra wrote: On 2/23/23 16:26, Tomas Vondra wrote: Thanks for v30 with the updated commit messages. I've pushed 0001 after fixing a comment typo and removing (I think) an unnecessary change in an error message. I'll

Re: Add LZ4 compression in pg_dump

Hi Georgios, 11.03.2023 13:50, gkokola...@pm.me wrote: I can not answer about the buildfarms. Do you think that adding an explicit check for this warning in meson would help? I am a bit uncertain as I think that type-limits are included in extra. @@ -1748,6 +1748,7 @@ common_warning_flags = [

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Saturday, March 11th, 2023 at 7:00 AM, Alexander Lakhin wrote: > Hello, > 23.02.2023 23:24, Tomas Vondra wrote: > > > On 2/23/23 16:26, Tomas Vondra wrote: > > > > > Thanks for v30 with the updated commit messages. I've pushed 0001 after > > > fixing a

Re: Add LZ4 compression in pg_dump

Hello, 23.02.2023 23:24, Tomas Vondra wrote: On 2/23/23 16:26, Tomas Vondra wrote: Thanks for v30 with the updated commit messages. I've pushed 0001 after fixing a comment typo and removing (I think) an unnecessary change in an error message. I'll give the buildfarm a bit of time before

Re: Add LZ4 compression in pg_dump

On Fri, Mar 10, 2023 at 07:05:49AM -0600, Justin Pryzby wrote: > On Thu, Mar 09, 2023 at 06:58:20PM +0100, Tomas Vondra wrote: >> I'm a bit confused about the lz4 vs. lz4f stuff, TBH. If we switch to >> lz4f, doesn't that mean it (e.g. restore) won't work on systems that >> only have older lz4

Re: Add LZ4 compression in pg_dump

On Thu, Mar 09, 2023 at 06:58:20PM +0100, Tomas Vondra wrote: > I'm a bit confused about the lz4 vs. lz4f stuff, TBH. If we switch to > lz4f, doesn't that mean it (e.g. restore) won't work on systems that > only have older lz4 version? What would/should happen if we take backup > compressed with

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

On 3/9/23 01:30, Michael Paquier wrote: > On Thu, Mar 09, 2023 at 12:39:08AM +0100, Tomas Vondra wrote: >> IMO we should fix that. We have a bunch of buildfarm members running on >> Ubuntu 18.04 (or older) - it's true none of them seems to be running TAP >> tests. But considering how trivial

Re: Add LZ4 compression in pg_dump

On 3/9/23 17:15, Justin Pryzby wrote: > On Wed, Mar 01, 2023 at 05:39:54PM +0100, Tomas Vondra wrote: >> On 2/27/23 05:49, Justin Pryzby wrote: >>> On Sat, Feb 25, 2023 at 08:05:53AM -0600, Justin Pryzby wrote: On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: > I have

Re: Add LZ4 compression in pg_dump

On Wed, Mar 01, 2023 at 05:39:54PM +0100, Tomas Vondra wrote: > On 2/27/23 05:49, Justin Pryzby wrote: > > On Sat, Feb 25, 2023 at 08:05:53AM -0600, Justin Pryzby wrote: > >> On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: > >>> I have some fixes (attached) and questions while

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

On Thu, Mar 09, 2023 at 12:39:08AM +0100, Tomas Vondra wrote: > IMO we should fix that. We have a bunch of buildfarm members running on > Ubuntu 18.04 (or older) - it's true none of them seems to be running TAP > tests. But considering how trivial the fix is ... > > Barring objections, I'll push

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

On 3/8/23 20:20, Daniel Gustafsson wrote: >> On 8 Mar 2023, at 18:55, Christoph Berg wrote: > >> 18.04 will be EOL in a few weeks so it might be ok to just say it's >> not supported, but removing the input file manually after calling lz4 >> would be an easy fix. > > Is it reasonable to expect

Re: lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

> On 8 Mar 2023, at 18:55, Christoph Berg wrote: > 18.04 will be EOL in a few weeks so it might be ok to just say it's > not supported, but removing the input file manually after calling lz4 > would be an easy fix. Is it reasonable to expect that this version of LZ4 can/will appear on any other

lz4 --rm on Ubuntu 18.04 (Add LZ4 compression to pg_dump)

Re: Tomas Vondra > Add LZ4 compression to pg_dump This broke the TAP tests on Ubuntu 18.04 (bionic): [17:06:45.513](0.000s) ok 1927 - compression_lz4_custom: should not dump test_table with 4-row INSERTs # Running: pg_dump --jobs=2 --format=directory --compress=lz4:1 --file=/home/myon/proje

Re: Add LZ4 compression in pg_dump

On Wed, Mar 01, 2023 at 04:52:49PM +0100, Tomas Vondra wrote: > Thanks. That seems correct to me, but I find it somewhat confusing, > because we now have > > DeflateCompressorInit vs. InitCompressorGzip > > DeflateCompressorEnd vs. EndCompressorGzip > > DeflateCompressorData - The name

Re: Add LZ4 compression in pg_dump

On 3/2/23 18:18, Justin Pryzby wrote: > On Wed, Mar 01, 2023 at 05:20:05PM +0100, Tomas Vondra wrote: >> On 2/25/23 15:05, Justin Pryzby wrote: >>> On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: I have some fixes (attached) and questions while polishing the patch for

Re: Add LZ4 compression in pg_dump

On Wed, Mar 01, 2023 at 05:20:05PM +0100, Tomas Vondra wrote: > On 2/25/23 15:05, Justin Pryzby wrote: > > On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: > >> I have some fixes (attached) and questions while polishing the patch for > >> zstd compression. The fixes are small and

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Wednesday, March 1st, 2023 at 5:20 PM, Tomas Vondra wrote: > > > > > On 2/25/23 15:05, Justin Pryzby wrote: > > > On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: > > > > > I have some fixes (attached) and questions while polishing

Re: Add LZ4 compression in pg_dump

On Wed, Mar 01, 2023 at 01:39:14PM +, gkokola...@pm.me wrote: > On Wednesday, March 1st, 2023 at 12:58 AM, Justin Pryzby > wrote: > > > The current function order avoids 3 lines of declarations, but it's > > obviously pretty useful to be able to run that diff command. I already > > argued

Re: Add LZ4 compression in pg_dump

On 2/27/23 05:49, Justin Pryzby wrote: > On Sat, Feb 25, 2023 at 08:05:53AM -0600, Justin Pryzby wrote: >> On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: >>> I have some fixes (attached) and questions while polishing the patch for >>> zstd compression. The fixes are small and

Re: Add LZ4 compression in pg_dump

On 2/25/23 15:05, Justin Pryzby wrote: > On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: >> I have some fixes (attached) and questions while polishing the patch for >> zstd compression. The fixes are small and could be integrated with the >> patch for zstd, but could be applied

Re: Add LZ4 compression in pg_dump

On 2/27/23 15:56, gkokola...@pm.me wrote: > > > > > > --- Original Message --- > On Saturday, February 25th, 2023 at 3:05 PM, Justin Pryzby > wrote: > > >> >> >> On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: >> >>> I have some fixes (attached) and questions

Re: Add LZ4 compression in pg_dump

On 3/1/23 14:39, gkokola...@pm.me wrote: > > > > > > --- Original Message --- > On Wednesday, March 1st, 2023 at 12:58 AM, Justin Pryzby > wrote: > > > >> I found that e9960732a broke writing of empty gzip-compressed data, >> specifically LOs. pg_dump succeeds, but then the

Re: Add LZ4 compression in pg_dump

On 3/1/23 08:24, Michael Paquier wrote: > On Tue, Feb 28, 2023 at 05:58:34PM -0600, Justin Pryzby wrote: >> I found that e9960732a broke writing of empty gzip-compressed data, >> specifically LOs. pg_dump succeeds, but then the restore fails: > > The number of issues you have been reporting here

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Wednesday, March 1st, 2023 at 12:58 AM, Justin Pryzby wrote: > I found that e9960732a broke writing of empty gzip-compressed data, > specifically LOs. pg_dump succeeds, but then the restore fails: > > postgres=# SELECT lo_create(1234); > lo_create |

Re: Add LZ4 compression in pg_dump

On Tue, Feb 28, 2023 at 05:58:34PM -0600, Justin Pryzby wrote: > I found that e9960732a broke writing of empty gzip-compressed data, > specifically LOs. pg_dump succeeds, but then the restore fails: The number of issues you have been reporting here begins to worries me.. How many of them have

Re: Add LZ4 compression in pg_dump

On Thu, Feb 23, 2023 at 09:24:46PM +0100, Tomas Vondra wrote: > On 2/23/23 16:26, Tomas Vondra wrote: > > Thanks for v30 with the updated commit messages. I've pushed 0001 after > > fixing a comment typo and removing (I think) an unnecessary change in an > > error message. > > > > I'll give the

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Saturday, February 25th, 2023 at 3:05 PM, Justin Pryzby wrote: > > > On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: > > > I have some fixes (attached) and questions while polishing the patch for > > zstd compression. The fixes are

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Sunday, February 26th, 2023 at 3:59 PM, Tomas Vondra wrote: > > > > > On 2/25/23 06:02, Justin Pryzby wrote: > > > I have some fixes (attached) and questions while polishing the patch for > > zstd compression. The fixes are small and could be

Re: Add LZ4 compression in pg_dump

On Sat, Feb 25, 2023 at 08:05:53AM -0600, Justin Pryzby wrote: > On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: > > I have some fixes (attached) and questions while polishing the patch for > > zstd compression. The fixes are small and could be integrated with the > > patch for

Re: Add LZ4 compression in pg_dump

On 2/25/23 06:02, Justin Pryzby wrote: > I have some fixes (attached) and questions while polishing the patch for > zstd compression. The fixes are small and could be integrated with the > patch for zstd, but could be applied independently. > > - I'm unclear about get_error_func(). That's

Re: Add LZ4 compression in pg_dump

On Fri, Feb 24, 2023 at 11:02:14PM -0600, Justin Pryzby wrote: > I have some fixes (attached) and questions while polishing the patch for > zstd compression. The fixes are small and could be integrated with the > patch for zstd, but could be applied independently. One more -

Re: Add LZ4 compression in pg_dump

I have some fixes (attached) and questions while polishing the patch for zstd compression. The fixes are small and could be integrated with the patch for zstd, but could be applied independently. - I'm unclear about get_error_func(). That's called in three places from pg_backup_directory.c,

Re: Add LZ4 compression in pg_dump

--- Original Message --- On Friday, February 24th, 2023 at 5:35 AM, Michael Paquier wrote: > > > On Thu, Feb 23, 2023 at 07:51:16PM -0600, Justin Pryzby wrote: > > > On Thu, Feb 23, 2023 at 09:24:46PM +0100, Tomas Vondra wrote: > > > > > I've now pushed 0002 and 0003, after

Re: Add LZ4 compression in pg_dump

On Thu, Feb 23, 2023 at 07:51:16PM -0600, Justin Pryzby wrote: > On Thu, Feb 23, 2023 at 09:24:46PM +0100, Tomas Vondra wrote: >> I've now pushed 0002 and 0003, after minor tweaks (a couple typos etc.), >> and marked the CF entry as committed. Thanks for the patch! > > A big thanks from me to

Re: Add LZ4 compression in pg_dump

On Thu, Feb 23, 2023 at 09:24:46PM +0100, Tomas Vondra wrote: > I've now pushed 0002 and 0003, after minor tweaks (a couple typos etc.), > and marked the CF entry as committed. Thanks for the patch! A big thanks from me to everyone involved. > I wonder how difficult would it be to add the zstd

  1   2   >