diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2018-01-23 10:55:08 -0500 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2018-01-23 10:55:16 -0500 |
| commit | 160a4f62ee7b8a96984f8bef19c90488aa6c8045 (patch) | |
| tree | 94132d90908747df1f154ccf50d5263c8cb8d878 /src/bin/pg_dump/pg_backup_db.c | |
| parent | a541dbb6fa389bb0ffdd24a403bc6d276d77a074 (diff) | |
| download | postgresql-160a4f62ee7b8a96984f8bef19c90488aa6c8045.tar.gz postgresql-160a4f62ee7b8a96984f8bef19c90488aa6c8045.zip | |
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'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
Diffstat (limited to 'src/bin/pg_dump/pg_backup_db.c')
| -rw-r--r-- | src/bin/pg_dump/pg_backup_db.c | 15 |
1 files changed, 2 insertions, 13 deletions
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index 216853d6272..3b7dd241513 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -76,13 +76,9 @@ _check_database_version(ArchiveHandle *AH) /* * Reconnect to the server. If dbname is not NULL, use that database, * else the one associated with the archive handle. If username is - * not NULL, use that user name, else the one from the handle. If - * both the database and the user match the existing connection already, - * nothing will be done. - * - * Returns 1 in any case. + * not NULL, use that user name, else the one from the handle. */ -int +void ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username) { PGconn *newConn; @@ -99,11 +95,6 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username) else newusername = username; - /* Let's see if the request is already satisfied */ - if (strcmp(newdbname, PQdb(AH->connection)) == 0 && - strcmp(newusername, PQuser(AH->connection)) == 0) - return 1; - newConn = _connectDB(AH, newdbname, newusername); /* Update ArchiveHandle's connCancel before closing old connection */ @@ -111,8 +102,6 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username) PQfinish(AH->connection); AH->connection = newConn; - - return 1; } /* |