Re: [HACKERS] Custom compression methods

2021-04-08 Thread Robert Haas
On Thu, Apr 8, 2021 at 3:38 PM Justin Pryzby wrote: > It looks like this should not remove the word "data" ? Oh, yes, right. > The compression technique used for either in-line or out-of-line compressed > -data is a fairly simple and very fast member > -of the LZ family of compression

Re: [HACKERS] Custom compression methods

2021-04-08 Thread Justin Pryzby
On Thu, Apr 08, 2021 at 02:58:04PM -0400, Robert Haas wrote: > On Thu, Apr 8, 2021 at 11:32 AM David Steele wrote: > > It looks like this CF entry should have been marked as committed so I > > did that. > > Thanks. > > Here's a patch for the doc update which was mentioned as an open item >

Re: [HACKERS] Custom compression methods

2021-04-08 Thread Robert Haas
On Thu, Apr 8, 2021 at 11:32 AM David Steele wrote: > It looks like this CF entry should have been marked as committed so I > did that. Thanks. Here's a patch for the doc update which was mentioned as an open item upthread. -- Robert Haas EDB: http://www.enterprisedb.com

Re: [HACKERS] Custom compression methods

2021-04-08 Thread David Steele
On 3/30/21 10:30 AM, Tom Lane wrote: Robert Haas writes: On Wed, Mar 24, 2021 at 2:15 PM Tom Lane wrote: But let's ignore the case of pg_upgrade and just consider a dump/restore. I'd still say that unless you give --no-toast-compression then I would expect the dump/restore to preserve the

Re: [HACKERS] Custom compression methods

2021-03-30 Thread Tom Lane
Robert Haas writes: > On Wed, Mar 24, 2021 at 2:15 PM Tom Lane wrote: >> But let's ignore the case of pg_upgrade and just consider a dump/restore. >> I'd still say that unless you give --no-toast-compression then I would >> expect the dump/restore to preserve the tables' old compression

Re: [HACKERS] Custom compression methods (buildfarm xupgrade)

2021-03-28 Thread Justin Pryzby
On Sun, Mar 28, 2021 at 04:48:29PM -0400, Andrew Dunstan wrote: > Nothing is hidden here - the diffs are reported, see for example > > What we're comparing here is

Re: [HACKERS] Custom compression methods

2021-03-28 Thread Andrew Dunstan
On 3/24/21 12:45 PM, Tom Lane wrote: > Robert Haas writes: >> On Wed, Mar 24, 2021 at 11:42 AM Tom Lane wrote: >>> On reflection, though, I wonder if we've made pg_dump do the right >>> thing anyway. There is a strong case to be made for the idea that >>> when dumping from a pre-14 server, it

Re: [HACKERS] Custom compression methods

2021-03-25 Thread Justin Pryzby
On Mon, Mar 22, 2021 at 10:41:33AM -0400, Robert Haas wrote: > trying to explain how TOAST works here, I added a link. It looks, > though, like that documentation also needs to be patched for this > change. I'll look into that, and your remaining patches, next. I added an Opened Item for any

Re: [HACKERS] Custom compression methods

2021-03-25 Thread Robert Haas
On Thu, Mar 25, 2021 at 5:44 AM Dilip Kumar wrote: > Okay got it. Fixed as suggested. Committed with a bit of editing of the comments. -- Robert Haas EDB: http://www.enterprisedb.com

Re: [HACKERS] Custom compression methods

2021-03-25 Thread Robert Haas
On Wed, Mar 24, 2021 at 2:15 PM Tom Lane wrote: > But let's ignore the case of pg_upgrade and just consider a dump/restore. > I'd still say that unless you give --no-toast-compression then I would > expect the dump/restore to preserve the tables' old compression behavior. > Robert's argument that

Re: [HACKERS] Custom compression methods

