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

Commit1888ff8

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 parentdd36d6b commit1888ff8

File tree

6 files changed

+131
-291
lines changed

6 files changed

+131
-291
lines changed

‎src/bin/pg_dump/pg_backup.h

Lines changed: 21 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 */
@@ -106,12 +120,9 @@ typedef struct _restoreOptions
106120
SimpleStringListtableNames;
107121

108122
intuseDB;
109-
char*dbname;/* subject to expand_dbname */
110-
char*pgport;
111-
char*pghost;
112-
char*username;
123+
ConnParamscparams;/* parameters to use if useDB */
124+
113125
intnoDataForFailedTables;
114-
trivaluepromptPassword;
115126
intexit_on_error;
116127
intcompression;
117128
intsuppressDumpWarnings;/* Suppress output of WARNING entries
@@ -126,10 +137,8 @@ typedef struct _restoreOptions
126137

127138
typedefstruct_dumpOptions
128139
{
129-
constchar*dbname;/* subject to expand_dbname */
130-
constchar*pghost;
131-
constchar*pgport;
132-
constchar*username;
140+
ConnParamscparams;
141+
133142
booloids;
134143

135144
intbinary_upgrade;
@@ -245,12 +254,9 @@ typedef void (*SetupWorkerPtrType) (Archive *AH);
245254
* Main archiver interface.
246255
*/
247256

248-
externvoidConnectDatabase(Archive*AH,
249-
constchar*dbname,
250-
constchar*pghost,
251-
constchar*pgport,
252-
constchar*username,
253-
trivalueprompt_password);
257+
externvoidConnectDatabase(Archive*AHX,
258+
constConnParams*cparams,
259+
boolisReconnect);
254260
externvoidDisconnectDatabase(Archive*AHX);
255261
externPGconn*GetConnection(Archive*AHX);
256262

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 21 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ InitDumpOptions(DumpOptions *opts)
144144
memset(opts,0,sizeof(DumpOptions));
145145
/* set any fields that shouldn't default to zeroes */
146146
opts->include_everything= true;
147+
opts->cparams.promptPassword=TRI_DEFAULT;
147148
opts->dumpSections=DUMP_UNSECTIONED;
148149
}
149150

@@ -157,6 +158,11 @@ dumpOptionsFromRestoreOptions(RestoreOptions *ropt)
157158
DumpOptions*dopt=NewDumpOptions();
158159

159160
/* this is the inverse of what's at the end of pg_dump.c's main() */
161+
dopt->cparams.dbname=ropt->cparams.dbname ?pg_strdup(ropt->cparams.dbname) :NULL;
162+
dopt->cparams.pgport=ropt->cparams.pgport ?pg_strdup(ropt->cparams.pgport) :NULL;
163+
dopt->cparams.pghost=ropt->cparams.pghost ?pg_strdup(ropt->cparams.pghost) :NULL;
164+
dopt->cparams.username=ropt->cparams.username ?pg_strdup(ropt->cparams.username) :NULL;
165+
dopt->cparams.promptPassword=ropt->cparams.promptPassword;
160166
dopt->outputClean=ropt->dropSchema;
161167
dopt->dataOnly=ropt->dataOnly;
162168
dopt->schemaOnly=ropt->schemaOnly;
@@ -401,9 +407,7 @@ RestoreArchive(Archive *AHX)
401407
AHX->minRemoteVersion=0;
402408
AHX->maxRemoteVersion=9999999;
403409

404-
ConnectDatabase(AHX,ropt->dbname,
405-
ropt->pghost,ropt->pgport,ropt->username,
406-
ropt->promptPassword);
410+
ConnectDatabase(AHX,&ropt->cparams, false);
407411

408412
/*
409413
* If we're talking to the DB directly, don't send comments since they
@@ -823,16 +827,8 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
823827
/* If we created a DB, connect to it... */
824828
if (strcmp(te->desc,"DATABASE")==0)
825829
{
826-
PQExpBufferDataconnstr;
827-
828-
initPQExpBuffer(&connstr);
829-
appendPQExpBufferStr(&connstr,"dbname=");
830-
appendConnStrVal(&connstr,te->tag);
831-
/* Abandon struct, but keep its buffer until process exit. */
832-
833830
ahlog(AH,1,"connecting to new database \"%s\"\n",te->tag);
834831
_reconnectToDB(AH,te->tag);
835-
ropt->dbname=connstr.data;
836832
}
837833
}
838834

@@ -966,7 +962,7 @@ NewRestoreOptions(void)
966962

967963
/* set any fields that shouldn't default to zeroes */
968964
opts->format=archUnknown;
969-
opts->promptPassword=TRI_DEFAULT;
965+
opts->cparams.promptPassword=TRI_DEFAULT;
970966
opts->dumpSections=DUMP_UNSECTIONED;
971967

972968
returnopts;
@@ -2398,8 +2394,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
23982394
else
23992395
AH->format=fmt;
24002396

