From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew@dunslane.net>
cc: Justin Pryzby <pryzby@telsasoft.com>, Dilip Kumar <dilipbalaut@gmail.com>,
        Robert Haas <robertmhaas@gmail.com>,
        Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Andres Freund <andres@anarazel.de>,
        Tomas Vondra <tomas.vondra@2ndquadrant.com>,
        Alexander Korotkov <a.korotkov@postgrespro.ru>,
        David Steele <david@pgmasters.net>,
        Ildus Kurbangaliev <i.kurbangaliev@gmail.com>,
        Dmitry Dolgov <9erthalion6@gmail.com>,
        pgsql-hackers@lists.postgresql.org
Subject: Re: [HACKERS] Custom compression methods
In-reply-to: <fe462d41-7063-72a1-2e70-0300c53ee2c4@dunslane.net>
References: <20210319222242.GA23930@alvherre.pgsql>
 <CA+TgmoYQ8hn1apJXbmxutYv-sHkTOUdPxdFJUzvZ7uT4cUT+cQ@mail.gmail.com>
 <CA+TgmobGm9umTgno4O4UqSAoGhOPOAA55uiKE8N_wehnHfg4Tw@mail.gmail.com>
 <4158590.1616185209@sss.pgh.pa.us> <20210320000625.GP11765@telsasoft.com>
 <20210320074420.GR11765@telsasoft.com>
 <CAFiTN-uX=hLscjL=0raA6dYcoiZB=9YqL3DmX15CMvZiu+WB+w@mail.gmail.com>
 <20210320153933.GT11765@telsasoft.com> <229250.1616261775@sss.pgh.pa.us>
 <261449.1616267030@sss.pgh.pa.us>
 <fe462d41-7063-72a1-2e70-0300c53ee2c4@dunslane.net>
Comments: In-reply-to Andrew Dunstan <andrew@dunslane.net>
	message dated "Sat, 20 Mar 2021 16:37:51 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <1099279.1616600518.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Wed, 24 Mar 2021 11:41:58 -0400
Message-ID: <1099280.1616600518@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1099280.1616600518%40sss.pgh.pa.us>
Precedence: bulk

Andrew Dunstan <andrew@dunslane.net> 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 more difference between the
> dumps. For now I've increased the fuzz factor a bit like this:
> -   if (   ($oversion ne $this_branch && $difflines < 2000)
> +   if (   ($oversion ne $this_branch && $difflines < 2700)
> I'll try to come up with something better. Maybe just ignore lines like
>     SET default_toast_compression = 'pglz';
> when taking the diff.

I see that some other buildfarm animals besides your own critters
are still failing the xversion tests, presumably because they lack
this hack :-(.

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 than omitting any mention of the variable, which is what
I made it do in aa25d1089.  If we changed that, I think all these
diffs would go away.  Am I right in thinking that what's being
compared here is new pg_dump's dump from old server versus new
pg_dump's dump from new server?

The "strong case" goes like this: initdb a v14 cluster, change
default_toast_compression to lz4 in its postgresql.conf, then
try to pg_upgrade from an old server.  If the dump script doesn't
set default_toast_compression = 'pglz' then the upgrade will
do the wrong thing because all the tables will be recreated with
a different behavior than they had before.  IIUC, this wouldn't
result in broken data, but it still seems to me to be undesirable.
dump/restore ought to do its best to preserve the old DB state,
unless you explicitly tell it --no-toast-compression or the like.

			regards, tom lane