2021-03-25 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 10:57 PM Robert Haas wrote: > > On Wed, Mar 24, 2021 at 12:14 PM Dilip Kumar wrote: > > Actually, we are already doing this, I mean ALTER TABLE .. ALTER > > COLUMN .. SET COMPRESSION is already updating the compression method > > of the index attribute. So 0003 doesn't

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Tom Lane
Justin Pryzby writes: > On Wed, Mar 24, 2021 at 01:30:26PM -0400, Robert Haas wrote: >> ... So --no-toast-compression is just fine for people who are >> dumping and restoring, but it's no help at all if you want to switch >> TOAST compression methods while doing a pg_upgrade. However, what does

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Justin Pryzby
On Wed, Mar 24, 2021 at 01:30:26PM -0400, Robert Haas wrote: > On Wed, Mar 24, 2021 at 1:24 PM Justin Pryzby wrote: > > I think it's not specific to pg_upgrade, but any pg_dump |pg_restore. > > > > The analogy with tablespaces is restoring from a cluster where the > > tablespace > > is named

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Robert Haas
On Wed, Mar 24, 2021 at 1:24 PM Justin Pryzby wrote: > I think it's not specific to pg_upgrade, but any pg_dump |pg_restore. > > The analogy with tablespaces is restoring from a cluster where the tablespace > is named "vast" to one where it's named "huge". I do this by running >

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Robert Haas
On Wed, Mar 24, 2021 at 12:14 PM Dilip Kumar wrote: > Actually, we are already doing this, I mean ALTER TABLE .. ALTER > COLUMN .. SET COMPRESSION is already updating the compression method > of the index attribute. So 0003 doesn't make sense, sorry for the > noise. However, 0001 and 0002 are

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Justin Pryzby
On Wed, Mar 24, 2021 at 12:24:38PM -0400, Robert Haas wrote: > On Wed, Mar 24, 2021 at 11:42 AM Tom Lane wrote: > > On reflection, though, I wonder if we've made pg_dump do the right > > thing anyway. There is a strong case to be made for the idea that > > when dumping from a pre-14 server, it

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Robert Haas
On Wed, Mar 24, 2021 at 12:45 PM Tom Lane wrote: > I wouldn't be proposing this if the xversion failures were the only > reason; making them go away is just a nice side-effect. The core > point is that the charter of pg_dump is to reproduce the source > database's state, and as things stand

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Tom Lane
Robert Haas writes: > On Wed, Mar 24, 2021 at 11:42 AM Tom Lane wrote: >> On reflection, though, I wonder if we've made pg_dump do the right >> thing anyway. There is a strong case to be made for the idea that >> when dumping from a pre-14 server, it should emit >> SET default_toast_compression

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Robert Haas
On Mon, Mar 22, 2021 at 4:57 PM Robert Haas wrote: > > Fixed. > > Fixed some more. Committed. -- Robert Haas EDB: http://www.enterprisedb.com

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Robert Haas
On Wed, Mar 24, 2021 at 11:42 AM Tom Lane wrote: > On reflection, though, I wonder if we've made pg_dump do the right > thing anyway. There is a strong case to be made for the idea that > when dumping from a pre-14 server, it should emit > SET default_toast_compression = 'pglz'; > rather

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 9:32 PM Robert Haas wrote: > > On Wed, Mar 24, 2021 at 11:41 AM Dilip Kumar wrote: > > Okay, that sounds like a reasonable design idea. But the problem is > > that in index_form_tuple we only have index tuple descriptor, not the > > heap tuple descriptor. Maybe we will

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Robert Haas
On Wed, Mar 24, 2021 at 11:41 AM Dilip Kumar wrote: > Okay, that sounds like a reasonable design idea. But the problem is > that in index_form_tuple we only have index tuple descriptor, not the > heap tuple descriptor. Maybe we will have to pass the heap tuple > descriptor as a parameter to

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Tom Lane
Andrew Dunstan writes: > On 3/20/21 3:03 PM, Tom Lane wrote: >> I fixed up some issues in 0008/0009 (mostly cosmetic, except that >> you forgot a server version check in dumpToastCompression) and >> pushed that, so we can see if it makes crake happy. > It's still produced a significant amount

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 8:41 PM Robert Haas wrote: > > On Wed, Mar 24, 2021 at 7:45 AM Dilip Kumar wrote: > > I have anyway created a patch for this as well. Including all three > > patches so we don't lose track. > > > > 0001 ->shows compression method for the index attribute in index describe

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Robert Haas
On Wed, Mar 24, 2021 at 7:45 AM Dilip Kumar wrote: > I have anyway created a patch for this as well. Including all three > patches so we don't lose track. > > 0001 ->shows compression method for the index attribute in index describe > 0002 -> fix the reported bug (test case included) >

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 3:40 PM Dilip Kumar wrote: > > 0001 ->shows compression method for the index attribute in index describe > 0002 -> fix the reported bug (test case included) > > Apart from this, I was thinking that currently, we are allowing to > ALTER SET COMPRESSION only for the table

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 3:10 PM Dilip Kumar wrote: > > On Wed, Mar 24, 2021 at 2:49 PM Justin Pryzby wrote: > > > > On Wed, Mar 24, 2021 at 02:24:41PM +0530, Dilip Kumar wrote: > > > On Wed, Mar 24, 2021 at 1:43 PM Dilip Kumar wrote: > > > > > create table t1 (col1 text, col2 text); > > > > >

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 2:49 PM Justin Pryzby wrote: > > On Wed, Mar 24, 2021 at 02:24:41PM +0530, Dilip Kumar wrote: > > On Wed, Mar 24, 2021 at 1:43 PM Dilip Kumar wrote: > > > > create table t1 (col1 text, col2 text); > > > > create unique index on t1 ((col1 || col2)); > > > > insert into t1

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Justin Pryzby
On Wed, Mar 24, 2021 at 02:24:41PM +0530, Dilip Kumar wrote: > On Wed, Mar 24, 2021 at 1:43 PM Dilip Kumar wrote: > > > create table t1 (col1 text, col2 text); > > > create unique index on t1 ((col1 || col2)); > > > insert into t1 values((select array_agg(md5(g::text))::text from > > >

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 1:43 PM Dilip Kumar wrote: > > > > """ > > create table t1 (col1 text, col2 text); > > create unique index on t1 ((col1 || col2)); > > insert into t1 values((select array_agg(md5(g::text))::text from > > generate_series(1, 256) g), version()); > > """ > > > > Attached is

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Dilip Kumar
On Wed, Mar 24, 2021 at 1:22 PM Jaime Casanova wrote: > > On Fri, Mar 19, 2021 at 2:44 PM Robert Haas wrote: > > > > I committed the core patch (0003) with a bit more editing. Let's see > > what the buildfarm thinks. > > > > I think this is bbe0a81db69bd10bd166907c3701492a29aca294, right? >

Re: [HACKERS] Custom compression methods

2021-03-24 Thread Jaime Casanova
On Fri, Mar 19, 2021 at 2:44 PM Robert Haas wrote: > > I committed the core patch (0003) with a bit more editing. Let's see > what the buildfarm thinks. > I think this is bbe0a81db69bd10bd166907c3701492a29aca294, right? This introduced a new assert failure, steps to reproduce: """ create table

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Tom Lane
Justin Pryzby writes: > On Mon, Mar 22, 2021 at 03:47:58PM -0400, Robert Haas wrote: >> Are you sure you're looking at the patch I sent, >> toast-compression-guc-rmh.patch? I can't help wondering if you applied >> it to a dirty source tree or got the wrong file or something, because >> otherwise

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Justin Pryzby
On Mon, Mar 22, 2021 at 03:47:58PM -0400, Robert Haas wrote: > On Mon, Mar 22, 2021 at 2:10 PM Tom Lane wrote: > > Robert Haas writes: > > > I think this is significantly cleaner than what we have now, and I > > > also prefer it to your proposal. > > > > +1 in general. However, I suspect that

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 4:33 PM Robert Haas wrote: > On Mon, Mar 22, 2021 at 1:58 PM Justin Pryzby wrote: > > guc.c should not longer define this as extern: > > default_toast_compression_options > > Fixed. Fixed some more. -- Robert Haas EDB: http://www.enterprisedb.com

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 1:58 PM Justin Pryzby wrote: > guc.c should not longer define this as extern: > default_toast_compression_options Fixed. > I think you should comment that default_toast_compression is an int as far as > guc.c is concerned, but storing one of the char value of

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 2:10 PM Tom Lane wrote: > Robert Haas writes: > > I think this is significantly cleaner than what we have now, and I > > also prefer it to your proposal. > > +1 in general. However, I suspect that you did not try to compile > this without --with-lz4, because if you had

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Tom Lane
Robert Haas writes: > I think this is significantly cleaner than what we have now, and I > also prefer it to your proposal. +1 in general. However, I suspect that you did not try to compile this without --with-lz4, because if you had you'd have noticed the other uses of NO_LZ4_SUPPORT() that

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Justin Pryzby
On Mon, Mar 22, 2021 at 01:38:36PM -0400, Robert Haas wrote: > On Mon, Mar 22, 2021 at 12:16 PM Robert Haas wrote: > > But, what about giving the default_toast_compression_method GUC an > > assign hook that sets a global variable of type "char" to the > > appropriate value? Then

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 12:16 PM Robert Haas wrote: > But, what about giving the default_toast_compression_method GUC an > assign hook that sets a global variable of type "char" to the > appropriate value? Then GetDefaultToastCompression() goes away > entirely. That might be worth exploring.

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 11:13 AM Justin Pryzby wrote: > The first iteration was pretty rough, and there's still some question in my > mind about where default_toast_compression_options[] should be defined. If > it's in the header file, then I could use lengthof() - but then it probably > gets

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

2021-03-22 Thread Justin Pryzby
On Mon, Mar 22, 2021 at 11:05:19AM -0400, Robert Haas wrote: > On Mon, Mar 22, 2021 at 10:44 AM Justin Pryzby wrote: > > Thanks. I just realized that if you also push the GUC change, then the docs > > should change from to > > > > doc/src/sgml/config.sgml: > > default_toast_compression

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Robert Haas
On Mon, Mar 22, 2021 at 10:44 AM Justin Pryzby wrote: > Thanks. I just realized that if you also push the GUC change, then the docs > should change from to > > doc/src/sgml/config.sgml: > default_toast_compression (string) I've now also committed your 0005. As for 0006, aside from the

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Justin Pryzby
On Mon, Mar 22, 2021 at 10:41:33AM -0400, Robert Haas wrote: > On Sun, Mar 21, 2021 at 7:55 PM Justin Pryzby wrote: > > Rebased again. > > Thanks, Justin. I committed 0003 and 0004 together as > 226e2be3876d0bda3dc33d16dfa0bed246b7b74f. I also committed 0001 and > 0002 together as

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

2021-03-22 Thread Robert Haas
On Sun, Mar 21, 2021 at 7:55 PM Justin Pryzby wrote: > Rebased again. Thanks, Justin. I committed 0003 and 0004 together as 226e2be3876d0bda3dc33d16dfa0bed246b7b74f. I also committed 0001 and 0002 together as 24f0e395ac5892cd12e8914646fe921fac5ba23d, but with some revisions, because your text

Re: [HACKERS] Custom compression methods

2021-03-22 Thread Dilip Kumar
On Mon, Mar 22, 2021 at 5:25 AM Justin Pryzby wrote: > > On Sat, Mar 20, 2021 at 06:20:39PM -0500, Justin Pryzby wrote: > > Rebased on HEAD. > > 0005 forgot to update compression_1.out. > > Included changes to ./configure.ac and some other patches, but not Tomas's, > > since it'll make CFBOT get

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

2021-03-21 Thread Justin Pryzby
On Sat, Mar 20, 2021 at 06:20:39PM -0500, Justin Pryzby wrote: > Rebased on HEAD. > 0005 forgot to update compression_1.out. > Included changes to ./configure.ac and some other patches, but not Tomas's, > since it'll make CFBOT get mad as soon as that's pushed. Rebased again. Renamed "t" to a

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

2021-03-21 Thread Tom Lane
... btw, now that I look at this, why are we expending a configure probe for ? If we need to cater for that spelling of the header name, the C code proper is not ready for it. regards, tom lane

Re: [HACKERS] Custom compression methods

2021-03-21 Thread Tom Lane
Justin Pryzby writes: > Rebased on HEAD. > 0005 forgot to update compression_1.out. > Included changes to ./configure.ac and some other patches, but not Tomas's, > since it'll make CFBOT get mad as soon as that's pushed. I pushed a version of the configure fixes that passes my own sanity checks,

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

2021-03-20 Thread Tomas Vondra
On 3/20/21 4:21 PM, Justin Pryzby wrote: > On Sat, Mar 20, 2021 at 04:13:47PM +0100, Tomas Vondra wrote: >> +++ b/src/backend/access/brin/brin_tuple.c >> @@ -213,10 +213,20 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, >> BrinMemTuple *tuple, >>

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Justin Pryzby
Rebased on HEAD. 0005 forgot to update compression_1.out. Included changes to ./configure.ac and some other patches, but not Tomas's, since it'll make CFBOT get mad as soon as that's pushed. -- Justin >From bf1336d284792b29e30b42af2ec820bb0c256916 Mon Sep 17 00:00:00 2001 From: Justin Pryzby

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 > >

Re: [HACKERS] Custom compression methods (./configure)

2021-03-20 Thread Tom Lane
Justin Pryzby writes: > On Fri, Mar 19, 2021 at 05:35:58PM -0300, Alvaro Herrera wrote: >> I find this behavior confusing; I'd rather have configure error out if >> it can't find the package support I requested, than continuing with a >> set of configure options different from what I gave. >

Re: [HACKERS] Custom compression methods (./configure)

2021-03-20 Thread Alvaro Herrera
On 2021-Mar-20, Justin Pryzby wrote: > On Fri, Mar 19, 2021 at 05:35:58PM -0300, Alvaro Herrera wrote: > > Hmm, if I use configure --with-lz4, I get this: > > > > checking whether to build with LZ4 support... yes > > checking for liblz4... no > > configure: error: Package

Re: [HACKERS] Custom compression methods (./configure)

2021-03-20 Thread Justin Pryzby
On Fri, Mar 19, 2021 at 05:35:58PM -0300, Alvaro Herrera wrote: > Hmm, if I use configure --with-lz4, I get this: > > checking whether to build with LZ4 support... yes > checking for liblz4... no > configure: error: Package requirements (liblz4) were not met: > > No

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Tom Lane
Andrew Dunstan writes: > On 3/20/21 3:03 PM, Tom Lane wrote: >> I fixed up some issues in 0008/0009 (mostly cosmetic, except that >> you forgot a server version check in dumpToastCompression) and >> pushed that, so we can see if it makes crake happy. > It's still produced a significant amount

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Tom Lane
I wrote: > I fixed up some issues in 0008/0009 (mostly cosmetic, except that > you forgot a server version check in dumpToastCompression) and > pushed that, so we can see if it makes crake happy. crake was still unhappy with that:

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Andrew Dunstan
On 3/20/21 3:03 PM, Tom Lane wrote: > I wrote: >> Yeah, _doSetFixedOutputState is the wrong place: that runs on the >> pg_restore side of the fence, and would not have access to the >> necessary info in a separated dump/restore run. >> It might be necessary to explicitly pass the state through

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Tom Lane
I wrote: > Yeah, _doSetFixedOutputState is the wrong place: that runs on the > pg_restore side of the fence, and would not have access to the > necessary info in a separated dump/restore run. > It might be necessary to explicitly pass the state through in a TOC item, > as we do for things like

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Justin Pryzby
On Sat, Mar 20, 2021 at 01:36:15PM -0400, Tom Lane wrote: > Justin Pryzby writes: > > On Sat, Mar 20, 2021 at 04:37:53PM +0530, Dilip Kumar wrote: > >> - And, 0008 and 0009, I think my > >> 0001-Fixup-dump-toast-compression-method.patch[1] is doing this in a > >> much simpler way, please have a

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Tom Lane
Justin Pryzby writes: > On Sat, Mar 20, 2021 at 04:37:53PM +0530, Dilip Kumar wrote: >> - And, 0008 and 0009, I think my >> 0001-Fixup-dump-toast-compression-method.patch[1] is doing this in a >> much simpler way, please have a look and let me know if you think that >> has any problems and we

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Justin Pryzby
On Sat, Mar 20, 2021 at 04:37:53PM +0530, Dilip Kumar wrote: > - 0006 is fine but not sure what is the advantage over what we have today? The advantage is that it's dozens of lines shorter, and automatically includes a HINT. SET default_toast_compression = 'I do not exist compression';

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Justin Pryzby
On Sat, Mar 20, 2021 at 04:13:47PM +0100, Tomas Vondra wrote: > +++ b/src/backend/access/brin/brin_tuple.c > @@ -213,10 +213,20 @@ brin_form_tuple(BrinDesc *brdesc, BlockNumber blkno, > BrinMemTuple *tuple, > (atttype->typstorage == TYPSTORAGE_EXTENDED || >

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Tomas Vondra
On 3/20/21 11:45 AM, Tomas Vondra wrote: > > > On 3/20/21 11:18 AM, Dilip Kumar wrote: >> On Sat, Mar 20, 2021 at 3:05 PM Tomas Vondra >> wrote: >>> >>> Hi, >>> >>> I think this bit in brin_tuple.c is wrong: >>> >>> ... >>> Form_pg_attribute att = TupleDescAttr(brdesc->bd_tupdesc, >>>

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Dilip Kumar
On Sat, Mar 20, 2021 at 1:14 PM Justin Pryzby wrote: > > See attached. I have looked into your patches - 0001 to 0005 and 0007 look fine to me so maybe you can merge them all and create a fixup patch. Thanks for fixing this, these were some silly mistakes I made in my patch. - 0006 is fine but

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Tomas Vondra
On 3/20/21 11:18 AM, Dilip Kumar wrote: > On Sat, Mar 20, 2021 at 3:05 PM Tomas Vondra > wrote: >> >> Hi, >> >> I think this bit in brin_tuple.c is wrong: >> >> ... >> Form_pg_attribute att = TupleDescAttr(brdesc->bd_tupdesc, >> keyno); >>

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Dilip Kumar
On Sat, Mar 20, 2021 at 3:05 PM Tomas Vondra wrote: > > Hi, > > I think this bit in brin_tuple.c is wrong: > > ... > Form_pg_attribute att = TupleDescAttr(brdesc->bd_tupdesc, > keyno); > Datum cvalue = toast_compress_datum(value, >

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Tomas Vondra
Hi, I think this bit in brin_tuple.c is wrong: ... Form_pg_attribute att = TupleDescAttr(brdesc->bd_tupdesc, keyno); Datum cvalue = toast_compress_datum(value, att->attcompression); The

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Andres Freund
On 2021-03-19 15:44:34 -0400, Robert Haas wrote: > I committed the core patch (0003) with a bit more editing. Let's see > what the buildfarm thinks. Congrats Dilip, Robert, All. The slow toast compression has been a significant issue for a long time.

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Justin Pryzby
On Fri, Mar 19, 2021 at 04:38:03PM -0400, Robert Haas wrote: > On Fri, Mar 19, 2021 at 4:20 PM Tom Lane wrote: > > Nope ... crake's displeased with your assumption that it's OK to > > clutter dumps with COMPRESSION clauses. As am I: that is going to > > be utterly fatal for cross-version

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Dilip Kumar
On Sat, Mar 20, 2021 at 1:22 PM Dilip Kumar wrote: > > On Sat, Mar 20, 2021 at 8:11 AM Robert Haas wrote: > > > > On Fri, Mar 19, 2021 at 8:25 PM Tom Lane wrote: > > > Extrapolating from the way we've dealt with similar issues > > > in the past, I think the structure of pg_dump's output ought

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Dilip Kumar
On Sat, Mar 20, 2021 at 8:11 AM Robert Haas wrote: > > On Fri, Mar 19, 2021 at 8:25 PM Tom Lane wrote: > > Extrapolating from the way we've dealt with similar issues > > in the past, I think the structure of pg_dump's output ought to be: > > > > 1. SET default_toast_compression = 'source

Re: [HACKERS] Custom compression methods

2021-03-20 Thread Justin Pryzby
See attached. One issue is that the pg_dump tests are no longer exercising the COMPRESSION clause. I don't know how to improve on that, since lz4 may not be available. ..unless we changed attcompression='\0' to mean (for varlena) "the default compression". Rather than "resolving" to the

Re: [HACKERS] Custom compression methods

2021-03-19 Thread Robert Haas
On Fri, Mar 19, 2021 at 6:38 PM Alvaro Herrera wrote: > I suggest to add comments to this effect, > perhaps as the attached (feel free to reword, I think mine is awkward.) It's not bad, although "the decompressed version of the full datum" might be a little better. I'd probably say instead:

Re: [HACKERS] Custom compression methods

2021-03-19 Thread Robert Haas
On Fri, Mar 19, 2021 at 8:25 PM Tom Lane wrote: > Extrapolating from the way we've dealt with similar issues > in the past, I think the structure of pg_dump's output ought to be: > > 1. SET default_toast_compression = 'source system's value' > in among the existing passel of SETs at the top.

Re: [HACKERS] Custom compression methods

2021-03-19 Thread Andrew Dunstan
On 3/19/21 8:25 PM, Tom Lane wrote: > Alvaro Herrera writes: >> On 2021-Mar-19, Robert Haas wrote: >>> Well, I really do hope that some day in the bright future, pglz will >>> no longer be the thing we're shipping as the postgresql.conf default. >>> So we'd just be postponing the noise until

Re: [HACKERS] Custom compression methods

2021-03-19 Thread Tom Lane
Alvaro Herrera writes: > On 2021-Mar-19, Robert Haas wrote: >> Well, I really do hope that some day in the bright future, pglz will >> no longer be the thing we're shipping as the postgresql.conf default. >> So we'd just be postponing the noise until then. I think we need a >> better idea than

Re: [HACKERS] Custom compression methods

2021-03-19 Thread David Steele
On 3/19/21 8:00 PM, Andres Freund wrote: On 2021-03-19 15:44:34 -0400, Robert Haas wrote: I committed the core patch (0003) with a bit more editing. Let's see what the buildfarm thinks. Congrats Dilip, Robert, All. The slow toast compression has been a significant issue for a long time.

Re: [HACKERS] Custom compression methods

2021-03-19 Thread Alvaro Herrera
On 2021-Mar-19, Robert Haas wrote: > On Fri, Mar 19, 2021 at 6:22 PM Alvaro Herrera > wrote: > > (At least, for binary upgrade surely you must make sure to apply the > > correct setting regardless of defaults on either system). > > It's not critical from a system integrity point of view; the

Re: [HACKERS] Custom compression methods

2021-03-19 Thread Robert Haas
On Fri, Mar 19, 2021 at 6:22 PM Alvaro Herrera wrote: > Do you mean the column storage strategy, attstorage? I don't think > that's really related, because the difference there is not a GUC setting > but a compiled-in default for the type. In the case of compression, I'm > not sure it makes

  1   2   3   4   >