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

Commitfb93f78

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 parent7664cc8 commitfb93f78

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

@@ -248,12 +256,9 @@ typedef void (*SetupWorkerPtrType) (Archive *AH);
248256
* Main archiver interface.
249257
*/
250258

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

‎src/bin/pg_dump/pg_backup_archiver.c

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

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

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

414-
ConnectDatabase(AHX,ropt->dbname,
415-
ropt->pghost,ropt->pgport,ropt->username,
416-
ropt->promptPassword);
420+
ConnectDatabase(AHX,&ropt->cparams, false);
417421

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

@@ -974,7 +970,7 @@ NewRestoreOptions(void)
974970

975971
/* set any fields that shouldn't default to zeroes */
976972
opts->format=archUnknown;
977-
opts->promptPassword=TRI_DEFAULT;
973+
opts->cparams.promptPassword=TRI_DEFAULT;
978974
opts->dumpSections=DUMP_UNSECTIONED;
979975

980976
returnopts;
@@ -2361,8 +2357,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
23612357
else
23622358
AH->format=fmt;
23632359

2364-
AH->promptPassword=TRI_DEFAULT;
2365-
23662360
switch (AH->format)
23672361
{
23682362
casearchCustom:
@@ -3223,27 +3217,20 @@ _doSetSessionAuth(ArchiveHandle *AH, const char *user)
32233217
* If we're currently restoring right into a database, this will
32243218
* actually establish a connection. Otherwise it puts a \connect into
32253219
* the script output.
3226-
*
3227-
* NULL dbname implies reconnecting to the current DB (pretty useless).
32283220
*/
32293221
staticvoid
32303222
_reconnectToDB(ArchiveHandle*AH,constchar*dbname)
32313223
{
32323224
if (RestoringToDB(AH))
3233-
ReconnectToServer(AH,dbname,NULL);
3225+
ReconnectToServer(AH,dbname);
32343226
else
32353227
{
3236-
if (dbname)
3237-
{
3238-
PQExpBufferDataconnectbuf;
3228+
PQExpBufferDataconnectbuf;
32393229

3240-
initPQExpBuffer(&connectbuf);
3241-
appendPsqlMetaConnect(&connectbuf,dbname);
3242-
ahprintf(AH,"%s\n",connectbuf.data);
3243-
termPQExpBuffer(&connectbuf);
3244-
}
3245-
else
3246-
ahprintf(AH,"%s\n","\\connect -\n");
3230+
initPQExpBuffer(&connectbuf);
3231+
appendPsqlMetaConnect(&connectbuf,dbname);
3232+
ahprintf(AH,"%s\n",connectbuf.data);
3233+
termPQExpBuffer(&connectbuf);
32473234
}
32483235

32493236
/*
@@ -4184,9 +4171,7 @@ restore_toc_entries_postfork(ArchiveHandle *AH, TocEntry *pending_list)
41844171
/*
41854172
* Now reconnect the single parent connection.
41864173
*/
4187-
ConnectDatabase((Archive*)AH,ropt->dbname,
4188-
ropt->pghost,ropt->pgport,ropt->username,
4189-
ropt->promptPassword);
4174+
ConnectDatabase((Archive*)AH,&ropt->cparams, true);
41904175

41914176
/* re-establish fixed state */
41924177
_doSetFixedOutputState(AH);
@@ -4848,54 +4833,15 @@ CloneArchive(ArchiveHandle *AH)
48484833
clone->public.n_errors=0;
48494834

48504835
/*
4851-
* Connect our new clone object to the database: In parallel restore the
4852-
* parent is already disconnected, because we can connect the worker
4853-
* processes independently to the database (no snapshot sync required). In
4854-
* parallel backup we clone the parent's existing connection.
4836+
* Connect our new clone object to the database, using the same connection
4837+
* parameters used for the original connection.
48554838
*/
4856-
if (AH->mode==archModeRead)
4857-
{
4858-
RestoreOptions*ropt=AH->public.ropt;
4859-
4860-
Assert(AH->connection==NULL);
4861-
4862-
/* this also sets clone->connection */
4863-
ConnectDatabase((Archive*)clone,ropt->dbname,
4864-
ropt->pghost,ropt->pgport,ropt->username,
4865-
ropt->promptPassword);
4839+
ConnectDatabase((Archive*)clone,&clone->public.ropt->cparams, true);
48664840

4867-
/* re-establish fixed state */
4841+
/* re-establish fixed state */
4842+
if (AH->mode==archModeRead)
48684843
_doSetFixedOutputState(clone);
4869-
}
4870-
else
4871-
{
4872-
PQExpBufferDataconnstr;
4873-
char*pghost;
4874-
char*pgport;
4875-
char*username;
4876-
4877-
Assert(AH->connection!=NULL);
4878-
4879-
/*
4880-
* Even though we are technically accessing the parent's database
4881-
* object here, these functions are fine to be called like that
4882-
* because all just return a pointer and do not actually send/receive
4883-
* any data to/from the database.
4884-
*/
4885-
initPQExpBuffer(&connstr);
4886-
appendPQExpBuffer(&connstr,"dbname=");
4887-
appendConnStrVal(&connstr,PQdb(AH->connection));
4888-
pghost=PQhost(AH->connection);
4889-
pgport=PQport(AH->connection);
4890-
username=PQuser(AH->connection);
4891-
4892-
/* this also sets clone->connection */
4893-
ConnectDatabase((Archive*)clone,connstr.data,
4894-
pghost,pgport,username,TRI_NO);
4895-
4896-
termPQExpBuffer(&connstr);
4897-
/* setupDumpWorker will fix up connection state */
4898-
}
4844+
/* in write case, setupDumpWorker will fix up connection state */
48994845

49004846
/* Let the format-specific code have a chance too */
49014847
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
@@ -305,7 +305,6 @@ struct _archiveHandle
305305

306306
/* Stuff for direct DB connection */
307307
char*archdbname;/* DB name *read* from archive */
308-
trivaluepromptPassword;
309308
char*savedPassword;/* password for ropt->username, if known */
310309
char*use_role;
311310
PGconn*connection;
@@ -475,7 +474,7 @@ extern void InitArchiveFmt_Tar(ArchiveHandle *AH);
475474

476475
externboolisValidTarHeader(char*header);
477476

478-
externvoidReconnectToServer(ArchiveHandle*AH,constchar*dbname,constchar*newUser);
477+
externvoidReconnectToServer(ArchiveHandle*AH,constchar*dbname);
479478
externvoidDropBlobIfExists(ArchiveHandle*AH,Oidoid);
480479

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

0 commit comments

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