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

Commitcb8885a

Browse files
committed
Fix handling of -d "connection string" in pg_dump/pg_restore.
Parallel pg_dump failed if its -d parameter was a connection stringcontaining any essential information other than host, port, or username.The same was true for pg_restore with --create.The reason is that these scenarios failed to preserve the connectionstring from the command line; the code felt free to replace that withjust the database name when reconnecting from a pg_dump parallel workeror after creating the target database. By chance, parallel pg_restoredid not suffer this defect, as long as you didn't say --create.In practice it seems that the error would be obvious only if theconnstring included essential, non-default SSL or GSS parameters.This may explain why it took us so long to notice. (It also makesit very difficult to craft a regression test case illustrating theproblem, since the test would fail in builds without those options.)Fix by refactoring so that ConnectDatabase always receives all therelevant options directly from the command line, rather thanreconstructed values. Inject a different database name, when necessary,by relying on libpq's rules for handling multiple "dbname" parameters.While here, let's get rid of the essentially duplicate _connectDBfunction, as well as some obsolete nearby cruft.Per bug #16604 from Zsolt Ero. Back-patch to all supported branches.Discussion:https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
1 parent052014a commitcb8885a

File tree

6 files changed

+130
-280
lines changed

6 files changed

+130
-280
lines changed

‎src/bin/pg_dump/pg_backup.h

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,20 @@ typedef enum _teSection
5858
SECTION_POST_DATA/* stuff to be processed after data */
5959
}teSection;
6060

61+
/* Parameters needed by ConnectDatabase; same for dump and restore */
62+
typedefstruct_connParams
63+
{
64+
/* These fields record the actual command line parameters */
65+
char*dbname;/* this may be a connstring! */
66+
char*pgport;
67+
char*pghost;
68+
char*username;
69+
trivaluepromptPassword;
70+
/* If not NULL, this overrides the dbname obtained from command line */
71+
/* (but *only* the DB name, not anything else in the connstring) */
72+
char*override_dbname;
73+
}ConnParams;
74+
6175
typedefstruct_restoreOptions
6276
{
6377
intcreateDB;/* Issue commands to create the database */
@@ -107,12 +121,9 @@ typedef struct _restoreOptions
107121
SimpleStringListtableNames;
108122

109123
intuseDB;
110-
char*dbname;/* subject to expand_dbname */
111-
char*pgport;
112-
char*pghost;
113-
char*username;
124+
ConnParamscparams;/* parameters to use if useDB */
125+
114126
intnoDataForFailedTables;
115-
trivaluepromptPassword;
116127
intexit_on_error;
117128
intcompression;
118129
intsuppressDumpWarnings;/* Suppress output of WARNING entries
@@ -127,10 +138,7 @@ typedef struct _restoreOptions
127138

128139
typedefstruct_dumpOptions
129140
{
130-
constchar*dbname;/* subject to expand_dbname */
131-
constchar*pghost;
132-
constchar*pgport;
133-
constchar*username;
141+
ConnParamscparams;
134142

135143
intbinary_upgrade;
136144

@@ -247,12 +255,9 @@ typedef void (*SetupWorkerPtrType) (Archive *AH);
247255
* Main archiver interface.
248256
*/
249257

250-
externvoidConnectDatabase(Archive*AH,
251-
constchar*dbname,
252-
constchar*pghost,
253-
constchar*pgport,
254-
constchar*username,
255-
trivalueprompt_password);
258+
externvoidConnectDatabase(Archive*AHX,
259+
constConnParams*cparams,
260+
boolisReconnect);
256261
externvoidDisconnectDatabase(Archive*AHX);
257262
externPGconn*GetConnection(Archive*AHX);
258263

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 21 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ InitDumpOptions(DumpOptions *opts)
165165
memset(opts,0,sizeof(DumpOptions));
166166
/* set any fields that shouldn't default to zeroes */
167167
opts->include_everything= true;
168+
opts->cparams.promptPassword=TRI_DEFAULT;
168169
opts->dumpSections=DUMP_UNSECTIONED;
169170
}
170171

@@ -178,6 +179,11 @@ dumpOptionsFromRestoreOptions(RestoreOptions *ropt)
178179
DumpOptions*dopt=NewDumpOptions();
179180

