Re: Small issues with CREATE TABLE COMPRESSION

On Mon, May 10, 2021 at 11:27 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Sat, May 08, 2021 at 08:19:03PM +0530, Dilip Kumar wrote:
> > I have fixed some of them, I could not write the test cases where we
> > have to ensure that 'ALTER TABLE COMPRESSION' is not appearing in the
> > dump.  Basically, I don't have knowledge of the perl language so even
> > after trying for some time I could not write those 2 test cases.  I
> > have fixed the remaining comments.
>
> Thanks.  I have spent some time on that, and after adding some tests
> with --no-toast-compression, I have applied this part.

Thanks!

> Now comes the last part of the thread: support for the build with
> MSVC.  I have looked in details at the binaries provided by upstream
> on its release page, but these are for msys and mingw, so MSVC won't
> work with that.
>
> Saying that, the upstream code can be compiled across various MSVC
> versions, with 2010 being the oldest version supported, even if there
> is no compiled libraries provided on the release pages.  The way of
> doing things here is to compile the code by yourself after downloading
> the source tarball, with liblz4.lib and liblz4.dll being the generated
> bits interesting for Postgres, so using
> https://github.com/lz4/lz4/releases as reference for the download
> looks enough, still that requires some efforts from the users to be
> able to do that.  Another trick is to use vcpkg, but the deliverables
> generated are named lz4.{dll,lib} which is inconsistent with the
> upstream naming liblz4.{dll,lib} (see Makefile.inc for the details).
> My image of the whole thing is that this finishes by being a pain,
> still that's possible, but that's similar with my experience with any
> other dependencies.

Even I was confused about that's the reason I used liblz4_static.lib,
but I see you have changed to liblz4.lib to make it consistent I
guess?

> I have been spending some time playing with the builds and that was
> working nicely.  Please note that you have missed an update in
> config_default.pl and not all the CFLAGS entries were present in
> GenerateFiles().

Yeah, I have noticed, and thanks for changing that.

> It may be nice to see if this stuff requires any adjustments for msys
> and mingw, but I don't have such environments at hand.
>
> All that leads me to the updated version attached.
>
> Thoughts?

Patch looks good to me, I can not verify though because I don't have
such an environment.  Thanks for improving the patch.


--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com