2401-
AH->promptPassword=TRI_DEFAULT;
2402-
24032397
switch (AH->format)
24042398
{
24052399
casearchCustom:
@@ -3188,27 +3182,20 @@ _doSetWithOids(ArchiveHandle *AH, const bool withOids)
31883182
* If we're currently restoring right into a database, this will
31893183
* actually establish a connection. Otherwise it puts a \connect into
31903184
* the script output.
3191-
*
3192-
* NULL dbname implies reconnecting to the current DB (pretty useless).
31933185
*/
31943186
staticvoid
31953187
_reconnectToDB(ArchiveHandle*AH,constchar*dbname)
31963188
{
31973189
if (RestoringToDB(AH))
3198-
ReconnectToServer(AH,dbname,NULL);
3190+
ReconnectToServer(AH,dbname);
31993191
else
32003192
{
3201-
if (dbname)
3202-
{
3203-
PQExpBufferDataconnectbuf;
3193+
PQExpBufferDataconnectbuf;
32043194

3205-
initPQExpBuffer(&connectbuf);
3206-
appendPsqlMetaConnect(&connectbuf,dbname);
3207-
ahprintf(AH,"%s\n",connectbuf.data);
3208-
termPQExpBuffer(&connectbuf);
3209-
}
3210-
else
3211-
ahprintf(AH,"%s\n","\\connect -\n");
3195+
initPQExpBuffer(&connectbuf);
3196+
appendPsqlMetaConnect(&connectbuf,dbname);
3197+
ahprintf(AH,"%s\n",connectbuf.data);
3198+
termPQExpBuffer(&connectbuf);
32123199
}
32133200

32143201
/*
@@ -4138,9 +4125,7 @@ restore_toc_entries_postfork(ArchiveHandle *AH, TocEntry *pending_list)
41384125
/*
41394126
* Now reconnect the single parent connection.
41404127
*/
4141-
ConnectDatabase((Archive*)AH,ropt->dbname,
4142-
ropt->pghost,ropt->pgport,ropt->username,
4143-
ropt->promptPassword);
4128+
ConnectDatabase((Archive*)AH,&ropt->cparams, true);
41444129

41454130
/* re-establish fixed state */
41464131
_doSetFixedOutputState(AH);
@@ -4729,54 +4714,15 @@ CloneArchive(ArchiveHandle *AH)
47294714
clone->public.n_errors=0;
47304715

47314716
/*
4732-
* Connect our new clone object to the database: In parallel restore the
4733-
* parent is already disconnected, because we can connect the worker
4734-
* processes independently to the database (no snapshot sync required). In
4735-
* parallel backup we clone the parent's existing connection.
4717+
* Connect our new clone object to the database, using the same connection
4718+
* parameters used for the original connection.
47364719
*/
4737-
if (AH->mode==archModeRead)
4738-
{
4739-
RestoreOptions*ropt=AH->public.ropt;
4740-
4741-
Assert(AH->connection==NULL);
4742-
4743-
/* this also sets clone->connection */
4744-
ConnectDatabase((Archive*)clone,ropt->dbname,
4745-
ropt->pghost,ropt->pgport,ropt->username,
4746-
ropt->promptPassword);
4720+
ConnectDatabase((Archive*)clone,&clone->public.ropt->cparams, true);
47474721

4748-
/* re-establish fixed state */
4722+
/* re-establish fixed state */
4723+
if (AH->mode==archModeRead)
47494724
_doSetFixedOutputState(clone);
4750-
}
4751-
else
4752-
{
4753-
PQExpBufferDataconnstr;
4754-
char*pghost;
4755-
char*pgport;
4756-
char*username;
4757-
4758-
Assert(AH->connection!=NULL);
4759-
4760-
/*
4761-
* Even though we are technically accessing the parent's database
4762-
* object here, these functions are fine to be called like that
4763-
* because all just return a pointer and do not actually send/receive
4764-
* any data to/from the database.
4765-
*/
4766-
initPQExpBuffer(&connstr);
4767-
appendPQExpBuffer(&connstr,"dbname=");
4768-
appendConnStrVal(&connstr,PQdb(AH->connection));
4769-
pghost=PQhost(AH->connection);
4770-
pgport=PQport(AH->connection);
4771-
username=PQuser(AH->connection);
4772-
4773-
/* this also sets clone->connection */
4774-
ConnectDatabase((Archive*)clone,connstr.data,
4775-
pghost,pgport,username,TRI_NO);
4776-
4777-
termPQExpBuffer(&connstr);
4778-
/* setupDumpWorker will fix up connection state */
4779-
}
4725+
/* in write case, setupDumpWorker will fix up connection state */
47804726

47814727
/* Let the format-specific code have a chance too */
47824728
(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
@@ -308,7 +308,6 @@ struct _archiveHandle
308308

309309
/* Stuff for direct DB connection */
310310
char*archdbname;/* DB name *read* from archive */
311-
trivaluepromptPassword;
312311
char*savedPassword;/* password for ropt->username, if known */
313312
char*use_role;
314313
PGconn*connection;
@@ -454,7 +453,7 @@ extern void InitArchiveFmt_Tar(ArchiveHandle *AH);
454453

455454
externboolisValidTarHeader(char*header);
456455

457-
externintReconnectToServer(ArchiveHandle*AH,constchar*dbname,constchar*newUser);
456+
externvoidReconnectToServer(ArchiveHandle*AH,constchar*dbname);
458457
externvoidDropBlobIfExists(ArchiveHandle*AH,Oidoid);
459458

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

0 commit comments

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