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

Commita45bc8a

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 parent55b7e2f commita45bc8a

File tree

6 files changed

+130
-281
lines changed

6 files changed

+130
-281
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;
@@ -2345,8 +2341,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
23452341
else
23462342
AH->format=fmt;
23472343

2348-
AH->promptPassword=TRI_DEFAULT;
2349-
23502344
switch (AH->format)
23512345
{
23522346
casearchCustom:
@@ -3207,27 +3201,20 @@ _doSetSessionAuth(ArchiveHandle *AH, const char *user)
32073201
* If we're currently restoring right into a database, this will
32083202
* actually establish a connection. Otherwise it puts a \connect into
32093203
* the script output.
3210-
*
3211-
* NULL dbname implies reconnecting to the current DB (pretty useless).
32123204
*/
32133205
staticvoid
32143206
_reconnectToDB(ArchiveHandle*AH,constchar*dbname)
32153207
{
32163208
if (RestoringToDB(AH))
3217-
ReconnectToServer(AH,dbname,NULL);
3209+
ReconnectToServer(AH,dbname);
32183210
else
32193211
{
3220-
if (dbname)
3221-
{
3222-
PQExpBufferDataconnectbuf;
3212+
PQExpBufferDataconnectbuf;
32233213

3224-
initPQExpBuffer(&connectbuf);
3225-
appendPsqlMetaConnect(&connectbuf,dbname);
3226-
ahprintf(AH,"%s\n",connectbuf.data);
3227-
termPQExpBuffer(&connectbuf);
3228-
}
3229-
else
3230-
ahprintf(AH,"%s\n","\\connect -\n");
3214+
initPQExpBuffer(&connectbuf);
3215+
appendPsqlMetaConnect(&connectbuf,dbname);
3216+
ahprintf(AH,"%s\n",connectbuf.data);
3217+
termPQExpBuffer(&connectbuf);
32313218
}
32323219

32333220
/*
@@ -4159,9 +4146,7 @@ restore_toc_entries_postfork(ArchiveHandle *AH, TocEntry *pending_list)
41594146
/*
41604147
* Now reconnect the single parent connection.
41614148
*/
4162-
ConnectDatabase((Archive*)AH,ropt->dbname,
4163-
ropt->pghost,ropt->pgport,ropt->username,
4164-
ropt->promptPassword);
4149+
ConnectDatabase((Archive*)AH,&ropt->cparams, true);
41654150

41664151
/* re-establish fixed state */
41674152
_doSetFixedOutputState(AH);
@@ -4823,54 +4808,15 @@ CloneArchive(ArchiveHandle *AH)
48234808
clone->public.n_errors=0;
48244809

48254810
/*
4826-
* Connect our new clone object to the database: In parallel restore the
4827-
* parent is already disconnected, because we can connect the worker
4828-
* processes independently to the database (no snapshot sync required). In
4829-
* parallel backup we clone the parent's existing connection.
4811+
* Connect our new clone object to the database, using the same connection
4812+
* parameters used for the original connection.
48304813
*/
4831-
if (AH->mode==archModeRead)
4832-
{
4833-
RestoreOptions*ropt=AH->public.ropt;
4834-
4835-
Assert(AH->connection==NULL);
4836-
4837-
/* this also sets clone->connection */
4838-
ConnectDatabase((Archive*)clone,ropt->dbname,
4839-
ropt->pghost,ropt->pgport,ropt->username,
4840-
ropt->promptPassword);
4814+
ConnectDatabase((Archive*)clone,&clone->public.ropt->cparams, true);
48414815

4842-
/* re-establish fixed state */
4816+
/* re-establish fixed state */
4817+
if (AH->mode==archModeRead)
48434818
_doSetFixedOutputState(clone);
4844-
}
4845-
else
4846-
{
4847-
PQExpBufferDataconnstr;
4848-
char*pghost;
4849-
char*pgport;
4850-
char*username;
4851-
4852-
Assert(AH->connection!=NULL);
4853-
4854-
/*
4855-
* Even though we are technically accessing the parent's database
4856-
* object here, these functions are fine to be called like that
4857-
* because all just return a pointer and do not actually send/receive
4858-
* any data to/from the database.
4859-
*/
4860-
initPQExpBuffer(&connstr);
4861-
appendPQExpBufferStr(&connstr,"dbname=");
4862-
appendConnStrVal(&connstr,PQdb(AH->connection));
4863-
pghost=PQhost(AH->connection);
4864-
pgport=PQport(AH->connection);
4865-
username=PQuser(AH->connection);
4866-
4867-
/* this also sets clone->connection */
4868-
ConnectDatabase((Archive*)clone,connstr.data,
4869-
pghost,pgport,username,TRI_NO);
4870-
4871-
termPQExpBuffer(&connstr);
4872-
/* setupDumpWorker will fix up connection state */
4873-
}
4819+
/* in write case, setupDumpWorker will fix up connection state */
48744820

48754821
/* Let the format-specific code have a chance too */
48764822
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