Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit160a4f6

Browse files
committed
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 tointerpreting the reloaded data correctly. Another argument why we can'treally get away with the scheme proposed in commitb3f8401 is that itcannot work for parallel restore: even if the parent process manages tohang onto the previous GUC state, worker processes would see the statepost-ALTER-DATABASE. (Perhaps we could have dodged that bullet bydelaying DATABASE PROPERTIES restoration to the end of the run, butthat does nothing for the data semantics problem.)This leaves us with no solution for the default_transaction_read_only issuethat commit4bd371f intended to work around, other than "you gotta removesuch settings before dumping/upgrading". However, in view of the fact thatparallel restore broke that hack years ago and no one has noticed, it'sfair to question how many people care. I'm unexcited about adding a largedollop of new complexity to handle that corner case.This would be a one-liner fix, except it turns out that ReconnectToServertries to optimize away "redundant" reconnections. While that may have beenvaluable when coded, a quick survey of current callers shows that there areno cases where that's actually useful, so just remove that check. While atit, remove the function's useless return value.Discussion:https://postgr.es/m/12453.1516655001@sss.pgh.pa.us
1 parenta541dbb commit160a4f6

File tree

4 files changed

+16
-22
lines changed

4 files changed

+16
-22
lines changed

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -833,8 +833,13 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
833833
}
834834
}
835835

836-
/* If we created a DB, connect to it... */
837-
if (strcmp(te->desc,"DATABASE")==0)
836+
/*
837+
* If we created a DB, connect to it. Also, if we changed DB
838+
* properties, reconnect to ensure that relevant GUC settings are
839+
* applied to our session.
840+
*/
841+
if (strcmp(te->desc,"DATABASE")==0||
842+
strcmp(te->desc,"DATABASE PROPERTIES")==0)
838843
{
839844
PQExpBufferDataconnstr;
840845

‎src/bin/pg_dump/pg_backup_archiver.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ extern void InitArchiveFmt_Tar(ArchiveHandle *AH);
448448

449449
externboolisValidTarHeader(char*header);
450450

451-
externintReconnectToServer(ArchiveHandle*AH,constchar*dbname,constchar*newUser);
451+
externvoidReconnectToServer(ArchiveHandle*AH,constchar*dbname,constchar*newUser);
452452
externvoidDropBlobIfExists(ArchiveHandle*AH,Oidoid);
453453

454454
voidahwrite(constvoid*ptr,size_tsize,size_tnmemb,ArchiveHandle*AH);

‎src/bin/pg_dump/pg_backup_db.c

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,9 @@ _check_database_version(ArchiveHandle *AH)
7676
/*
7777
* Reconnect to the server. If dbname is not NULL, use that database,
7878
* else the one associated with the archive handle. If username is
79-
* not NULL, use that user name, else the one from the handle. If
80-
* both the database and the user match the existing connection already,
81-
* nothing will be done.
82-
*
83-
* Returns 1 in any case.
79+
* not NULL, use that user name, else the one from the handle.
8480
*/
85-
int
81+
void
8682
ReconnectToServer(ArchiveHandle*AH,constchar*dbname,constchar*username)
8783
{
8884
PGconn*newConn;
@@ -99,20 +95,13 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
9995
else
10096
newusername=username;
10197

102-
/* Let's see if the request is already satisfied */
103-
if (strcmp(newdbname,PQdb(AH->connection))==0&&
104-
strcmp(newusername,PQuser(AH->connection))==0)
105-
return1;
106-
10798
newConn=_connectDB(AH,newdbname,newusername);
10899

109100
/* Update ArchiveHandle's connCancel before closing old connection */
110101
set_archive_cancel_info(AH,newConn);
111102

112103
PQfinish(AH->connection);
113104
AH->connection=newConn;
114-
115-
return1;
116105
}
117106

118107
/*

‎src/bin/pg_dump/pg_dump.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2819,10 +2819,11 @@ dumpDatabase(Archive *fout)
28192819

28202820
/*
28212821
* Now construct a DATABASE PROPERTIES archive entry to restore any
2822-
* non-default database-level properties. We want to do this after
2823-
* reconnecting so that these properties won't apply during the restore
2824-
* session. In this way, restoring works even if there is, say, an ALTER
2825-
* DATABASE SET that turns on default_transaction_read_only.
2822+
* non-default database-level properties. (The reason this must be
2823+
* separate is that we cannot put any additional commands into the TOC
2824+
* entry that has CREATE DATABASE. pg_restore would execute such a group
2825+
* in an implicit transaction block, and the backend won't allow CREATE
2826+
* DATABASE in that context.)
28262827
*/
28272828
resetPQExpBuffer(creaQry);
28282829
resetPQExpBuffer(delQry);
@@ -2854,8 +2855,7 @@ dumpDatabase(Archive *fout)
28542855

28552856
/*
28562857
* We stick this binary-upgrade query into the DATABASE PROPERTIES archive
2857-
* entry, too. It can't go into the DATABASE entry because that would
2858-
* result in an implicit transaction block around the CREATE DATABASE.
2858+
* entry, too, for lack of a better place.
28592859
*/
28602860
if (dopt->binary_upgrade)
28612861
{

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp