Re: == PostgreSQL Weekly News - January 28 2018 ==

On Mon, Jan 29, 2018 at 03:57:48AM +0100, David Fetter wrote:

> == PostgreSQL Weekly News - January 28 2018 ==
...
> == Applied Patches ==
...
> Tom Lane pushed:
...
> - In pg_dump, force reconnection after issuing ALTER DATABASE SET command(s).
>   The folly of not doing this was exposed by the buildfarm: in some cases, the
>   GUC settings applied through ALTER DATABASE SET may be essential to
>   interpreting the reloaded data correctly.  Another argument why we can't
>   really get away with the scheme proposed in commit b3f840120 is that it cannot
>   work for parallel restore: even if the parent process manages to hang onto the
>   previous GUC state, worker processes would see the state post-ALTER-DATABASE.
>   (Perhaps we could have dodged that bullet by delaying DATABASE PROPERTIES
>   restoration to the end of the run, but that does nothing for the data
>   semantics problem.) This leaves us with no solution for the
>   default_transaction_read_only issue that commit 4bd371f6f intended to work
>   around, other than "you gotta remove such settings before dumping/upgrading".
>   However, in view of the fact that parallel restore broke that hack years ago
>   and no one has noticed, it's fair to question how many people care.

I do (I am OP from the original report). The reason no one's
noticed is that so far Debian's pg_upgradecluster doesn't by
default seem to use more than one job for restore (it does
use pg_dump/pg_restore over pg_upgrade by default).

>   I'm unexcited about adding a large dollop of new complexity to handle that corner
>   case.  This would be a one-liner fix, except it turns out that
>   ReconnectToServer tries to optimize away "redundant" reconnections.  While
>   that may have been valuable when coded, a quick survey of current callers
>   shows that there are no cases where that's actually useful, so just remove
>   that check.  While at it, remove the function's useless return value.
>   Discussion: https://postgr.es/m/12453.1516655001@sss.pgh.pa.us
>   https://git.postgresql.org/pg/commitdiff/160a4f62ee7b8a96984f8bef19c90488aa6c8045

However, reading _this_ commit message ...

> - Move handling of database properties from pg_dumpall into pg_dump.  This patch
>   rearranges the division of labor between pg_dump and pg_dumpall so that
>   pg_dump itself handles all properties attached to a single database.  Notably,
>   a database's ACL (GRANT/REVOKE status) and local GUC settings established by
>   ALTER DATABASE SET and ALTER ROLE IN DATABASE SET can be dumped and restored
>   by pg_dump.  This is a long-requested improvement.  "pg_dumpall -g" will now
>   produce only role- and tablespace-related output, nothing about individual
>   databases.  The total output of a regular pg_dumpall run remains the same.
>   pg_dump (or pg_restore) will restore database-level properties only when
>   creating the target database with --create.  This applies not only to ACLs and
>   GUCs but to the other database properties it already handled, that is database
>   comments and security labels.  This is more consistent and useful, but does
>   represent an incompatibility in the behavior seen without --create.  (This
>   change makes the proposed patch to have pg_dump use "COMMENT ON DATABASE
>   CURRENT_DATABASE" unnecessary, since there is no case where the command is
>   issued that we won't know the true name of the database.  We might still want
>   that patch as a feature in its own right, but pg_dump no longer needs it.)
>   pg_dumpall with --clean will now drop and recreate the "postgres" and
>   "template1" databases in the target cluster, allowing their locale and
>   encoding settings to be changed if necessary, and providing a cleaner way to
>   set nondefault tablespaces for them than we had before.  This means that such
>   a script must now always be started in the "postgres" database; the order of
>   drops and reconnects will not work otherwise.  Without --clean, the script
>   will not adjust any database-level properties of those two databases
>   (including their comments, ACLs, and security labels, which it formerly would
>   try to set).  Another minor incompatibility is that the CREATE DATABASE
>   commands in a pg_dumpall script will now always specify locale and encoding
>   settings.  Formerly those would be omitted if they matched the cluster's
>   default.  While that behavior had some usefulness in some migration scenarios,
>   it also posed a significant hazard of unwanted locale/encoding changes.  To
>   migrate to another locale/encoding, it's now necessary to use pg_dump without
>   --create to restore into a database with the desired settings.  Commit
>   4bd371f6f's hack to emit "SET default_transaction_read_only = off" is gone: we
>   now dodge that problem by the expedient of not issuing ALTER DATABASE SET
>   commands until after reconnecting to the target database.  Therefore, such
>   settings won't apply during the restore session.  In passing, improve some
>   shaky grammar in the docs, and add a note pointing out that pg_dumpall's
>   output can't be expected to load without any errors.  (Someday we might want
>   to fix that, but this is not that patch.) Haribabu Kommi, reviewed at various
>   times by Andreas Karlsson, Vaishnavi Prabakaran, and Robert Haas; further
>   hacking by me.  Discussion:
>   https://postgr.es/m/CAJrrPGcUurV0eWTeXODwsOYFN=Ekq36t1s0YnFYUNzsmRfdAyA@mail.gmail.com
>   https://git.postgresql.org/pg/commitdiff/b3f8401205afdaf63cb20dc316d44644c933d5a1

... am I right in assuming my use case (dumping/restoring
databases with default_transaction_read_only=True) should
still work without removing that setting beforehand ?

Thanks,
Karsten
-- 
GPG key ID E4071346 @ eu.pool.sks-keyservers.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346