Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-22 Thread Tom Lane
Robert Haas writes: > On Mon, Mar 22, 2021 at 11:48 AM Tom Lane wrote: >> Possibly the former names should survive and the latter become >> wrappers around them, not sure. But we shouldn't be using the "4B" >> terminology anyplace except this part of postgres.h. > I would argue that it

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 11:48 AM Tom Lane wrote: > > Anyway, this particular macro name was chosen, it seems, for symmetry > > with VARDATA_4B_C, but if you want to change it to something else, I'm > > OK with that, too. > > After looking at postgres.h for a bit, I'm thinking that what these >

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-22 Thread Tom Lane
Robert Haas writes: > On Mon, Mar 22, 2021 at 10:41 AM Tom Lane wrote: >> Yeah, I thought about that too, but do we want to assume that >> VARRAWSIZE_4B_C is the correct way to get the decompressed size >> for all compression methods? > I think it's OK to assume this. OK, cool. >> (If so, I

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 10:41 AM Tom Lane wrote: > > Okay, the fix makes sense. In fact, IMHO, in general also this fix > > looks like an optimization, I mean when slicelength >= > > VARRAWSIZE_4B_C(value), then why do we need to allocate extra memory > > even in the case of pglz. So shall we

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-22 Thread Dilip Kumar
On Mon, Mar 22, 2021 at 8:11 PM Tom Lane wrote: > > Dilip Kumar writes: > > On Mon, Mar 22, 2021 at 5:22 AM Tom Lane wrote: > >> Also, after studying the documentation for LZ4_decompress_safe > >> and LZ4_decompress_safe_partial, I realized that liblz4 is also > >> counting on the *output*

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-22 Thread Tom Lane
Dilip Kumar writes: > On Mon, Mar 22, 2021 at 5:22 AM Tom Lane wrote: >> Also, after studying the documentation for LZ4_decompress_safe >> and LZ4_decompress_safe_partial, I realized that liblz4 is also >> counting on the *output* buffer size to not be a lie. So we >> cannot pass it a number

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-22 Thread Dilip Kumar
On Mon, Mar 22, 2021 at 5:22 AM Tom Lane wrote: > Actually, after reading that closer, the problem only affects the > case where the compressed-data-length passed to the function is > a lie. So it shouldn't be a problem for our usage. > > Also, after studying the documentation for

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-21 Thread Tom Lane
Justin Pryzby writes: > On Sun, Mar 21, 2021 at 07:11:50PM -0400, Tom Lane wrote: >> I hate to be the bearer of bad news, but this suggests that >> LZ4_decompress_safe_partial is seriously broken in 1.9.2 >> as well: >> https://github.com/lz4/lz4/issues/783 > Ouch Actually, after reading that

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-21 Thread Justin Pryzby
On Sun, Mar 21, 2021 at 07:11:50PM -0400, Tom Lane wrote: > I wrote: > > Justin Pryzby writes: > >> On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote: > >>> This seems somewhat repeatable (three identical failures in three > >>> attempts). Not sure why I did not see it yesterday; but

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-21 Thread Tom Lane
I wrote: > Justin Pryzby writes: >> On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote: >>> This seems somewhat repeatable (three identical failures in three >>> attempts). Not sure why I did not see it yesterday; but anyway, >>> there is something wrong with partial detoasting for LZ4.

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-21 Thread Tom Lane
Justin Pryzby writes: > On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote: >> This seems somewhat repeatable (three identical failures in three >> attempts). Not sure why I did not see it yesterday; but anyway, >> there is something wrong with partial detoasting for LZ4. > With what

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-21 Thread Justin Pryzby
On Sun, Mar 21, 2021 at 04:32:31PM -0400, Tom Lane wrote: > This seems somewhat repeatable (three identical failures in three > attempts). Not sure why I did not see it yesterday; but anyway, > there is something wrong with partial detoasting for LZ4. With what version of LZ4 ? -- Justin

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-21 Thread Tom Lane
Dilip Kumar writes: >> Yeah, we need to set the default_toast_compression in the beginning of >> the test as attached. > In the last patch, I did not adjust the compression_1.out so fixed > that in the attached patch. Pushed that; however, while testing that it works as expected, I saw a new and

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-21 Thread Dilip Kumar
On Sun, Mar 21, 2021 at 7:03 AM Tom Lane wrote: > Also, I see some diffs in the > indirect_toast test, which seems perhaps worthy of investigation. > (The diffs look to be just row ordering, but why?) I have investigated that, actually in the below insert, after compression the data size of

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-20 Thread Dilip Kumar
On Sun, Mar 21, 2021 at 9:10 AM Dilip Kumar wrote: > > On Sun, Mar 21, 2021 at 7:03 AM Tom Lane wrote: > > > > BTW, I tried doing "make installcheck" after having adjusted > > default_toast_compression to be "lz4". The compression test > > itself fails because it's expecting the other setting;

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-20 Thread Dilip Kumar
On Sun, Mar 21, 2021 at 7:03 AM Tom Lane wrote: > > BTW, I tried doing "make installcheck" after having adjusted > default_toast_compression to be "lz4". The compression test > itself fails because it's expecting the other setting; that > ought to be made more robust. Yeah, we need to set the

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-20 Thread Tom Lane
BTW, I tried doing "make installcheck" after having adjusted default_toast_compression to be "lz4". The compression test itself fails because it's expecting the other setting; that ought to be made more robust. Also, I see some diffs in the indirect_toast test, which seems perhaps worthy of

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-20 Thread Tom Lane
Justin Pryzby writes: > On Sat, Mar 20, 2021 at 05:37:07PM -0400, Tom Lane wrote: >> Digging around, it looks like the "-I/opt/local/include" bit came >> from LZ4_CFLAGS, which we then stuck into CFLAGS, but it needed >> to be put in CPPFLAGS in order to make this test work. > If it's the same

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-20 Thread Justin Pryzby
On Sat, Mar 20, 2021 at 05:37:07PM -0400, Tom Lane wrote: > Justin Pryzby writes: > > On Fri, Mar 19, 2021 at 02:07:31PM -0400, Robert Haas wrote: > >> On Fri, Mar 19, 2021 at 1:44 PM Justin Pryzby wrote: > >>> configure: WARNING: lz4.h: accepted by the compiler, rejected by the > >>>

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-20 Thread Tom Lane
Justin Pryzby writes: > On Fri, Mar 19, 2021 at 02:07:31PM -0400, Robert Haas wrote: >> On Fri, Mar 19, 2021 at 1:44 PM Justin Pryzby wrote: >>> configure: WARNING: lz4.h: accepted by the compiler, rejected by the >>> preprocessor! >>> configure: WARNING: lz4.h: proceeding with the compiler's

Re: [HACKERS] Custom compression methods (mac+lz4.h)

2021-03-20 Thread Justin Pryzby
On Fri, Mar 19, 2021 at 02:07:31PM -0400, Robert Haas wrote: > On Fri, Mar 19, 2021 at 1:44 PM Justin Pryzby wrote: > > Working with one of Andrey's patches on another thread, he reported offlist > > getting this message, resolved by this patch. Do you see this warning > > during > >