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@xxxxxxxxxxxxx > 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@xxxxxxxxxxxxxx > 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
