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