Обсуждение: BUG #16604: pg_dump with --jobs breaks SSL connections
The following bug has been logged on the website:
Bug reference: 16604
Logged by: Zsolt Ero
Email address: zsolt.ero@gmail.com
PostgreSQL version: 12.4
Operating system: Ubuntu 20.04
Description:
I'm using pg_dump in the following syntax:
pg_dump --dbname="sslmode=verify-ca sslrootcert=server-ca.pem \
sslcert=client-cert.pem sslkey=client-key.pem \
hostaddr=1.2.3.4 \
user=postgres dbname=app" \
--format=directory \
--file=dump_app \
--jobs=3
As long as the --jobs parameter is present, the process breaks after
"pg_dump: saving database definition".
It breaks with "FATAL: connection requires a valid client certificate".
Without --jobs it completes without errors. I also think it's happening with
pg_restore as well.
Client is postgresql-client-12 under Ubuntu 20.04 from latest official
packages (http://apt.postgresql.org/pub/repos/apt).
> On 1 Sep 2020, at 22:08, PG Bug reporting form <noreply@postgresql.org> wrote: > > The following bug has been logged on the website: > > Bug reference: 16604 > Logged by: Zsolt Ero > Email address: zsolt.ero@gmail.com > PostgreSQL version: 12.4 > Operating system: Ubuntu 20.04 > Description: > > I'm using pg_dump in the following syntax: > > pg_dump --dbname="sslmode=verify-ca sslrootcert=server-ca.pem \ > sslcert=client-cert.pem sslkey=client-key.pem \ > hostaddr=1.2.3.4 \ > user=postgres dbname=app" \ > --format=directory \ > --file=dump_app \ > --jobs=3 > > As long as the --jobs parameter is present, the process breaks after > "pg_dump: saving database definition". > It breaks with "FATAL: connection requires a valid client certificate". > > Without --jobs it completes without errors. I also think it's happening with > pg_restore as well. I am unable to reproduce this with 12.4 as well as 14devel. If this error is reproducible for you, can you please share more details on the setup? (ideally a recipe for setting up a repro environment) cheers ./daniel
On Tue, Sep 15, 2020 at 1:36 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> On 1 Sep 2020, at 22:08, PG Bug reporting form <noreply@postgresql.org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 16604
> Logged by: Zsolt Ero
> Email address: zsolt.ero@gmail.com
> PostgreSQL version: 12.4
> Operating system: Ubuntu 20.04
> Description:
>
> I'm using pg_dump in the following syntax:
>
> pg_dump --dbname="sslmode=verify-ca sslrootcert=server-ca.pem \
> sslcert=client-cert.pem sslkey=client-key.pem \
> hostaddr=1.2.3.4 \
> user=postgres dbname=app" \
> --format=directory \
> --file=dump_app \
> --jobs=3
>
> As long as the --jobs parameter is present, the process breaks after
> "pg_dump: saving database definition".
> It breaks with "FATAL: connection requires a valid client certificate".
>
> Without --jobs it completes without errors. I also think it's happening with
> pg_restore as well.
I am unable to reproduce this with 12.4 as well as 14devel. If this error is
reproducible for you, can you please share more details on the setup? (ideally
a recipe for setting up a repro environment)
Grasping a long straw hwere, but I wonder if it could be relataed to the debian/ubuntu wrapper.
Zsolt, can you try running it with hardcoding the pg_dump path to /usr/lib/postgresql/12/bin/pg_dump instead of just pg_dump, to see if that might be it?
I've created a minimal reproducible Dockerfile, it reproduces 100%. The PG server is configured to require client certificates.
Dockerfile (repro with: "docker build .")
FROM ubuntu:18.04
RUN apt-get update && apt-get install -y wget gnupg
RUN echo "deb http://apt.postgresql.org/pub/repos/apt bionic-pgdg main" > /etc/apt/sources.list.d/pgdg.list
RUN wget --quiet -O - https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add -
RUN apt-get update && apt-get install -y postgresql-client-12
WORKDIR /tmp
COPY *.pem /tmp/
RUN chmod 400 /tmp/*.pem
ENV PGPASSWORD=xxx
RUN pg_dump --dbname="sslmode=verify-ca sslrootcert=server-ca.pem \
sslcert=client-cert.pem sslkey=client-key.pem \
hostaddr=1.2.3.4 \
port=5432 \
user=postgres dbname=postgres" \
--format=directory \
--file=dump_app \
--jobs=3
Zsolt
On 15 Sep 2020 at 14:04:09, Magnus Hagander <magnus@hagander.net> wrote:
On Tue, Sep 15, 2020 at 1:36 PM Daniel Gustafsson <daniel@yesql.se> wrote:> On 1 Sep 2020, at 22:08, PG Bug reporting form <noreply@postgresql.org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 16604
> Logged by: Zsolt Ero
> Email address: zsolt.ero@gmail.com
> PostgreSQL version: 12.4
> Operating system: Ubuntu 20.04
> Description:
>
> I'm using pg_dump in the following syntax:
>
> pg_dump --dbname="sslmode=verify-ca sslrootcert=server-ca.pem \
> sslcert=client-cert.pem sslkey=client-key.pem \
> hostaddr=1.2.3.4 \
> user=postgres dbname=app" \
> --format=directory \
> --file=dump_app \
> --jobs=3
>
> As long as the --jobs parameter is present, the process breaks after
> "pg_dump: saving database definition".
> It breaks with "FATAL: connection requires a valid client certificate".
>
> Without --jobs it completes without errors. I also think it's happening with
> pg_restore as well.
I am unable to reproduce this with 12.4 as well as 14devel. If this error is
reproducible for you, can you please share more details on the setup? (ideally
a recipe for setting up a repro environment)Grasping a long straw hwere, but I wonder if it could be relataed to the debian/ubuntu wrapper.Zsolt, can you try running it with hardcoding the pg_dump path to /usr/lib/postgresql/12/bin/pg_dump instead of just pg_dump, to see if that might be it?--
Zsolt Ero <zsolt.ero@gmail.com> writes:
> I've created a minimal reproducible Dockerfile, it reproduces 100%. The PG
> server is configured to require client certificates.
I reproduced this locally, and the problem seems to be that
CloneArchive() is doing a far-less-than-half-baked job of
reconstructing the original connection parameters.
The data that PQconnectdbParams gets is just
(gdb) p keywords
$1 = {0x44ef22 "host", 0x44d52d "port", 0x44e4f0 "user", 0x4532b6 "password",
0x44ef14 "dbname", 0x45325f "fallback_application_name", 0x0}
(gdb) p values
$2 = {0x25459e0 "127.0.0.1", 0x25459a0 "5432", 0x2545180 "postgres", 0x0,
0x2a61d60 "dbname=regression", 0x2538410 "pg_dump", 0x0}
so parallel pg_dump is basically guaranteed to fail in any
case with even slightly unusual connection parameters.
Not sure why we should be trying to do it like that at
all; it'd be better if the original command-line parameters
got passed down in all cases. Looking at that now.
regards, tom lane
I wrote:
> I reproduced this locally, and the problem seems to be that
> CloneArchive() is doing a far-less-than-half-baked job of
> reconstructing the original connection parameters. ...
> so parallel pg_dump is basically guaranteed to fail in any
> case with even slightly unusual connection parameters.
> Not sure why we should be trying to do it like that at
> all; it'd be better if the original command-line parameters
> got passed down in all cases. Looking at that now.
The attached patch seems to fix it, and also takes care of a nasty
habit that parallel pg_restore had of issuing repeated password
prompts if you're fool enough to specify -W. IMO it's only
sensible to apply that option on the first connection attempt.
Note, however, that AFAICS parallel pg_restore was okay on duplicating
the original connection parameters otherwise (the prompting issue
was because it went too far with that...). So I fail to confirm
the OP's claim that pg_restore had the same bug as parallel pg_dump.
If there really is an issue there, we'll need a test case that
demonstrates it.
regards, tom lane
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 3567e9f365..bbe1774a3e 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -178,6 +178,10 @@ dumpOptionsFromRestoreOptions(RestoreOptions *ropt)
DumpOptions *dopt = NewDumpOptions();
/* this is the inverse of what's at the end of pg_dump.c's main() */
+ dopt->dbname = ropt->dbname ? pg_strdup(ropt->dbname) : NULL;
+ dopt->pgport = ropt->pgport ? pg_strdup(ropt->pgport) : NULL;
+ dopt->pghost = ropt->pghost ? pg_strdup(ropt->pghost) : NULL;
+ dopt->username = ropt->username ? pg_strdup(ropt->username) : NULL;
dopt->outputClean = ropt->dropSchema;
dopt->dataOnly = ropt->dataOnly;
dopt->schemaOnly = ropt->schemaOnly;
@@ -4157,11 +4161,12 @@ restore_toc_entries_postfork(ArchiveHandle *AH, TocEntry *pending_list)
pg_log_debug("entering restore_toc_entries_postfork");
/*
- * Now reconnect the single parent connection.
+ * Now reconnect the single parent connection. Don't re-ask for a
+ * password.
*/
ConnectDatabase((Archive *) AH, ropt->dbname,
ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ TRI_NO);
/* re-establish fixed state */
_doSetFixedOutputState(AH);
@@ -4800,6 +4805,7 @@ ArchiveHandle *
CloneArchive(ArchiveHandle *AH)
{
ArchiveHandle *clone;
+ RestoreOptions *ropt = AH->public.ropt;
/* Make a "flat" copy */
clone = (ArchiveHandle *) pg_malloc(sizeof(ArchiveHandle));
@@ -4823,54 +4829,18 @@ CloneArchive(ArchiveHandle *AH)
clone->public.n_errors = 0;
/*
- * Connect our new clone object to the database: In parallel restore the
- * parent is already disconnected, because we can connect the worker
- * processes independently to the database (no snapshot sync required). In
- * parallel backup we clone the parent's existing connection.
+ * Connect our new clone object to the database, using the same connection
+ * parameters used for the original connection; except that workers should
+ * never re-ask for a password.
*/
- if (AH->mode == archModeRead)
- {
- RestoreOptions *ropt = AH->public.ropt;
-
- Assert(AH->connection == NULL);
-
- /* this also sets clone->connection */
- ConnectDatabase((Archive *) clone, ropt->dbname,
- ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ ConnectDatabase((Archive *) clone, ropt->dbname,
+ ropt->pghost, ropt->pgport, ropt->username,
+ TRI_NO);
- /* re-establish fixed state */
+ /* re-establish fixed state */
+ if (AH->mode == archModeRead)
_doSetFixedOutputState(clone);
- }
- else
- {
- PQExpBufferData connstr;
- char *pghost;
- char *pgport;
- char *username;
-
- Assert(AH->connection != NULL);
-
- /*
- * Even though we are technically accessing the parent's database
- * object here, these functions are fine to be called like that
- * because all just return a pointer and do not actually send/receive
- * any data to/from the database.
- */
- initPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, PQdb(AH->connection));
- pghost = PQhost(AH->connection);
- pgport = PQport(AH->connection);
- username = PQuser(AH->connection);
-
- /* this also sets clone->connection */
- ConnectDatabase((Archive *) clone, connstr.data,
- pghost, pgport, username, TRI_NO);
-
- termPQExpBuffer(&connstr);
- /* setupDumpWorker will fix up connection state */
- }
+ /* in write case, setupDumpWorker will fix up connection state */
/* Let the format-specific code have a chance too */
clone->ClonePtr(clone);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f021bb72f4..7676b93a10 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -918,6 +918,10 @@ main(int argc, char **argv)
ropt->filename = filename;
/* if you change this list, see dumpOptionsFromRestoreOptions */
+ ropt->dbname = dopt.dbname ? pg_strdup(dopt.dbname) : NULL;
+ ropt->pgport = dopt.pgport ? pg_strdup(dopt.pgport) : NULL;
+ ropt->pghost = dopt.pghost ? pg_strdup(dopt.pghost) : NULL;
+ ropt->username = dopt.username ? pg_strdup(dopt.username) : NULL;
ropt->dropSchema = dopt.outputClean;
ropt->dataOnly = dopt.dataOnly;
ropt->schemaOnly = dopt.schemaOnly;
I wrote:
> Note, however, that AFAICS parallel pg_restore was okay on duplicating
> the original connection parameters otherwise (the prompting issue
> was because it went too far with that...). So I fail to confirm
> the OP's claim that pg_restore had the same bug as parallel pg_dump.
> If there really is an issue there, we'll need a test case that
> demonstrates it.
Hmm ... so a bit later, I have a test case that may or may not be
what Zsolt saw:
pg_restore -C --dbname="hostaddr=127.0.0.1 sslmode=verify-ca dbname=postgres user=postgres sslcert=client-cert.crt
sslkey=client-cert.key"-Fd dumpdir
pg_restore: error: could not reconnect to database: FATAL: connection requires a valid client certificate
What is happening here is the exact same issue, but in
ReconnectToServer/_connectDB: those functions suppose that they
can blow away all the original connection parameters and insert
just a database name instead. I'm inclined to think that
we should get rid of _connectDB, because it seems unnecessarily
duplicative with ConnectDatabase. But we'll need a bit more
API refactoring than what I had. New patch coming ...
regards, tom lane
I wrote:
> I'm inclined to think that
> we should get rid of _connectDB, because it seems unnecessarily
> duplicative with ConnectDatabase. But we'll need a bit more
> API refactoring than what I had. New patch coming ...
A bit of refactoring later, I have the attached, which seems to
clean up all cases. I got rid of some bits of dead code along
the way.
regards, tom lane
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 1017abbbe5..a6a8e6f2fd 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -58,6 +58,20 @@ typedef enum _teSection
SECTION_POST_DATA /* stuff to be processed after data */
} teSection;
+/* Parameters needed by ConnectDatabase; same for dump and restore */
+typedef struct _connParams
+{
+ /* These fields record the actual command line parameters */
+ char *dbname; /* this may be a connstring! */
+ char *pgport;
+ char *pghost;
+ char *username;
+ trivalue promptPassword;
+ /* If not NULL, this overrides the dbname obtained from command line */
+ /* (but *only* the DB name, not anything else in the connstring) */
+ char *override_dbname;
+} ConnParams;
+
typedef struct _restoreOptions
{
int createDB; /* Issue commands to create the database */
@@ -107,12 +121,9 @@ typedef struct _restoreOptions
SimpleStringList tableNames;
int useDB;
- char *dbname; /* subject to expand_dbname */
- char *pgport;
- char *pghost;
- char *username;
+ ConnParams cparams; /* parameters to use if useDB */
+
int noDataForFailedTables;
- trivalue promptPassword;
int exit_on_error;
int compression;
int suppressDumpWarnings; /* Suppress output of WARNING entries
@@ -127,10 +138,7 @@ typedef struct _restoreOptions
typedef struct _dumpOptions
{
- const char *dbname; /* subject to expand_dbname */
- const char *pghost;
- const char *pgport;
- const char *username;
+ ConnParams cparams;
int binary_upgrade;
@@ -247,12 +255,9 @@ typedef void (*SetupWorkerPtrType) (Archive *AH);
* Main archiver interface.
*/
-extern void ConnectDatabase(Archive *AH,
- const char *dbname,
- const char *pghost,
- const char *pgport,
- const char *username,
- trivalue prompt_password);
+extern void ConnectDatabase(Archive *AHX,
+ const ConnParams *cparams,
+ bool isReconnect);
extern void DisconnectDatabase(Archive *AHX);
extern PGconn *GetConnection(Archive *AHX);
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 3567e9f365..d61b290d2a 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -165,6 +165,7 @@ InitDumpOptions(DumpOptions *opts)
memset(opts, 0, sizeof(DumpOptions));
/* set any fields that shouldn't default to zeroes */
opts->include_everything = true;
+ opts->cparams.promptPassword = TRI_DEFAULT;
opts->dumpSections = DUMP_UNSECTIONED;
}
@@ -178,6 +179,11 @@ dumpOptionsFromRestoreOptions(RestoreOptions *ropt)
DumpOptions *dopt = NewDumpOptions();
/* this is the inverse of what's at the end of pg_dump.c's main() */
+ dopt->cparams.dbname = ropt->cparams.dbname ? pg_strdup(ropt->cparams.dbname) : NULL;
+ dopt->cparams.pgport = ropt->cparams.pgport ? pg_strdup(ropt->cparams.pgport) : NULL;
+ dopt->cparams.pghost = ropt->cparams.pghost ? pg_strdup(ropt->cparams.pghost) : NULL;
+ dopt->cparams.username = ropt->cparams.username ? pg_strdup(ropt->cparams.username) : NULL;
+ dopt->cparams.promptPassword = ropt->cparams.promptPassword;
dopt->outputClean = ropt->dropSchema;
dopt->dataOnly = ropt->dataOnly;
dopt->schemaOnly = ropt->schemaOnly;
@@ -410,9 +416,7 @@ RestoreArchive(Archive *AHX)
AHX->minRemoteVersion = 0;
AHX->maxRemoteVersion = 9999999;
- ConnectDatabase(AHX, ropt->dbname,
- ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ ConnectDatabase(AHX, &ropt->cparams, false);
/*
* 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)
if (strcmp(te->desc, "DATABASE") == 0 ||
strcmp(te->desc, "DATABASE PROPERTIES") == 0)
{
- PQExpBufferData connstr;
-
- initPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, te->tag);
- /* Abandon struct, but keep its buffer until process exit. */
-
pg_log_info("connecting to new database \"%s\"", te->tag);
_reconnectToDB(AH, te->tag);
- ropt->dbname = connstr.data;
}
}
@@ -973,7 +969,7 @@ NewRestoreOptions(void)
/* set any fields that shouldn't default to zeroes */
opts->format = archUnknown;
- opts->promptPassword = TRI_DEFAULT;
+ opts->cparams.promptPassword = TRI_DEFAULT;
opts->dumpSections = DUMP_UNSECTIONED;
return opts;
@@ -2345,8 +2341,6 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
else
AH->format = fmt;
- AH->promptPassword = TRI_DEFAULT;
-
switch (AH->format)
{
case archCustom:
@@ -3207,27 +3201,20 @@ _doSetSessionAuth(ArchiveHandle *AH, const char *user)
* If we're currently restoring right into a database, this will
* actually establish a connection. Otherwise it puts a \connect into
* the script output.
- *
- * NULL dbname implies reconnecting to the current DB (pretty useless).
*/
static void
_reconnectToDB(ArchiveHandle *AH, const char *dbname)
{
if (RestoringToDB(AH))
- ReconnectToServer(AH, dbname, NULL);
+ ReconnectToServer(AH, dbname);
else
{
- if (dbname)
- {
- PQExpBufferData connectbuf;
+ PQExpBufferData connectbuf;
- initPQExpBuffer(&connectbuf);
- appendPsqlMetaConnect(&connectbuf, dbname);
- ahprintf(AH, "%s\n", connectbuf.data);
- termPQExpBuffer(&connectbuf);
- }
- else
- ahprintf(AH, "%s\n", "\\connect -\n");
+ initPQExpBuffer(&connectbuf);
+ appendPsqlMetaConnect(&connectbuf, dbname);
+ ahprintf(AH, "%s\n", connectbuf.data);
+ termPQExpBuffer(&connectbuf);
}
/*
@@ -4159,9 +4146,7 @@ restore_toc_entries_postfork(ArchiveHandle *AH, TocEntry *pending_list)
/*
* Now reconnect the single parent connection.
*/
- ConnectDatabase((Archive *) AH, ropt->dbname,
- ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ ConnectDatabase((Archive *) AH, &ropt->cparams, true);
/* re-establish fixed state */
_doSetFixedOutputState(AH);
@@ -4823,54 +4808,15 @@ CloneArchive(ArchiveHandle *AH)
clone->public.n_errors = 0;
/*
- * Connect our new clone object to the database: In parallel restore the
- * parent is already disconnected, because we can connect the worker
- * processes independently to the database (no snapshot sync required). In
- * parallel backup we clone the parent's existing connection.
+ * Connect our new clone object to the database, using the same connection
+ * parameters used for the original connection.
*/
- if (AH->mode == archModeRead)
- {
- RestoreOptions *ropt = AH->public.ropt;
-
- Assert(AH->connection == NULL);
-
- /* this also sets clone->connection */
- ConnectDatabase((Archive *) clone, ropt->dbname,
- ropt->pghost, ropt->pgport, ropt->username,
- ropt->promptPassword);
+ ConnectDatabase((Archive *) clone, &clone->public.ropt->cparams, true);
- /* re-establish fixed state */
+ /* re-establish fixed state */
+ if (AH->mode == archModeRead)
_doSetFixedOutputState(clone);
- }
- else
- {
- PQExpBufferData connstr;
- char *pghost;
- char *pgport;
- char *username;
-
- Assert(AH->connection != NULL);
-
- /*
- * Even though we are technically accessing the parent's database
- * object here, these functions are fine to be called like that
- * because all just return a pointer and do not actually send/receive
- * any data to/from the database.
- */
- initPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, PQdb(AH->connection));
- pghost = PQhost(AH->connection);
- pgport = PQport(AH->connection);
- username = PQuser(AH->connection);
-
- /* this also sets clone->connection */
- ConnectDatabase((Archive *) clone, connstr.data,
- pghost, pgport, username, TRI_NO);
-
- termPQExpBuffer(&connstr);
- /* setupDumpWorker will fix up connection state */
- }
+ /* in write case, setupDumpWorker will fix up connection state */
/* Let the format-specific code have a chance too */
clone->ClonePtr(clone);
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index f9e6b42752..fb8d226d48 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -303,7 +303,6 @@ struct _archiveHandle
/* Stuff for direct DB connection */
char *archdbname; /* DB name *read* from archive */
- trivalue promptPassword;
char *savedPassword; /* password for ropt->username, if known */
char *use_role;
PGconn *connection;
@@ -471,7 +470,7 @@ extern void InitArchiveFmt_Tar(ArchiveHandle *AH);
extern bool isValidTarHeader(char *header);
-extern void ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *newUser);
+extern void ReconnectToServer(ArchiveHandle *AH, const char *dbname);
extern void DropBlobIfExists(ArchiveHandle *AH, Oid oid);
void ahwrite(const void *ptr, size_t size, size_t nmemb, ArchiveHandle *AH);
diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c
index 12899e26e2..5ba43441f5 100644
--- a/src/bin/pg_dump/pg_backup_db.c
+++ b/src/bin/pg_dump/pg_backup_db.c
@@ -27,7 +27,6 @@
#include "pg_backup_utils.h"
static void _check_database_version(ArchiveHandle *AH);
-static PGconn *_connectDB(ArchiveHandle *AH, const char *newdbname, const char *newUser);
static void notice_processor(void *arg, const char *message);
static void
@@ -73,211 +72,100 @@ _check_database_version(ArchiveHandle *AH)
/*
* Reconnect to the server. If dbname is not NULL, use that database,
- * else the one associated with the archive handle. If username is
- * not NULL, use that user name, else the one from the handle.
+ * else the one associated with the archive handle.
*/
void
-ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
+ReconnectToServer(ArchiveHandle *AH, const char *dbname)
{
- PGconn *newConn;
- const char *newdbname;
- const char *newusername;
-
- if (!dbname)
- newdbname = PQdb(AH->connection);
- else
- newdbname = dbname;
-
- if (!username)
- newusername = PQuser(AH->connection);
- else
- newusername = username;
-
- newConn = _connectDB(AH, newdbname, newusername);
-
- /* Update ArchiveHandle's connCancel before closing old connection */
- set_archive_cancel_info(AH, newConn);
-
- PQfinish(AH->connection);
- AH->connection = newConn;
-
- /* Start strict; later phases may override this. */
- PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
- ALWAYS_SECURE_SEARCH_PATH_SQL));
-}
-
-/*
- * Connect to the db again.
- *
- * Note: it's not really all that sensible to use a single-entry password
- * cache if the username keeps changing. In current usage, however, the
- * username never does change, so one savedPassword is sufficient. We do
- * update the cache on the off chance that the password has changed since the
- * start of the run.
- */
-static PGconn *
-_connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
-{
- PQExpBufferData connstr;
- PGconn *newConn;
- const char *newdb;
- const char *newuser;
- char *password;
- bool new_pass;
-
- if (!reqdb)
- newdb = PQdb(AH->connection);
- else
- newdb = reqdb;
-
- if (!requser || strlen(requser) == 0)
- newuser = PQuser(AH->connection);
- else
- newuser = requser;
-
- pg_log_info("connecting to database \"%s\" as user \"%s\"",
- newdb, newuser);
-
- password = AH->savedPassword;
-
- if (AH->promptPassword == TRI_YES && password == NULL)
- password = simple_prompt("Password: ", false);
-
- initPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, newdb);
-
- do
- {
- const char *keywords[7];
- const char *values[7];
-
- keywords[0] = "host";
- values[0] = PQhost(AH->connection);
- keywords[1] = "port";
- values[1] = PQport(AH->connection);
- keywords[2] = "user";
- values[2] = newuser;
- keywords[3] = "password";
- values[3] = password;
- keywords[4] = "dbname";
- values[4] = connstr.data;
- keywords[5] = "fallback_application_name";
- values[5] = progname;
- keywords[6] = NULL;
- values[6] = NULL;
-
- new_pass = false;
- newConn = PQconnectdbParams(keywords, values, true);
-
- if (!newConn)
- fatal("could not reconnect to database");
-
- if (PQstatus(newConn) == CONNECTION_BAD)
- {
- if (!PQconnectionNeedsPassword(newConn))
- fatal("could not reconnect to database: %s",
- PQerrorMessage(newConn));
- PQfinish(newConn);
-
- if (password)
- fprintf(stderr, "Password incorrect\n");
-
- fprintf(stderr, "Connecting to %s as %s\n",
- newdb, newuser);
-
- if (AH->promptPassword != TRI_NO)
- {
- if (password && password != AH->savedPassword)
- free(password);
- password = simple_prompt("Password: ", false);
- }
- else
- fatal("connection needs password");
-
- new_pass = true;
- }
- } while (new_pass);
-
- if (password && password != AH->savedPassword)
- free(password);
+ PGconn *oldConn = AH->connection;
+ RestoreOptions *ropt = AH->public.ropt;
/*
- * We want to remember connection's actual password, whether or not we got
- * it by prompting. So we don't just store the password variable.
+ * Save the dbname, if given, in override_dbname so that it will also
+ * affect any later reconnection attempt.
*/
- if (PQconnectionUsedPassword(newConn))
- {
- if (AH->savedPassword)
- free(AH->savedPassword);
- AH->savedPassword = pg_strdup(PQpass(newConn));
- }
-
- termPQExpBuffer(&connstr);
+ if (dbname)
+ ropt->cparams.override_dbname = pg_strdup(dbname);
- /* check for version mismatch */
- _check_database_version(AH);
+ /*
+ * Note: we want to establish the new connection, and in particular update
+ * ArchiveHandle's connCancel, before closing old connection. Otherwise
+ * an ill-timed SIGINT could try to access a dead connection.
+ */
+ AH->connection = NULL; /* dodge error check in ConnectDatabase */
- PQsetNoticeProcessor(newConn, notice_processor, NULL);
+ ConnectDatabase((Archive *) AH, &ropt->cparams, true);
- return newConn;
+ PQfinish(oldConn);
}
-
/*
- * Make a database connection with the given parameters. The
- * connection handle is returned, the parameters are stored in AHX.
- * An interactive password prompt is automatically issued if required.
+ * Make, or remake, a database connection with the given parameters.
+ *
+ * The resulting connection handle is stored in AHX->connection.
*
+ * An interactive password prompt is automatically issued if required.
+ * We store the results of that in AHX->savedPassword.
* Note: it's not really all that sensible to use a single-entry password
* cache if the username keeps changing. In current usage, however, the
* username never does change, so one savedPassword is sufficient.
*/
void
ConnectDatabase(Archive *AHX,
- const char *dbname,
- const char *pghost,
- const char *pgport,
- const char *username,
- trivalue prompt_password)
+ const ConnParams *cparams,
+ bool isReconnect)
{
ArchiveHandle *AH = (ArchiveHandle *) AHX;
+ trivalue prompt_password;
char *password;
bool new_pass;
if (AH->connection)
fatal("already connected to a database");
+ /* Never prompt for a password during a reconnection */
+ prompt_password = isReconnect ? TRI_NO : cparams->promptPassword;
+
password = AH->savedPassword;
if (prompt_password == TRI_YES && password == NULL)
password = simple_prompt("Password: ", false);
- AH->promptPassword = prompt_password;
-
/*
* Start the connection. Loop until we have a password if requested by
* backend.
*/
do
{
- const char *keywords[7];
- const char *values[7];
-
- keywords[0] = "host";
- values[0] = pghost;
- keywords[1] = "port";
- values[1] = pgport;
- keywords[2] = "user";
- values[2] = username;
- keywords[3] = "password";
- values[3] = password;
- keywords[4] = "dbname";
- values[4] = dbname;
- keywords[5] = "fallback_application_name";
- values[5] = progname;
- keywords[6] = NULL;
- values[6] = NULL;
+ const char *keywords[8];
+ const char *values[8];
+ int i = 0;
+
+ /*
+ * If dbname is a connstring, its entries can override the other
+ * values obtained from cparams; but in turn, override_dbname can
+ * override the dbname component of it.
+ */
+ keywords[i] = "host";
+ values[i++] = cparams->pghost;
+ keywords[i] = "port";
+ values[i++] = cparams->pgport;
+ keywords[i] = "user";
+ values[i++] = cparams->username;
+ keywords[i] = "password";
+ values[i++] = password;
+ keywords[i] = "dbname";
+ values[i++] = cparams->dbname;
+ if (cparams->override_dbname)
+ {
+ keywords[i] = "dbname";
+ values[i++] = cparams->override_dbname;
+ }
+ keywords[i] = "fallback_application_name";
+ values[i++] = progname;
+ keywords[i] = NULL;
+ values[i++] = NULL;
+ Assert(i <= lengthof(keywords));
new_pass = false;
AH->connection = PQconnectdbParams(keywords, values, true);
@@ -298,9 +186,16 @@ ConnectDatabase(Archive *AHX,
/* check to see that the backend connection was successfully made */
if (PQstatus(AH->connection) == CONNECTION_BAD)
- fatal("connection to database \"%s\" failed: %s",
- PQdb(AH->connection) ? PQdb(AH->connection) : "",
- PQerrorMessage(AH->connection));
+ {
+ if (isReconnect)
+ fatal("reconnection to database \"%s\" failed: %s",
+ PQdb(AH->connection) ? PQdb(AH->connection) : "",
+ PQerrorMessage(AH->connection));
+ else
+ fatal("connection to database \"%s\" failed: %s",
+ PQdb(AH->connection) ? PQdb(AH->connection) : "",
+ PQerrorMessage(AH->connection));
+ }
/* Start strict; later phases may override this. */
PQclear(ExecuteSqlQueryForSingleRow((Archive *) AH,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f021bb72f4..76320468ba 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -314,7 +314,6 @@ main(int argc, char **argv)
char *use_role = NULL;
long rowsPerInsert;
int numWorkers = 1;
- trivalue prompt_password = TRI_DEFAULT;
int compressLevel = -1;
int plainText = 0;
ArchiveFormat archiveFormat = archUnknown;
@@ -444,7 +443,7 @@ main(int argc, char **argv)
break;
case 'd': /* database name */
- dopt.dbname = pg_strdup(optarg);
+ dopt.cparams.dbname = pg_strdup(optarg);
break;
case 'E': /* Dump encoding */
@@ -460,7 +459,7 @@ main(int argc, char **argv)
break;
case 'h': /* server host */
- dopt.pghost = pg_strdup(optarg);
+ dopt.cparams.pghost = pg_strdup(optarg);
break;
case 'j': /* number of dump jobs */
@@ -481,7 +480,7 @@ main(int argc, char **argv)
break;
case 'p': /* server port */
- dopt.pgport = pg_strdup(optarg);
+ dopt.cparams.pgport = pg_strdup(optarg);
break;
case 'R':
@@ -506,7 +505,7 @@ main(int argc, char **argv)
break;
case 'U':
- dopt.username = pg_strdup(optarg);
+ dopt.cparams.username = pg_strdup(optarg);
break;
case 'v': /* verbose */
@@ -515,11 +514,11 @@ main(int argc, char **argv)
break;
case 'w':
- prompt_password = TRI_NO;
+ dopt.cparams.promptPassword = TRI_NO;
break;
case 'W':
- prompt_password = TRI_YES;
+ dopt.cparams.promptPassword = TRI_YES;
break;
case 'x': /* skip ACL dump */
@@ -613,8 +612,8 @@ main(int argc, char **argv)
* Non-option argument specifies database name as long as it wasn't
* already specified with -d / --dbname
*/
- if (optind < argc && dopt.dbname == NULL)
- dopt.dbname = argv[optind++];
+ if (optind < argc && dopt.cparams.dbname == NULL)
+ dopt.cparams.dbname = argv[optind++];
/* Complain if any arguments remain */
if (optind < argc)
@@ -740,7 +739,7 @@ main(int argc, char **argv)
* Open the database using the Archiver, so it knows about it. Errors mean
* death.
*/
- ConnectDatabase(fout, dopt.dbname, dopt.pghost, dopt.pgport, dopt.username, prompt_password);
+ ConnectDatabase(fout, &dopt.cparams, false);
setup_connection(fout, dumpencoding, dumpsnapshot, use_role);
/*
@@ -918,6 +917,11 @@ main(int argc, char **argv)
ropt->filename = filename;
/* if you change this list, see dumpOptionsFromRestoreOptions */
+ ropt->cparams.dbname = dopt.cparams.dbname ? pg_strdup(dopt.cparams.dbname) : NULL;
+ ropt->cparams.pgport = dopt.cparams.pgport ? pg_strdup(dopt.cparams.pgport) : NULL;
+ ropt->cparams.pghost = dopt.cparams.pghost ? pg_strdup(dopt.cparams.pghost) : NULL;
+ ropt->cparams.username = dopt.cparams.username ? pg_strdup(dopt.cparams.username) : NULL;
+ ropt->cparams.promptPassword = dopt.cparams.promptPassword;
ropt->dropSchema = dopt.outputClean;
ropt->dataOnly = dopt.dataOnly;
ropt->schemaOnly = dopt.schemaOnly;
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index eebf0d300b..589b4aed53 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -163,7 +163,7 @@ main(int argc, char **argv)
opts->createDB = 1;
break;
case 'd':
- opts->dbname = pg_strdup(optarg);
+ opts->cparams.dbname = pg_strdup(optarg);
break;
case 'e':
opts->exit_on_error = true;
@@ -177,7 +177,7 @@ main(int argc, char **argv)
break;
case 'h':
if (strlen(optarg) != 0)
- opts->pghost = pg_strdup(optarg);
+ opts->cparams.pghost = pg_strdup(optarg);
break;
case 'j': /* number of restore jobs */
@@ -206,7 +206,7 @@ main(int argc, char **argv)
case 'p':
if (strlen(optarg) != 0)
- opts->pgport = pg_strdup(optarg);
+ opts->cparams.pgport = pg_strdup(optarg);
break;
case 'R':
/* no-op, still accepted for backwards compatibility */
@@ -240,7 +240,7 @@ main(int argc, char **argv)
break;
case 'U':
- opts->username = pg_strdup(optarg);
+ opts->cparams.username = pg_strdup(optarg);
break;
case 'v': /* verbose */
@@ -249,11 +249,11 @@ main(int argc, char **argv)
break;
case 'w':
- opts->promptPassword = TRI_NO;
+ opts->cparams.promptPassword = TRI_NO;
break;
case 'W':
- opts->promptPassword = TRI_YES;
+ opts->cparams.promptPassword = TRI_YES;
break;
case 'x': /* skip ACL dump */
@@ -303,14 +303,14 @@ main(int argc, char **argv)
}
/* Complain if neither -f nor -d was specified (except if dumping TOC) */
- if (!opts->dbname && !opts->filename && !opts->tocSummary)
+ if (!opts->cparams.dbname && !opts->filename && !opts->tocSummary)
{
pg_log_error("one of -d/--dbname and -f/--file must be specified");
exit_nicely(1);
}
/* Should get at most one of -d and -f, else user is confused */
- if (opts->dbname)
+ if (opts->cparams.dbname)
{
if (opts->filename)
{
I wrote:
> A bit of refactoring later, I have the attached, which seems to
> clean up all cases. I got rid of some bits of dead code along
> the way.
I pushed that at commit a45bc8a4f. However...
In some off-list discussion, Peter Eisentraut pointed out that
pg_dump/pg_restore are not the only programs with this problem.
Several of the src/bin/scripts/ programs, such as vacuumdb
and reindexdb, perform reconnections when told to use parallel
processing. These reconnections use methods similar to parallel
pg_restore and have the same bug. We can fix this straightforwardly
using much the same methods as in a45bc8a4f, as per 0001 below.
(0001 also fixes the documentation oversight that we didn't say that
these programs' --maintenance-db option could be a connstring.)
pg_dumpall, as well as "pg_dump -C" with text output, have a
variant of the issue: they emit psql scripts containing commands
such as "\connect databasename", and psql's \connect processing
isn't very good about re-using all connection options either.
In fact, it'll *only* re-use username, host name (and hostaddr if
given), and port. So if you do "psql -d '...some connection options'
and then try to run a dump script, the reconnections will fail
if there were other critical options in the -d connstring, much
like what was reported here. 0002 below is a draft attempt at
addressing this, but it's probably more controversial than 0001,
because there's a larger behavioral change involved.
An important thing to understand is that except for the \connect
case, all these programs perform a reconnection by re-using the
textual parameters given originally. Thus for example, if you do
"vacuumdb -j 4 -h host1,host2 ...", it's unclear which of host1 and
host2 will be connected to, either by the initial connection or any
of the parallel worker processes. I don't regard that as a bug
particularly. Perhaps some users would wish that once vacuumdb
made an initial connection, the workers would guarantee to reconnect
to that same host; but ISTM the real answer is "don't use that kind
of host spec for this purpose". (Note that there are other ways
to shoot yourself in the foot here, for example if your DNS can
resolve several different IPs for the same server host name. All
these cases seem more like pilot error than things for us to fix.)
\connect, however, is way out in left field. In the first place,
while these other programs are just interested in re-using all the
connection parameters except possibly the database name, \connect
intends to allow substitution of new values for some but not
necessarily all of the parameters. In the second place, \connect
makes some effort to force reconnection to occur to the very same
server if you don't specify a new host. Rather than re-using the
original parameter strings, it pulls out PQhost() and PQport()
values from the previous connection, and also PQhostaddr() if you
specified hostaddr previously. These values will reflect just the
server actually connected to, not the multi-host values you might
have given originally.
I'm inclined to think that this second behavior is simply wrong,
or at least a failure to adhere to the principle of least surprise.
An example of why it might be thought buggy is
postgres=# \connect "dbname=postgres host=sss1,sss1 hostaddr=127.0.0.1,::1"
You are now connected to database "postgres" as user "postgres".
postgres=# \connect -reuse-previous=on "dbname=postgres host=sss1,sss1"
could not match 2 host names to 1 hostaddr values
Previous connection kept
This happens because what -reuse-previous injects is not the previous
hostaddr connection parameter value ("127.0.0.1,::1"), but only the
actual connection address ("127.0.0.1"), resulting in a surprising
mismatch.
Moreover, psql isn't even being self-consistent by doing this.
If you get a connection loss, it will invoke PQreset() which just
re-uses the connection parameters previously given, and therefore
is perfectly content to connect to any of the hosts you named before.
It's quite unclear why \connect should be pickier than that case.
Therefore, my proposal in 0002 below is to drop all that logic.
The patch's method for re-using connection parameters is to fetch
the parameters of the old connection with PQconninfo(), which will
provide the textual values for each parameter slot, and then just
overwrite the specific slots that you provide new values for.
This seems to me to have considerably less surprise factor than
what happens now. But there's no denying that it changes behavior
in multi-host cases to a greater degree than is minimally necessary
to fix the bug complained of here.
There are some things adjacent to this that I'd kind of like to fix.
For example, I noted that createuser and dropuser lack any way to use
a connstring, as they have neither -d nor --maintenance-db options.
That seems pretty bogus, but it also seems like a missing feature not
a bug, so I didn't do anything about it here. (To be clear, I'm
proposing these changes for back-patch, as a45bc8a4f was.)
Comments?
regards, tom lane
diff --git a/doc/src/sgml/ref/clusterdb.sgml b/doc/src/sgml/ref/clusterdb.sgml
index c53bacf864..c838b22c44 100644
--- a/doc/src/sgml/ref/clusterdb.sgml
+++ b/doc/src/sgml/ref/clusterdb.sgml
@@ -90,9 +90,9 @@ PostgreSQL documentation
<term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
<listitem>
<para>
- Specifies the name of the database to be clustered.
- If this is not specified and <option>-a</option> (or
- <option>--all</option>) is not used, the database name is read
+ Specifies the name of the database to be clustered,
+ when <option>-a</option>/<option>--all</option> is not used.
+ If this is not specified, the database name is read
from the environment variable <envar>PGDATABASE</envar>. If
that is not set, the user name specified for the connection is
used. The <replaceable>dbname</replaceable> can be a <link
@@ -249,10 +249,16 @@ PostgreSQL documentation
<term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></option></term>
<listitem>
<para>
- Specifies the name of the database to connect to discover what other
- databases should be clustered. If not specified, the
- <literal>postgres</literal> database will be used,
- and if that does not exist, <literal>template1</literal> will be used.
+ Specifies the name of the database to connect to to discover which
+ databases should be clustered,
+ when <option>-a</option>/<option>--all</option> is used.
+ If not specified, the <literal>postgres</literal> database will be used,
+ or if that does not exist, <literal>template1</literal> will be used.
+ This can be a <link linkend="libpq-connstring">connection
+ string</link>. If so, connection string parameters will override any
+ conflicting command line options. Also, connection string parameters
+ other than the database name itself will be re-used when connecting
+ to other databases.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/createdb.sgml b/doc/src/sgml/ref/createdb.sgml
index 95cc82dc88..86473455c9 100644
--- a/doc/src/sgml/ref/createdb.sgml
+++ b/doc/src/sgml/ref/createdb.sgml
@@ -284,6 +284,9 @@ PostgreSQL documentation
database will be used; if that does not exist (or if it is the name
of the new database being created), <literal>template1</literal> will
be used.
+ This can be a <link linkend="libpq-connstring">connection
+ string</link>. If so, connection string parameters will override any
+ conflicting command line options.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/dropdb.sgml b/doc/src/sgml/ref/dropdb.sgml
index fe523a2ee1..d36aed38c5 100644
--- a/doc/src/sgml/ref/dropdb.sgml
+++ b/doc/src/sgml/ref/dropdb.sgml
@@ -217,6 +217,9 @@ PostgreSQL documentation
target database. If not specified, the <literal>postgres</literal>
database will be used; if that does not exist (or is the database
being dropped), <literal>template1</literal> will be used.
+ This can be a <link linkend="libpq-connstring">connection
+ string</link>. If so, connection string parameters will override any
+ conflicting command line options.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/reindexdb.sgml b/doc/src/sgml/ref/reindexdb.sgml
index a6d93693c5..5741445333 100644
--- a/doc/src/sgml/ref/reindexdb.sgml
+++ b/doc/src/sgml/ref/reindexdb.sgml
@@ -134,9 +134,9 @@ PostgreSQL documentation
<term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
<listitem>
<para>
- Specifies the name of the database to be reindexed.
- If this is not specified and <option>-a</option> (or
- <option>--all</option>) is not used, the database name is read
+ Specifies the name of the database to be reindexed,
+ when <option>-a</option>/<option>--all</option> is not used.
+ If this is not specified, the database name is read
from the environment variable <envar>PGDATABASE</envar>. If
that is not set, the user name specified for the connection is
used. The <replaceable>dbname</replaceable> can be a <link
@@ -351,10 +351,16 @@ PostgreSQL documentation
<term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></option></term>
<listitem>
<para>
- Specifies the name of the database to connect to discover what other
- databases should be reindexed. If not specified, the
- <literal>postgres</literal> database will be used,
- and if that does not exist, <literal>template1</literal> will be used.
+ Specifies the name of the database to connect to to discover which
+ databases should be reindexed,
+ when <option>-a</option>/<option>--all</option> is used.
+ If not specified, the <literal>postgres</literal> database will be used,
+ or if that does not exist, <literal>template1</literal> will be used.
+ This can be a <link linkend="libpq-connstring">connection
+ string</link>. If so, connection string parameters will override any
+ conflicting command line options. Also, connection string parameters
+ other than the database name itself will be re-used when connecting
+ to other databases.
</para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 6dcdab9caf..a90fc9322f 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -92,9 +92,9 @@ PostgreSQL documentation
<term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
<listitem>
<para>
- Specifies the name of the database to be cleaned or analyzed.
- If this is not specified and <option>-a</option> (or
- <option>--all</option>) is not used, the database name is read
+ Specifies the name of the database to be cleaned or analyzed,
+ when <option>-a</option>/<option>--all</option> is not used.
+ If this is not specified, the database name is read
from the environment variable <envar>PGDATABASE</envar>. If
that is not set, the user name specified for the connection is
used. The <replaceable>dbname</replaceable> can be a <link
@@ -474,10 +474,16 @@ PostgreSQL documentation
<term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></option></term>
<listitem>
<para>
- Specifies the name of the database to connect to discover what other
- databases should be vacuumed. If not specified, the
- <literal>postgres</literal> database will be used,
- and if that does not exist, <literal>template1</literal> will be used.
+ Specifies the name of the database to connect to to discover which
+ databases should be vacuumed,
+ when <option>-a</option>/<option>--all</option> is used.
+ If not specified, the <literal>postgres</literal> database will be used,
+ or if that does not exist, <literal>template1</literal> will be used.
+ This can be a <link linkend="libpq-connstring">connection
+ string</link>. If so, connection string parameters will override any
+ conflicting command line options. Also, connection string parameters
+ other than the database name itself will be re-used when connecting
+ to other databases.
</para>
</listitem>
</varlistentry>
diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index 12972de0e9..2f786e6103 100644
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -17,15 +17,10 @@
#include "fe_utils/string_utils.h"
-static void cluster_one_database(const char *dbname, bool verbose, const char *table,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
- const char *progname, bool echo);
-static void cluster_all_databases(bool verbose, const char *maintenance_db,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
- const char *progname, bool echo, bool quiet);
-
+static void cluster_one_database(const ConnParams *cparams, const char *table,
+ const char *progname, bool verbose, bool echo);
+static void cluster_all_databases(ConnParams *cparams, const char *progname,
+ bool verbose, bool echo, bool quiet);
static void help(const char *progname);
@@ -58,6 +53,7 @@ main(int argc, char *argv[])
char *port = NULL;
char *username = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
+ ConnParams cparams;
bool echo = false;
bool quiet = false;
bool alldb = false;
@@ -134,6 +130,13 @@ main(int argc, char *argv[])
exit(1);
}
+ /* fill cparams except for dbname, which is set below */
+ cparams.pghost = host;
+ cparams.pgport = port;
+ cparams.pguser = username;
+ cparams.prompt_password = prompt_password;
+ cparams.override_dbname = NULL;
+
setup_cancel_handler(NULL);
if (alldb)
@@ -150,8 +153,9 @@ main(int argc, char *argv[])
exit(1);
}
- cluster_all_databases(verbose, maintenance_db, host, port, username, prompt_password,
- progname, echo, quiet);
+ cparams.dbname = maintenance_db;
+
+ cluster_all_databases(&cparams, progname, verbose, echo, quiet);
}
else
{
@@ -165,21 +169,21 @@ main(int argc, char *argv[])
dbname = get_user_name_or_exit(progname);
}
+ cparams.dbname = dbname;
+
if (tables.head != NULL)
{
SimpleStringListCell *cell;
for (cell = tables.head; cell; cell = cell->next)
{
- cluster_one_database(dbname, verbose, cell->val,
- host, port, username, prompt_password,
- progname, echo);
+ cluster_one_database(&cparams, cell->val,
+ progname, verbose, echo);
}
}
else
- cluster_one_database(dbname, verbose, NULL,
- host, port, username, prompt_password,
- progname, echo);
+ cluster_one_database(&cparams, NULL,
+ progname, verbose, echo);
}
exit(0);
@@ -187,17 +191,14 @@ main(int argc, char *argv[])
static void
-cluster_one_database(const char *dbname, bool verbose, const char *table,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
- const char *progname, bool echo)
+cluster_one_database(const ConnParams *cparams, const char *table,
+ const char *progname, bool verbose, bool echo)
{
PQExpBufferData sql;
PGconn *conn;
- conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, echo, false, false);
+ conn = connectDatabase(cparams, progname, echo, false, false);
initPQExpBuffer(&sql);
@@ -228,22 +229,17 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
static void
-cluster_all_databases(bool verbose, const char *maintenance_db,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
- const char *progname, bool echo, bool quiet)
+cluster_all_databases(ConnParams *cparams, const char *progname,
+ bool verbose, bool echo, bool quiet)
{
PGconn *conn;
PGresult *result;
- PQExpBufferData connstr;
int i;
- conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname, echo);
+ conn = connectMaintenanceDatabase(cparams, progname, echo);
result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", echo);
PQfinish(conn);
- initPQExpBuffer(&connstr);
for (i = 0; i < PQntuples(result); i++)
{
char *dbname = PQgetvalue(result, i, 0);
@@ -254,15 +250,10 @@ cluster_all_databases(bool verbose, const char *maintenance_db,
fflush(stdout);
}
- resetPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, dbname);
+ cparams->override_dbname = dbname;
- cluster_one_database(connstr.data, verbose, NULL,
- host, port, username, prompt_password,
- progname, echo);
+ cluster_one_database(cparams, NULL, progname, verbose, echo);
}
- termPQExpBuffer(&connstr);
PQclear(result);
}
diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index e987eef234..3362221a31 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -54,7 +54,7 @@ handle_help_version_opts(int argc, char *argv[],
* Make a database connection with the given parameters.
*
* An interactive password prompt is automatically issued if needed and
- * allowed by prompt_password.
+ * allowed by cparams->prompt_password.
*
* If allow_password_reuse is true, we will try to re-use any password
* given during previous calls to this routine. (Callers should not pass
@@ -62,22 +62,23 @@ handle_help_version_opts(int argc, char *argv[],
* as before, else we might create password exposure hazards.)
*/
PGconn *
-connectDatabase(const char *dbname, const char *pghost,
- const char *pgport, const char *pguser,
- enum trivalue prompt_password, const char *progname,
+connectDatabase(const ConnParams *cparams, const char *progname,
bool echo, bool fail_ok, bool allow_password_reuse)
{
PGconn *conn;
bool new_pass;
static char *password = NULL;
+ /* Callers must supply at least dbname; other params can be NULL */
+ Assert(cparams->dbname);
+
if (!allow_password_reuse && password)
{
free(password);
password = NULL;
}
- if (!password && prompt_password == TRI_YES)
+ if (cparams->prompt_password == TRI_YES && password == NULL)
password = simple_prompt("Password: ", false);
/*
@@ -86,23 +87,35 @@ connectDatabase(const char *dbname, const char *pghost,
*/
do
{
- const char *keywords[7];
- const char *values[7];
-
- keywords[0] = "host";
- values[0] = pghost;
- keywords[1] = "port";
- values[1] = pgport;
- keywords[2] = "user";
- values[2] = pguser;
- keywords[3] = "password";
- values[3] = password;
- keywords[4] = "dbname";
- values[4] = dbname;
- keywords[5] = "fallback_application_name";
- values[5] = progname;
- keywords[6] = NULL;
- values[6] = NULL;
+ const char *keywords[8];
+ const char *values[8];
+ int i = 0;
+
+ /*
+ * If dbname is a connstring, its entries can override the other
+ * values obtained from cparams; but in turn, override_dbname can
+ * override the dbname component of it.
+ */
+ keywords[i] = "host";
+ values[i++] = cparams->pghost;
+ keywords[i] = "port";
+ values[i++] = cparams->pgport;
+ keywords[i] = "user";
+ values[i++] = cparams->pguser;
+ keywords[i] = "password";
+ values[i++] = password;
+ keywords[i] = "dbname";
+ values[i++] = cparams->dbname;
+ if (cparams->override_dbname)
+ {
+ keywords[i] = "dbname";
+ values[i++] = cparams->override_dbname;
+ }
+ keywords[i] = "fallback_application_name";
+ values[i++] = progname;
+ keywords[i] = NULL;
+ values[i++] = NULL;
+ Assert(i <= lengthof(keywords));
new_pass = false;
conn = PQconnectdbParams(keywords, values, true);
@@ -110,7 +123,7 @@ connectDatabase(const char *dbname, const char *pghost,
if (!conn)
{
pg_log_error("could not connect to database %s: out of memory",
- dbname);
+ cparams->dbname);
exit(1);
}
@@ -119,7 +132,7 @@ connectDatabase(const char *dbname, const char *pghost,
*/
if (PQstatus(conn) == CONNECTION_BAD &&
PQconnectionNeedsPassword(conn) &&
- prompt_password != TRI_NO)
+ cparams->prompt_password != TRI_NO)
{
PQfinish(conn);
if (password)
@@ -138,10 +151,11 @@ connectDatabase(const char *dbname, const char *pghost,
return NULL;
}
pg_log_error("could not connect to database %s: %s",
- dbname, PQerrorMessage(conn));
+ cparams->dbname, PQerrorMessage(conn));
exit(1);
}
+ /* Start strict; callers may override this. */
PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL, echo));
return conn;
@@ -149,27 +163,30 @@ connectDatabase(const char *dbname, const char *pghost,
/*
* Try to connect to the appropriate maintenance database.
+ *
+ * This differs from connectDatabase only in that it has a rule for
+ * inserting a default "dbname" if none was given (which is why cparams
+ * is not const). Note that cparams->dbname should typically come from
+ * a --maintenance-db command line parameter.
*/
PGconn *
-connectMaintenanceDatabase(const char *maintenance_db,
- const char *pghost, const char *pgport,
- const char *pguser, enum trivalue prompt_password,
+connectMaintenanceDatabase(ConnParams *cparams,
const char *progname, bool echo)
{
PGconn *conn;
/* If a maintenance database name was specified, just connect to it. */
- if (maintenance_db)
- return connectDatabase(maintenance_db, pghost, pgport, pguser,
- prompt_password, progname, echo, false, false);
+ if (cparams->dbname)
+ return connectDatabase(cparams, progname, echo, false, false);
/* Otherwise, try postgres first and then template1. */
- conn = connectDatabase("postgres", pghost, pgport, pguser, prompt_password,
- progname, echo, true, false);
+ cparams->dbname = "postgres";
+ conn = connectDatabase(cparams, progname, echo, true, false);
if (!conn)
- conn = connectDatabase("template1", pghost, pgport, pguser,
- prompt_password, progname, echo, false, false);
-
+ {
+ cparams->dbname = "template1";
+ conn = connectDatabase(cparams, progname, echo, false, false);
+ }
return conn;
}
diff --git a/src/bin/scripts/common.h b/src/bin/scripts/common.h
index ddf6320b47..9ec57cdd87 100644
--- a/src/bin/scripts/common.h
+++ b/src/bin/scripts/common.h
@@ -21,20 +21,32 @@ enum trivalue
TRI_YES
};
+/* Parameters needed by connectDatabase/connectMaintenanceDatabase */
+typedef struct _connParams
+{
+ /* These fields record the actual command line parameters */
+ const char *dbname; /* this may be a connstring! */
+ const char *pghost;
+ const char *pgport;
+ const char *pguser;
+ enum trivalue prompt_password;
+ /* If not NULL, this overrides the dbname obtained from command line */
+ /* (but *only* the DB name, not anything else in the connstring) */
+ const char *override_dbname;
+} ConnParams;
+
typedef void (*help_handler) (const char *progname);
extern void handle_help_version_opts(int argc, char *argv[],
const char *fixed_progname,
help_handler hlp);
-extern PGconn *connectDatabase(const char *dbname, const char *pghost,
- const char *pgport, const char *pguser,
- enum trivalue prompt_password, const char *progname,
- bool echo, bool fail_ok, bool allow_password_reuse);
+extern PGconn *connectDatabase(const ConnParams *cparams,
+ const char *progname,
+ bool echo, bool fail_ok,
+ bool allow_password_reuse);
-extern PGconn *connectMaintenanceDatabase(const char *maintenance_db,
- const char *pghost, const char *pgport,
- const char *pguser, enum trivalue prompt_password,
+extern PGconn *connectMaintenanceDatabase(ConnParams *cparams,
const char *progname, bool echo);
extern void disconnectDatabase(PGconn *conn);
diff --git a/src/bin/scripts/createdb.c b/src/bin/scripts/createdb.c
index 1353af97c4..91e6e2194b 100644
--- a/src/bin/scripts/createdb.c
+++ b/src/bin/scripts/createdb.c
@@ -51,6 +51,7 @@ main(int argc, char *argv[])
char *port = NULL;
char *username = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
+ ConnParams cparams;
bool echo = false;
char *owner = NULL;
char *tablespace = NULL;
@@ -180,8 +181,14 @@ main(int argc, char *argv[])
if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
maintenance_db = "template1";
- conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname, echo);
+ cparams.dbname = maintenance_db;
+ cparams.pghost = host;
+ cparams.pgport = port;
+ cparams.pguser = username;
+ cparams.prompt_password = prompt_password;
+ cparams.override_dbname = NULL;
+
+ conn = connectMaintenanceDatabase(&cparams, progname, echo);
initPQExpBuffer(&sql);
diff --git a/src/bin/scripts/createuser.c b/src/bin/scripts/createuser.c
index 6179199563..d6b56f15c3 100644
--- a/src/bin/scripts/createuser.c
+++ b/src/bin/scripts/createuser.c
@@ -59,6 +59,7 @@ main(int argc, char *argv[])
char *username = NULL;
SimpleStringList roles = {NULL, NULL};
enum trivalue prompt_password = TRI_DEFAULT;
+ ConnParams cparams;
bool echo = false;
bool interactive = false;
int conn_limit = -2; /* less than minimum valid value */
@@ -252,8 +253,14 @@ main(int argc, char *argv[])
if (login == 0)
login = TRI_YES;
- conn = connectDatabase("postgres", host, port, username, prompt_password,
- progname, echo, false, false);
+ cparams.dbname = NULL; /* this program lacks any dbname option... */
+ cparams.pghost = host;
+ cparams.pgport = port;
+ cparams.pguser = username;
+ cparams.prompt_password = prompt_password;
+ cparams.override_dbname = NULL;
+
+ conn = connectMaintenanceDatabase(&cparams, progname, echo);
initPQExpBuffer(&sql);
diff --git a/src/bin/scripts/dropdb.c b/src/bin/scripts/dropdb.c
index 581c7749c8..ccbf78e91a 100644
--- a/src/bin/scripts/dropdb.c
+++ b/src/bin/scripts/dropdb.c
@@ -48,6 +48,7 @@ main(int argc, char *argv[])
char *port = NULL;
char *username = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
+ ConnParams cparams;
bool echo = false;
bool interactive = false;
bool force = false;
@@ -137,9 +138,14 @@ main(int argc, char *argv[])
if (maintenance_db == NULL && strcmp(dbname, "postgres") == 0)
maintenance_db = "template1";
- conn = connectMaintenanceDatabase(maintenance_db,
- host, port, username, prompt_password,
- progname, echo);
+ cparams.dbname = maintenance_db;
+ cparams.pghost = host;
+ cparams.pgport = port;
+ cparams.pguser = username;
+ cparams.prompt_password = prompt_password;
+ cparams.override_dbname = NULL;
+
+ conn = connectMaintenanceDatabase(&cparams, progname, echo);
if (echo)
printf("%s\n", sql.data);
diff --git a/src/bin/scripts/dropuser.c b/src/bin/scripts/dropuser.c
index f7ddd1402d..73d7328a88 100644
--- a/src/bin/scripts/dropuser.c
+++ b/src/bin/scripts/dropuser.c
@@ -46,6 +46,7 @@ main(int argc, char *argv[])
char *port = NULL;
char *username = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
+ ConnParams cparams;
bool echo = false;
bool interactive = false;
@@ -129,13 +130,19 @@ main(int argc, char *argv[])
exit(0);
}
+ cparams.dbname = NULL; /* this program lacks any dbname option... */
+ cparams.pghost = host;
+ cparams.pgport = port;
+ cparams.pguser = username;
+ cparams.prompt_password = prompt_password;
+ cparams.override_dbname = NULL;
+
+ conn = connectMaintenanceDatabase(&cparams, progname, echo);
+
initPQExpBuffer(&sql);
appendPQExpBuffer(&sql, "DROP ROLE %s%s;",
(if_exists ? "IF EXISTS " : ""), fmtId(dropuser));
- conn = connectDatabase("postgres", host, port, username, prompt_password,
- progname, echo, false, false);
-
if (echo)
printf("%s\n", sql.data);
result = PQexec(conn, sql.data);
diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c
index 40dcbc9283..40003380ae 100644
--- a/src/bin/scripts/reindexdb.c
+++ b/src/bin/scripts/reindexdb.c
@@ -34,15 +34,12 @@ static SimpleStringList *get_parallel_object_list(PGconn *conn,
ReindexType type,
SimpleStringList *user_list,
bool echo);
-static void reindex_one_database(const char *dbname, ReindexType type,
- SimpleStringList *user_list, const char *host,
- const char *port, const char *username,
- enum trivalue prompt_password, const char *progname,
+static void reindex_one_database(const ConnParams *cparams, ReindexType type,
+ SimpleStringList *user_list,
+ const char *progname,
bool echo, bool verbose, bool concurrently,
int concurrentCons);
-static void reindex_all_databases(const char *maintenance_db,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
+static void reindex_all_databases(ConnParams *cparams,
const char *progname, bool echo,
bool quiet, bool verbose, bool concurrently,
int concurrentCons);
@@ -86,6 +83,7 @@ main(int argc, char *argv[])
const char *port = NULL;
const char *username = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
+ ConnParams cparams;
bool syscatalog = false;
bool alldb = false;
bool echo = false;
@@ -188,6 +186,13 @@ main(int argc, char *argv[])
exit(1);
}
+ /* fill cparams except for dbname, which is set below */
+ cparams.pghost = host;
+ cparams.pgport = port;
+ cparams.pguser = username;
+ cparams.prompt_password = prompt_password;
+ cparams.override_dbname = NULL;
+
setup_cancel_handler(NULL);
if (alldb)
@@ -218,8 +223,9 @@ main(int argc, char *argv[])
exit(1);
}
- reindex_all_databases(maintenance_db, host, port, username,
- prompt_password, progname, echo, quiet, verbose,
+ cparams.dbname = maintenance_db;
+
+ reindex_all_databases(&cparams, progname, echo, quiet, verbose,
concurrently, concurrentCons);
}
else if (syscatalog)
@@ -256,9 +262,11 @@ main(int argc, char *argv[])
dbname = get_user_name_or_exit(progname);
}
- reindex_one_database(dbname, REINDEX_SYSTEM, NULL, host,
- port, username, prompt_password, progname,
- echo, verbose, concurrently, 1);
+ cparams.dbname = dbname;
+
+ reindex_one_database(&cparams, REINDEX_SYSTEM, NULL,
+ progname, echo, verbose,
+ concurrently, 1);
}
else
{
@@ -283,40 +291,40 @@ main(int argc, char *argv[])
dbname = get_user_name_or_exit(progname);
}
+ cparams.dbname = dbname;
+
if (schemas.head != NULL)
- reindex_one_database(dbname, REINDEX_SCHEMA, &schemas, host,
- port, username, prompt_password, progname,
- echo, verbose, concurrently, concurrentCons);
+ reindex_one_database(&cparams, REINDEX_SCHEMA, &schemas,
+ progname, echo, verbose,
+ concurrently, concurrentCons);
if (indexes.head != NULL)
- reindex_one_database(dbname, REINDEX_INDEX, &indexes, host,
- port, username, prompt_password, progname,
- echo, verbose, concurrently, 1);
+ reindex_one_database(&cparams, REINDEX_INDEX, &indexes,
+ progname, echo, verbose,
+ concurrently, 1);
if (tables.head != NULL)
- reindex_one_database(dbname, REINDEX_TABLE, &tables, host,
- port, username, prompt_password, progname,
- echo, verbose, concurrently,
- concurrentCons);
+ reindex_one_database(&cparams, REINDEX_TABLE, &tables,
+ progname, echo, verbose,
+ concurrently, concurrentCons);
/*
* reindex database only if neither index nor table nor schema is
* specified
*/
if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL)
- reindex_one_database(dbname, REINDEX_DATABASE, NULL, host,
- port, username, prompt_password, progname,
- echo, verbose, concurrently, concurrentCons);
+ reindex_one_database(&cparams, REINDEX_DATABASE, NULL,
+ progname, echo, verbose,
+ concurrently, concurrentCons);
}
exit(0);
}
static void
-reindex_one_database(const char *dbname, ReindexType type,
- SimpleStringList *user_list, const char *host,
- const char *port, const char *username,
- enum trivalue prompt_password, const char *progname, bool echo,
+reindex_one_database(const ConnParams *cparams, ReindexType type,
+ SimpleStringList *user_list,
+ const char *progname, bool echo,
bool verbose, bool concurrently, int concurrentCons)
{
PGconn *conn;
@@ -328,8 +336,7 @@ reindex_one_database(const char *dbname, ReindexType type,
bool failed = false;
int items_count = 0;
- conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, echo, false, false);
+ conn = connectDatabase(cparams, progname, echo, false, false);
if (concurrently && PQserverVersion(conn) < 120000)
{
@@ -436,8 +443,7 @@ reindex_one_database(const char *dbname, ReindexType type,
Assert(process_list != NULL);
- slots = ParallelSlotsSetup(dbname, host, port, username, prompt_password,
- progname, echo, conn, concurrentCons);
+ slots = ParallelSlotsSetup(cparams, progname, echo, conn, concurrentCons);
cell = process_list->head;
do
@@ -705,23 +711,18 @@ get_parallel_object_list(PGconn *conn, ReindexType type,
}
static void
-reindex_all_databases(const char *maintenance_db,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
+reindex_all_databases(ConnParams *cparams,
const char *progname, bool echo, bool quiet, bool verbose,
bool concurrently, int concurrentCons)
{
PGconn *conn;
PGresult *result;
- PQExpBufferData connstr;
int i;
- conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname, echo);
+ conn = connectMaintenanceDatabase(cparams, progname, echo);
result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", echo);
PQfinish(conn);
- initPQExpBuffer(&connstr);
for (i = 0; i < PQntuples(result); i++)
{
char *dbname = PQgetvalue(result, i, 0);
@@ -732,16 +733,12 @@ reindex_all_databases(const char *maintenance_db,
fflush(stdout);
}
- resetPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, dbname);
+ cparams->override_dbname = dbname;
- reindex_one_database(connstr.data, REINDEX_DATABASE, NULL, host,
- port, username, prompt_password,
+ reindex_one_database(cparams, REINDEX_DATABASE, NULL,
progname, echo, verbose, concurrently,
concurrentCons);
}
- termPQExpBuffer(&connstr);
PQclear(result);
}
diff --git a/src/bin/scripts/scripts_parallel.c b/src/bin/scripts/scripts_parallel.c
index 01bc6dfeff..ec264a269a 100644
--- a/src/bin/scripts/scripts_parallel.c
+++ b/src/bin/scripts/scripts_parallel.c
@@ -205,8 +205,7 @@ ParallelSlotsGetIdle(ParallelSlot *slots, int numslots)
* set.
*/
ParallelSlot *
-ParallelSlotsSetup(const char *dbname, const char *host, const char *port,
- const char *username, bool prompt_password,
+ParallelSlotsSetup(const ConnParams *cparams,
const char *progname, bool echo,
PGconn *conn, int numslots)
{
@@ -221,8 +220,7 @@ ParallelSlotsSetup(const char *dbname, const char *host, const char *port,
{
for (i = 1; i < numslots; i++)
{
- conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, echo, false, true);
+ conn = connectDatabase(cparams, progname, echo, false, true);
/*
* Fail and exit immediately if trying to use a socket in an
diff --git a/src/bin/scripts/scripts_parallel.h b/src/bin/scripts/scripts_parallel.h
index cf20449ce3..c9d9f0623e 100644
--- a/src/bin/scripts/scripts_parallel.h
+++ b/src/bin/scripts/scripts_parallel.h
@@ -12,6 +12,7 @@
#ifndef SCRIPTS_PARALLEL_H
#define SCRIPTS_PARALLEL_H
+#include "common.h"
#include "libpq-fe.h"
@@ -23,10 +24,7 @@ typedef struct ParallelSlot
extern ParallelSlot *ParallelSlotsGetIdle(ParallelSlot *slots, int numslots);
-extern ParallelSlot *ParallelSlotsSetup(const char *dbname, const char *host,
- const char *port,
- const char *username,
- bool prompt_password,
+extern ParallelSlot *ParallelSlotsSetup(const ConnParams *cparams,
const char *progname, bool echo,
PGconn *conn, int numslots);
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 2a1247a1b0..8c2eade1d5 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -42,19 +42,16 @@ typedef struct vacuumingOptions
} vacuumingOptions;
-static void vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
+static void vacuum_one_database(const ConnParams *cparams,
+ vacuumingOptions *vacopts,
int stage,
SimpleStringList *tables,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
int concurrentCons,
const char *progname, bool echo, bool quiet);
-static void vacuum_all_databases(vacuumingOptions *vacopts,
+static void vacuum_all_databases(ConnParams *cparams,
+ vacuumingOptions *vacopts,
bool analyze_in_stages,
- const char *maintenance_db,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
int concurrentCons,
const char *progname, bool echo, bool quiet);
@@ -112,6 +109,7 @@ main(int argc, char *argv[])
char *port = NULL;
char *username = NULL;
enum trivalue prompt_password = TRI_DEFAULT;
+ ConnParams cparams;
bool echo = false;
bool quiet = false;
vacuumingOptions vacopts;
@@ -311,6 +309,13 @@ main(int argc, char *argv[])
}
}
+ /* fill cparams except for dbname, which is set below */
+ cparams.pghost = host;
+ cparams.pgport = port;
+ cparams.pguser = username;
+ cparams.prompt_password = prompt_password;
+ cparams.override_dbname = NULL;
+
setup_cancel_handler(NULL);
/* Avoid opening extra connections. */
@@ -330,10 +335,10 @@ main(int argc, char *argv[])
exit(1);
}
- vacuum_all_databases(&vacopts,
+ cparams.dbname = maintenance_db;
+
+ vacuum_all_databases(&cparams, &vacopts,
analyze_in_stages,
- maintenance_db,
- host, port, username, prompt_password,
concurrentCons,
progname, echo, quiet);
}
@@ -349,25 +354,25 @@ main(int argc, char *argv[])
dbname = get_user_name_or_exit(progname);
}
+ cparams.dbname = dbname;
+
if (analyze_in_stages)
{
int stage;
for (stage = 0; stage < ANALYZE_NUM_STAGES; stage++)
{
- vacuum_one_database(dbname, &vacopts,
+ vacuum_one_database(&cparams, &vacopts,
stage,
&tables,
- host, port, username, prompt_password,
concurrentCons,
progname, echo, quiet);
}
}
else
- vacuum_one_database(dbname, &vacopts,
+ vacuum_one_database(&cparams, &vacopts,
ANALYZE_NO_STAGE,
&tables,
- host, port, username, prompt_password,
concurrentCons,
progname, echo, quiet);
}
@@ -389,11 +394,10 @@ main(int argc, char *argv[])
* a list of tables from the database.
*/
static void
-vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
+vacuum_one_database(const ConnParams *cparams,
+ vacuumingOptions *vacopts,
int stage,
SimpleStringList *tables,
- const char *host, const char *port,
- const char *username, enum trivalue prompt_password,
int concurrentCons,
const char *progname, bool echo, bool quiet)
{
@@ -424,8 +428,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
Assert(stage == ANALYZE_NO_STAGE ||
(stage >= 0 && stage < ANALYZE_NUM_STAGES));
- conn = connectDatabase(dbname, host, port, username, prompt_password,
- progname, echo, false, true);
+ conn = connectDatabase(cparams, progname, echo, false, true);
if (vacopts->disable_page_skipping && PQserverVersion(conn) < 90600)
{
@@ -663,8 +666,7 @@ vacuum_one_database(const char *dbname, vacuumingOptions *vacopts,
* for the first slot. If not in parallel mode, the first slot in the
* array contains the connection.
*/
- slots = ParallelSlotsSetup(dbname, host, port, username, prompt_password,
- progname, echo, conn, concurrentCons);
+ slots = ParallelSlotsSetup(cparams, progname, echo, conn, concurrentCons);
/*
* Prepare all the connections to run the appropriate analyze stage, if
@@ -736,28 +738,23 @@ finish:
* quickly everywhere before generating more detailed ones.
*/
static void
-vacuum_all_databases(vacuumingOptions *vacopts,
+vacuum_all_databases(ConnParams *cparams,
+ vacuumingOptions *vacopts,
bool analyze_in_stages,
- const char *maintenance_db, const char *host,
- const char *port, const char *username,
- enum trivalue prompt_password,
int concurrentCons,
const char *progname, bool echo, bool quiet)
{
PGconn *conn;
PGresult *result;
- PQExpBufferData connstr;
int stage;
int i;
- conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
- prompt_password, progname, echo);
+ conn = connectMaintenanceDatabase(cparams, progname, echo);
result = executeQuery(conn,
"SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;",
echo);
PQfinish(conn);
- initPQExpBuffer(&connstr);
if (analyze_in_stages)
{
/*
@@ -772,14 +769,11 @@ vacuum_all_databases(vacuumingOptions *vacopts,
{
for (i = 0; i < PQntuples(result); i++)
{
- resetPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, PQgetvalue(result, i, 0));
+ cparams->override_dbname = PQgetvalue(result, i, 0);
- vacuum_one_database(connstr.data, vacopts,
+ vacuum_one_database(cparams, vacopts,
stage,
NULL,
- host, port, username, prompt_password,
concurrentCons,
progname, echo, quiet);
}
@@ -789,19 +783,15 @@ vacuum_all_databases(vacuumingOptions *vacopts,
{
for (i = 0; i < PQntuples(result); i++)
{
- resetPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, PQgetvalue(result, i, 0));
+ cparams->override_dbname = PQgetvalue(result, i, 0);
- vacuum_one_database(connstr.data, vacopts,
+ vacuum_one_database(cparams, vacopts,
ANALYZE_NO_STAGE,
NULL,
- host, port, username, prompt_password,
concurrentCons,
progname, echo, quiet);
}
}
- termPQExpBuffer(&connstr);
PQclear(result);
}
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index ee3fc09577..77eae07106 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -889,27 +889,39 @@ testdb=>
<para>
Establishes a new connection to a <productname>PostgreSQL</productname>
server. The connection parameters to use can be specified either
- using a positional syntax, or using <replaceable>conninfo</replaceable> connection
- strings as detailed in <xref linkend="libpq-connstring"/>.
+ using a positional syntax (one or more of database name, user,
+ host, and port), or using a <replaceable>conninfo</replaceable>
+ connection string as detailed in
+ <xref linkend="libpq-connstring"/>. If no arguments are given, a
+ new connection is made using the same parameters as before.
</para>
<para>
- Where the command omits database name, user, host, or port, the new
- connection can reuse values from the previous connection. By default,
- values from the previous connection are reused except when processing
- a <replaceable>conninfo</replaceable> string. Passing a first argument
- of <literal>-reuse-previous=on</literal>
- or <literal>-reuse-previous=off</literal> overrides that default.
- When the command neither specifies nor reuses a particular parameter,
- the <application>libpq</application> default is used. Specifying any
+ Specifying any
of <replaceable class="parameter">dbname</replaceable>,
<replaceable class="parameter">username</replaceable>,
<replaceable class="parameter">host</replaceable> or
<replaceable class="parameter">port</replaceable>
as <literal>-</literal> is equivalent to omitting that parameter.
- If <literal>hostaddr</literal> was specified in the original
- connection's <structname>conninfo</structname>, that address is reused
- for the new connection (disregarding any other host specification).
+ </para>
+
+ <para>
+ The new connection can re-use connection parameters from the previous
+ connection; not only database name, user, host, and port, but other
+ settings such as <replaceable>sslmode</replaceable>. By default,
+ parameters are re-used in the positional syntax, but not when
+ a <replaceable>conninfo</replaceable> string is given. Passing a
+ first argument of <literal>-reuse-previous=on</literal>
+ or <literal>-reuse-previous=off</literal> overrides that default. If
+ parameters are re-used, then any parameter not explicitly specified as
+ a positional parameter or in the <replaceable>conninfo</replaceable>
+ string is taken from the existing connection's parameters. An
+ exception is that if the <replaceable>host</replaceable> setting
+ is changed from its previous value using the positional syntax,
+ any <replaceable>hostaddr</replaceable> setting present in the
+ existing connection's parameters is dropped.
+ When the command neither specifies nor reuses a particular parameter,
+ the <application>libpq</application> default is used.
</para>
<para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d4aa0976b5..ac272d0588 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3011,26 +3011,6 @@ param_is_newly_set(const char *old_val, const char *new_val)
return false;
}
-/* return whether the connection has 'hostaddr' in its conninfo */
-static bool
-has_hostaddr(PGconn *conn)
-{
- bool used = false;
- PQconninfoOption *ciopt = PQconninfo(conn);
-
- for (PQconninfoOption *p = ciopt; p->keyword != NULL; p++)
- {
- if (strcmp(p->keyword, "hostaddr") == 0 && p->val != NULL)
- {
- used = true;
- break;
- }
- }
-
- PQconninfoFree(ciopt);
- return used;
-}
-
/*
* do_connect -- handler for \connect
*
@@ -3048,13 +3028,15 @@ do_connect(enum trivalue reuse_previous_specification,
char *dbname, char *user, char *host, char *port)
{
PGconn *o_conn = pset.db,
- *n_conn;
+ *n_conn = NULL;
+ PQconninfoOption *cinfo;
+ int nconnopts = 0;
+ bool same_host = false;
char *password = NULL;
- char *hostaddr = NULL;
- bool keep_password;
+ bool success = true;
+ bool keep_password = true;
bool has_connection_string;
bool reuse_previous;
- PQExpBufferData connstr;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -3096,55 +3078,125 @@ do_connect(enum trivalue reuse_previous_specification,
}
/*
- * Grab missing values from the old connection. If we grab host (or host
- * is the same as before) and hostaddr was set, grab that too.
+ * If we intend to re-use connection parameters, collect them out of the
+ * old connection, then replace individual values as necessary. Otherwise,
+ * obtain a PQconninfoOption array containing libpq's defaults, and modify
+ * that. Note this function assumes that PQconninfo, PQconndefaults, and
+ * PQconninfoParse will all produce arrays containing the same options in
+ * the same order.
*/
if (reuse_previous)
+ cinfo = PQconninfo(o_conn);
+ else
+ cinfo = PQconndefaults();
+
+ if (cinfo)
{
- if (!user)
- user = PQuser(o_conn);
- if (host && strcmp(host, PQhost(o_conn)) == 0 &&
- has_hostaddr(o_conn))
+ if (has_connection_string)
{
- hostaddr = PQhostaddr(o_conn);
+ /* Parse the connstring and insert values into cinfo */
+ PQconninfoOption *replcinfo;
+ char *errmsg;
+
+ replcinfo = PQconninfoParse(dbname, &errmsg);
+ if (replcinfo)
+ {
+ PQconninfoOption *ci;
+ PQconninfoOption *replci;
+
+ for (ci = cinfo, replci = replcinfo;
+ ci->keyword && replci->keyword;
+ ci++, replci++)
+ {
+ Assert(strcmp(ci->keyword, replci->keyword) == 0);
+ /* Insert value from connstring if one was provided */
+ if (replci->val)
+ {
+ /*
+ * We know that both val strings were allocated by
+ * libpq, so the least messy way to avoid memory leaks
+ * is to swap them.
+ */
+ char *swap = replci->val;
+
+ replci->val = ci->val;
+ ci->val = swap;
+ }
+ }
+ Assert(ci->keyword == NULL && replci->keyword == NULL);
+
+ /* While here, determine how many option slots there are */
+ nconnopts = ci - cinfo;
+
+ PQconninfoFree(replcinfo);
+
+ /* We never re-use a password with a conninfo string. */
+ keep_password = false;
+
+ /* Don't let code below try to inject dbname into params. */
+ dbname = NULL;
+ }
+ else
+ {
+ /* PQconninfoParse failed */
+ if (errmsg)
+ {
+ pg_log_error("%s", errmsg);
+ PQfreemem(errmsg);
+ }
+ else
+ pg_log_error("out of memory");
+ success = false;
+ }
}
- if (!host)
+ else
{
- host = PQhost(o_conn);
- if (has_hostaddr(o_conn))
- hostaddr = PQhostaddr(o_conn);
+ /*
+ * If dbname isn't a connection string, then we'll inject it and
+ * the other parameters into the keyword array below. (We can't
+ * easily insert them into the cinfo array because of memory
+ * management issues: PQconninfoFree would misbehave on Windows.)
+ * However, to avoid dependencies on the order in which parameters
+ * appear in the array, make a preliminary scan to set
+ * keep_password and same_host correctly.
+ *
+ * While any change in user, host, or port causes us to ignore the
+ * old connection's password, we don't force that for dbname,
+ * since passwords aren't database-specific.
+ */
+ PQconninfoOption *ci;
+
+ for (ci = cinfo; ci->keyword; ci++)
+ {
+ if (user && strcmp(ci->keyword, "user") == 0)
+ {
+ if (!(ci->val && strcmp(user, ci->val) == 0))
+ keep_password = false;
+ }
+ else if (host && strcmp(ci->keyword, "host") == 0)
+ {
+ if (ci->val && strcmp(host, ci->val) == 0)
+ same_host = true;
+ else
+ keep_password = false;
+ }
+ else if (port && strcmp(ci->keyword, "port") == 0)
+ {
+ if (!(ci->val && strcmp(port, ci->val) == 0))
+ keep_password = false;
+ }
+ }
+
+ /* While here, determine how many option slots there are */
+ nconnopts = ci - cinfo;
}
- if (!port)
- port = PQport(o_conn);
}
-
- /*
- * Any change in the parameters read above makes us discard the password.
- * We also discard it if we're to use a conninfo rather than the
- * positional syntax.
- */
- if (has_connection_string)
- keep_password = false;
else
- keep_password =
- (user && PQuser(o_conn) && strcmp(user, PQuser(o_conn)) == 0) &&
- (host && PQhost(o_conn) && strcmp(host, PQhost(o_conn)) == 0) &&
- (port && PQport(o_conn) && strcmp(port, PQport(o_conn)) == 0);
-
- /*
- * Grab missing dbname from old connection. No password discard if this
- * changes: passwords aren't (usually) database-specific.
- */
- if (!dbname && reuse_previous)
{
- initPQExpBuffer(&connstr);
- appendPQExpBufferStr(&connstr, "dbname=");
- appendConnStrVal(&connstr, PQdb(o_conn));
- dbname = connstr.data;
- /* has_connection_string=true would be a dead store */
+ /* We failed to create the cinfo structure */
+ pg_log_error("out of memory");
+ success = false;
}
- else
- connstr.data = NULL;
/*
* If the user asked to be prompted for a password, ask for one now. If
@@ -3156,7 +3208,7 @@ do_connect(enum trivalue reuse_previous_specification,
* the postmaster's log. But libpq offers no API that would let us obtain
* a password and then continue with the first connection attempt.
*/
- if (pset.getPassword == TRI_YES)
+ if (pset.getPassword == TRI_YES && success)
{
/*
* If a connstring or URI is provided, we can't be sure we know which
@@ -3176,57 +3228,62 @@ do_connect(enum trivalue reuse_previous_specification,
password = NULL;
}
- while (true)
+ /* Loop till we have a connection or fail, which we might've already */
+ while (success)
{
-#define PARAMS_ARRAY_SIZE 9
- const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
- const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
- int paramnum = -1;
-
- keywords[++paramnum] = "host";
- values[paramnum] = host;
- if (hostaddr && *hostaddr)
- {
- keywords[++paramnum] = "hostaddr";
- values[paramnum] = hostaddr;
- }
- keywords[++paramnum] = "port";
- values[paramnum] = port;
- keywords[++paramnum] = "user";
- values[paramnum] = user;
+ const char **keywords = pg_malloc((nconnopts + 1) * sizeof(*keywords));
+ const char **values = pg_malloc((nconnopts + 1) * sizeof(*values));
+ int paramnum = 0;
+ PQconninfoOption *ci;
/*
- * Position in the array matters when the dbname is a connection
- * string, because settings in a connection string override earlier
- * array entries only. Thus, user= in the connection string always
- * takes effect, but client_encoding= often will not.
+ * Copy non-default settings into the PQconnectdbParams parameter
+ * arrays; but override any values specified old-style, as well as the
+ * password and a couple of fields we want to set forcibly.
+ *
+ * XXX: need to update this comment (and maybe main() too?)
*
* If you change this code, also change the initial-connection code in
* main(). For no good reason, a connection string password= takes
* precedence in main() but not here.
*/
- keywords[++paramnum] = "dbname";
- values[paramnum] = dbname;
- keywords[++paramnum] = "password";
- values[paramnum] = password;
- keywords[++paramnum] = "fallback_application_name";
- values[paramnum] = pset.progname;
- keywords[++paramnum] = "client_encoding";
- values[paramnum] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
-
+ for (ci = cinfo; ci->keyword; ci++)
+ {
+ keywords[paramnum] = ci->keyword;
+
+ if (dbname && strcmp(ci->keyword, "dbname") == 0)
+ values[paramnum++] = dbname;
+ else if (user && strcmp(ci->keyword, "user") == 0)
+ values[paramnum++] = user;
+ else if (host && strcmp(ci->keyword, "host") == 0)
+ values[paramnum++] = host;
+ else if (host && !same_host && strcmp(ci->keyword, "hostaddr") == 0)
+ {
+ /* If we're changing the host value, drop any old hostaddr */
+ values[paramnum++] = NULL;
+ }
+ else if (port && strcmp(ci->keyword, "port") == 0)
+ values[paramnum++] = port;
+ else if (strcmp(ci->keyword, "password") == 0)
+ values[paramnum++] = password;
+ else if (strcmp(ci->keyword, "fallback_application_name") == 0)
+ values[paramnum++] = pset.progname;
+ else if (strcmp(ci->keyword, "client_encoding") == 0)
+ values[paramnum++] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
+ else if (ci->val)
+ values[paramnum++] = ci->val;
+ /* else, don't bother making libpq parse this keyword */
+ }
/* add array terminator */
- keywords[++paramnum] = NULL;
+ keywords[paramnum] = NULL;
values[paramnum] = NULL;
- n_conn = PQconnectdbParams(keywords, values, true);
+ /* Note we do not want libpq to re-expand the dbname parameter */
+ n_conn = PQconnectdbParams(keywords, values, false);
pg_free(keywords);
pg_free(values);
- /* We can immediately discard the password -- no longer needed */
- if (password)
- pg_free(password);
-
if (PQstatus(n_conn) == CONNECTION_OK)
break;
@@ -3242,9 +3299,28 @@ do_connect(enum trivalue reuse_previous_specification,
*/
password = prompt_for_password(PQuser(n_conn));
PQfinish(n_conn);
+ n_conn = NULL;
continue;
}
+ /*
+ * We'll report the error below ... unless n_conn is NULL, indicating
+ * that libpq didn't have enough memory to make a PGconn.
+ */
+ if (n_conn == NULL)
+ pg_log_error("out of memory");
+
+ success = false;
+ } /* end retry loop */
+
+ /* Release locally allocated data, whether we succeeded or not */
+ if (password)
+ pg_free(password);
+ if (cinfo)
+ PQconninfoFree(cinfo);
+
+ if (!success)
+ {
/*
* Failed to connect to the database. In interactive mode, keep the
* previous connection to the DB; in scripting mode, close our
@@ -3252,7 +3328,11 @@ do_connect(enum trivalue reuse_previous_specification,
*/
if (pset.cur_cmd_interactive)
{
- pg_log_info("%s", PQerrorMessage(n_conn));
+ if (n_conn)
+ {
+ pg_log_info("%s", PQerrorMessage(n_conn));
+ PQfinish(n_conn);
+ }
/* pset.db is left unmodified */
if (o_conn)
@@ -3260,7 +3340,12 @@ do_connect(enum trivalue reuse_previous_specification,
}
else
{
- pg_log_error("\\connect: %s", PQerrorMessage(n_conn));
+ if (n_conn)
+ {
+ pg_log_error("\\connect: %s", PQerrorMessage(n_conn));
+ PQfinish(n_conn);
+ }
+
if (o_conn)
{
/*
@@ -3274,13 +3359,8 @@ do_connect(enum trivalue reuse_previous_specification,
}
}
- PQfinish(n_conn);
- if (connstr.data)
- termPQExpBuffer(&connstr);
return false;
}
- if (connstr.data)
- termPQExpBuffer(&connstr);
/*
* Replace the old connection with the new one, and update