180181
/* this is the inverse of what's at the end of pg_dump.c's main() */
182+
dopt->cparams.dbname=ropt->cparams.dbname ?pg_strdup(ropt->cparams.dbname) :NULL;
183+
dopt->cparams.pgport=ropt->cparams.pgport ?pg_strdup(ropt->cparams.pgport) :NULL;
184+
dopt->cparams.pghost=ropt->cparams.pghost ?pg_strdup(ropt->cparams.pghost) :NULL;
185+
dopt->cparams.username=ropt->cparams.username ?pg_strdup(ropt->cparams.username) :NULL;
186+
dopt->cparams.promptPassword=ropt->cparams.promptPassword;
181187
dopt->outputClean=ropt->dropSchema;
182188
dopt->dataOnly=ropt->dataOnly;
183189
dopt->schemaOnly=ropt->schemaOnly;
@@ -410,9 +416,7 @@ RestoreArchive(Archive *AHX)
410416
AHX->minRemoteVersion=0;
411417
AHX->maxRemoteVersion=9999999;
412418

413-
ConnectDatabase(AHX,ropt->dbname,
414-
ropt->pghost,ropt->pgport,ropt->username,
415-
ropt->promptPassword);
419+
ConnectDatabase(AHX,&ropt->cparams, false);
416420

417421
/*
418422
* If we're talking to the DB directly, don't send comments since they
@@ -832,16 +836,8 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
832836
if (strcmp(te->desc,"DATABASE")==0||
833837
strcmp(te->desc,"DATABASE PROPERTIES")==0)
834838
{
835-
PQExpBufferDataconnstr;
836-
837-
initPQExpBuffer(&connstr);
838-
appendPQExpBufferStr(&connstr,"dbname=");
839-
appendConnStrVal(&connstr,te->tag);
840-
/* Abandon struct, but keep its buffer until process exit. */
841-
842839
pg_log_info("connecting to new database \"%s\"",te->tag);
843840
_reconnectToDB(AH,te->tag);
844-
ropt->dbname=connstr.data;
845841
}
846842
}
847843

@@ -973,7 +969,7 @@ NewRestoreOptions(void)
973969

974970
/* set any fields that shouldn't default to zeroes */
975971
opts->format=archUnknown;
976-
opts->promptPassword=TRI_DEFAULT;
972+
opts->cparams.promptPassword=TRI_DEFAULT;
977973
opts->dumpSections=DUMP_UNSECTIONED;
978974

979975
returnopts;
@@ -2355,8 +2351,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
23552351
else
23562352
AH->format=fmt;
23572353

