Re: Add LZ4 compression in pg_dump

------- Original Message -------
On Tuesday, December 20th, 2022 at 4:26 PM, Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:

>
>
> On Tue, Dec 20, 2022 at 11:19:15AM +0000, gkokolatos(at)pm(dot)me wrote:
>
> > ------- Original Message -------
> > On Monday, December 19th, 2022 at 6:27 PM, Justin Pryzby pryzby(at)telsasoft(dot)com wrote:
> >
> > > On Mon, Dec 19, 2022 at 05:03:21PM +0000, gkokolatos(at)pm(dot)me wrote:
> > >
> > > > > > 001 still doesn't compile on freebsd, and 002 doesn't compile on
> > > > > > windows. Have you checked test results from cirrusci on your private
> > > > > > github account ?
> > > >
> > > > There are still known gaps in 0002 and 0003, for example documentation,
> > > > and I have not been focusing too much on those. You are right, it is helpful
> > > > and kind to try to reduce the noise. The attached should have hopefully
> > > > tackled the ci errors.
> > >
> > > Yep. Are you using cirrusci under your github account ?
> >
> > Thank you. To be very honest, I am not using github exclusively to post patches.
> > Sometimes I do, sometimes I do not. Is github a requirement?
>
>
> Github isn't a requirement for postgres (but cirrusci only supports
> github). I wasn't not trying to say that it's required, only trying to
> make sure that you (and others) know that it's available, since our
> cirrus.yml is relatively new.

Got it. Thank you very much for spreading the word. It is a useful feature which
should be known.

>
> > > > > > 002 breaks "pg_dump -Fc -Z2" because (I think) AllocateCompressor()
> > > > > > doesn't store the passed-in compression_spec.
> > > >
> > > > I am afraid I have not been able to reproduce this error. I tried both
> > > > debian and freebsd after I addressed the compilation warnings. Which
> > > > error did you get? Is it still present in the attached?
> > >
> > > It's not that there's an error - it's that compression isn't working.
> > >
> > > $ ./tmp_install/usr/local/pgsql/bin/pg_dump -h /tmp -Z1 -Fp regression |wc -c
> > > 659956
> > > $ ./tmp_install/usr/local/pgsql/bin/pg_dump -h /tmp -Z2 -Fp regression |wc -c
> > > 637192
> > >
> > > $ ./tmp_install/usr/local/pgsql/bin/pg_dump -h /tmp -Z1 -Fc regression |wc -c
> > > 1954890
> > > $ ./tmp_install/usr/local/pgsql/bin/pg_dump -h /tmp -Z2 -Fc regression |wc -c
> > > 1954890
> >
> > Thank you. Now I understand what you mean. Trying the same on top of v18-0003
> > on Ubuntu 22.04 yields:
>
>
> You're right; this seems to be fixed in v18. Thanks.

Great. Still there was a bug in v17 which you discovered. Thank you for the review
effort.

Please find in the attached v19 an extra check right before calling deflateInit().
This check will verify that only compressed output will be generated for this
method.

Also v19 is rebased on top f450695e889 and applies cleanly.

Cheers.
//Georgios

> --
> Justin