Re: pgsql: Move handling of database properties from pg_dumpall into pg_dum

Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> writes:
> On Tue, Jan 23, 2018 at 8:56 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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