2358-
AH->promptPassword=TRI_DEFAULT;
2359-
23602354
switch (AH->format)
23612355
{
23622356
casearchCustom:
@@ -3217,27 +3211,20 @@ _doSetSessionAuth(ArchiveHandle *AH, const char *user)
32173211
* If we're currently restoring right into a database, this will
32183212
* actually establish a connection. Otherwise it puts a \connect into
32193213
* the script output.
3220-
*
3221-
* NULL dbname implies reconnecting to the current DB (pretty useless).
32223214
*/
32233215
staticvoid
32243216
_reconnectToDB(ArchiveHandle*AH,constchar*dbname)
32253217
{
32263218
if (RestoringToDB(AH))
3227-
ReconnectToServer(AH,dbname,NULL);
3219+
ReconnectToServer(AH,dbname);
32283220
else
32293221
{
3230-
if (dbname)
3231-
{
3232-
PQExpBufferDataconnectbuf;
3222+
PQExpBufferDataconnectbuf;
32333223

3234-
initPQExpBuffer(&connectbuf);
3235-
appendPsqlMetaConnect(&connectbuf,dbname);
3236-
ahprintf(AH,"%s\n",connectbuf.data);
3237-
termPQExpBuffer(&connectbuf);
3238-
}
3239-
else
3240-
ahprintf(AH,"%s\n","\\connect -\n");
3224+
initPQExpBuffer(&connectbuf);
3225+
appendPsqlMetaConnect(&connectbuf,dbname);
3226+
ahprintf(AH,"%s\n",connectbuf.data);
3227+
termPQExpBuffer(&connectbuf);
32413228
}
32423229

32433230
/*
@@ -4169,9 +4156,7 @@ restore_toc_entries_postfork(ArchiveHandle *AH, TocEntry *pending_list)
41694156
/*
41704157
* Now reconnect the single parent connection.
41714158
*/
4172-
ConnectDatabase((Archive*)AH,ropt->dbname,
4173-
ropt->pghost,ropt->pgport,ropt->username,
4174-
ropt->promptPassword);
4159+
ConnectDatabase((Archive*)AH,&ropt->cparams, true);
41754160

41764161
/* re-establish fixed state */
41774162
_doSetFixedOutputState(AH);
@@ -4833,54 +4818,15 @@ CloneArchive(ArchiveHandle *AH)
48334818
clone->public.n_errors=0;
48344819

48354820
/*
4836-
* Connect our new clone object to the database: In parallel restore the
4837-
* parent is already disconnected, because we can connect the worker
4838-
* processes independently to the database (no snapshot sync required). In
4839-
* parallel backup we clone the parent's existing connection.
4821+
* Connect our new clone object to the database, using the same connection
4822+
* parameters used for the original connection.
48404823
*/
4841-
if (AH->mode==archModeRead)
4842-
{
4843-
RestoreOptions*ropt=AH->public.ropt;
4844-
4845-
Assert(AH->connection==NULL);
4846-
4847-
/* this also sets clone->connection */
4848-
ConnectDatabase((Archive*)clone,ropt->dbname,
4849-
ropt->pghost,ropt->pgport,ropt->username,
4850-
ropt->promptPassword);
4824+
ConnectDatabase((Archive*)clone,&clone->public.ropt->cparams, true);
48514825

4852-
/* re-establish fixed state */
4826+
/* re-establish fixed state */
4827+
if (AH->mode==archModeRead)
48534828
_doSetFixedOutputState(clone);
4854-
}
4855-
else
4856-
{
4857-
PQExpBufferDataconnstr;
4858-
char*pghost;
4859-
char*pgport;
4860-
char*username;
4861-
4862-
Assert(AH->connection!=NULL);
4863-
4864-
/*
4865-
* Even though we are technically accessing the parent's database
4866-
* object here, these functions are fine to be called like that
4867-
* because all just return a pointer and do not actually send/receive
4868-
* any data to/from the database.
4869-
*/
4870-
initPQExpBuffer(&connstr);
4871-
appendPQExpBufferStr(&connstr,"dbname=");
4872-
appendConnStrVal(&connstr,PQdb(AH->connection));
4873-
pghost=PQhost(AH->connection);
4874-
pgport=PQport(AH->connection);
4875-
username=PQuser(AH->connection);
4876-
4877-
/* this also sets clone->connection */
4878-
ConnectDatabase((Archive*)clone,connstr.data,
4879-
pghost,pgport,username,TRI_NO);
4880-
4881-
termPQExpBuffer(&connstr);
4882-
/* setupDumpWorker will fix up connection state */
4883-
}
4829+
/* in write case, setupDumpWorker will fix up connection state */
48844830

48854831
/* Let the format-specific code have a chance too */
48864832
clone->ClonePtr(clone);

‎src/bin/pg_dump/pg_backup_archiver.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,6 @@ struct _archiveHandle
303303

304304
/* Stuff for direct DB connection */
305305
char*archdbname;/* DB name *read* from archive */
306-
trivaluepromptPassword;
307306
char*savedPassword;/* password for ropt->username, if known */
308307
char*use_role;
309308
PGconn*connection;
@@ -471,7 +470,7 @@ extern void InitArchiveFmt_Tar(ArchiveHandle *AH);
471470

472471
externboolisValidTarHeader(char*header);
473472

474-
externvoidReconnectToServer(ArchiveHandle*AH,constchar*dbname,constchar*newUser);
473+
externvoidReconnectToServer(ArchiveHandle*AH,constchar*dbname);
475474
externvoidDropBlobIfExists(ArchiveHandle*AH,Oidoid);
476475

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

0 commit comments

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