From: Tom Lane <tgl@sss.pgh.pa.us>
To: Haribabu Kommi <kommi.haribabu@gmail.com>
cc: pgsql-hackers@lists.postgresql.org
Subject: Re: pgsql: Move handling of database properties from pg_dumpall into
 pg_dum
In-reply-to: 
 <CAJrrPGfcTB20J6B0bBL2EPiufrvm6bHYL3A1EbCrn81Ap5Us-w@mail.gmail.com>
References: <E1edhT6-0002lM-3m@gemulon.postgresql.org>
 <12453.1516655001@sss.pgh.pa.us> <17972.1516658167@sss.pgh.pa.us>
 <CAJrrPGfcTB20J6B0bBL2EPiufrvm6bHYL3A1EbCrn81Ap5Us-w@mail.gmail.com>
Comments: In-reply-to Haribabu Kommi <kommi.haribabu@gmail.com>
	message dated "Tue, 23 Jan 2018 18:23:35 +1100"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <32318.1516724640.1@sss.pgh.pa.us>
Date: Tue, 23 Jan 2018 11:24:00 -0500
Message-ID: <32319.1516724640@sss.pgh.pa.us>
Precedence: bulk

Haribabu Kommi <kommi.haribabu@gmail.com> writes:
> On Tue, Jan 23, 2018 at 8:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm thinking we'd still need to do what I said in the previous message,
>> and change pg_dump so that the restore session will absorb ALTER DATABASE
>> settings before proceeding.  Otherwise, at minimum, we have a hazard of
>> differing behaviors in serial and parallel restores.  It might be that
>> lc_monetary is the only GUC that makes a real difference for this purpose,
>> but I haven't got much faith in that proposition at the moment.

> Yes, restore session should absorb all the ALTER DATABASE and ALTER ROLE
> IN DATABASE settings also to make sure that the target database is in same
> state when the dump has started.

I've pushed a patch to do that.

> currently "default_transaction_read_only" is the only GUC that affects the
> absorbing the restore of a database.

Well, that's exactly the $64 question.  Are we sure we have a complete,
reliable list of which GUCs do or do not affect data restoration?
I wouldn't count on it.  If nothing else, extensions might have custom
GUCs that we could not predict the behavior of.

> As you said in upthread, I feel splitting them into two _TOC entries and
> dump as last commands of each database? Does it have any problem with
> parallel restore?

As I said yesterday, I'm not really eager to do that.  It's a lot of
complexity and a continuing maintenance headache to solve a use-case
that I find pretty debatable.  Anyone who's actually put in
"default_transaction_read_only = on" in a way that restricts their
maintenance roles must have created procedures for undoing it easily;
otherwise day-to-day maintenance would be a problem.  Further, I don't
find the original hack's distinction between pg_dump and pg_dumpall
to be really convincing.  Maybe that says we should resolve this by just
sticking "SET default_transaction_read_only = off" into regular pg_dump
output after all --- perhaps with a switch added to enable it.  But I'd
kind of like to see the argument why we should worry about this at all
made afresh.

			regards, tom lane