Skip to content

Commit da129a0

Browse files
tglsfdctglsfdc
committed
Fix connection string handling in src/bin/scripts/ programs.
When told to process all databases, clusterdb, reindexdb, and vacuumdb would reconnect by replacing their --maintenance-db parameter with the name of the target database. If that parameter is a connstring (which has been allowed for a long time, though we failed to document that before this patch), we'd lose any other options it might specify, for example SSL or GSS parameters, possibly resulting in failure to connect. Thus, this is the same bug as commit a45bc8a fixed in pg_dump and pg_restore. We can fix it in the same way, by using libpq's rules for handling multiple "dbname" parameters to add the target database name separately. I chose to apply the same refactoring approach as in that patch, with a struct to handle the command line parameters that need to be passed through to connectDatabase. (Maybe someday we can unify the very similar functions here and in pg_dump/pg_restore.) Per Peter Eisentraut's comments on bug #16604. Back-patch to all supported branches. Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org
1 parent 6603763 commit da129a0

File tree

16 files changed

+274
-200
lines changed

16 files changed

+274
-200
lines changed

‎doc/src/sgml/ref/clusterdb.sgml

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ PostgreSQL documentation
9090
<term><option><optional>--dbname=</><replaceable class="parameter">dbname</replaceable></></term>
9191
<listitem>
9292
<para>
93-
Specifies the name of the database to be clustered.
94-
If this is not specified and <option>-a</option> (or
95-
<option>--all</option>) is not used, the database name is read
93+
Specifies the name of the database to be clustered,
94+
when <option>-a</option>/<option>--all</option> is not used.
95+
If this is not specified, the database name is read
9696
from the environment variable <envar>PGDATABASE</envar>. If
9797
that is not set, the user name specified for the connection is
9898
used. The <replaceable>dbname</replaceable> can be a <link
@@ -249,10 +249,16 @@ PostgreSQL documentation
249249
<term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></></term>
250250
<listitem>
251251
<para>
252-
Specifies the name of the database to connect to discover what other
253-
databases should be clustered. If not specified, the
254-
<literal>postgres</literal> database will be used,
255-
and if that does not exist, <literal>template1</literal> will be used.
252+
Specifies the name of the database to connect to to discover which
253+
databases should be clustered,
254+
when <option>-a</option>/<option>--all</option> is used.
255+
If not specified, the <literal>postgres</literal> database will be used,
256+
or if that does not exist, <literal>template1</literal> will be used.
257+
This can be a <link linkend="libpq-connstring">connection
258+
string</link>. If so, connection string parameters will override any
259+
conflicting command line options. Also, connection string parameters
260+
other than the database name itself will be re-used when connecting
261+
to other databases.
256262
</para>
257263
</listitem>
258264
</varlistentry>

‎doc/src/sgml/ref/createdb.sgml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,9 @@ PostgreSQL documentation
286286
database will be used; if that does not exist (or if it is the name
287287
of the new database being created), <literal>template1</literal> will
288288
be used.
289+
This can be a <link linkend="libpq-connstring">connection
290+
string</link>. If so, connection string parameters will override any
291+
conflicting command line options.
289292
</para>
290293
</listitem>
291294
</varlistentry>

‎doc/src/sgml/ref/dropdb.sgml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ PostgreSQL documentation
205205
target database. If not specified, the <literal>postgres</literal>
206206
database will be used; if that does not exist (or is the database
207207
being dropped), <literal>template1</literal> will be used.
208+
This can be a <link linkend="libpq-connstring">connection
209+
string</link>. If so, connection string parameters will override any
210+
conflicting command line options.
208211
</para>
209212
</listitem>
210213
</varlistentry>

‎doc/src/sgml/ref/reindexdb.sgml

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,9 @@ PostgreSQL documentation
123123
<term><option><optional>--dbname=</><replaceable class="parameter">dbname</replaceable></></term>
124124
<listitem>
125125
<para>
126-
Specifies the name of the database to be reindexed.
127-
If this is not specified and <option>-a</option> (or
128-
<option>--all</option>) is not used, the database name is read
126+
Specifies the name of the database to be reindexed,
127+
when <option>-a</option>/<option>--all</option> is not used.
128+
If this is not specified, the database name is read
129129
from the environment variable <envar>PGDATABASE</envar>. If
130130
that is not set, the user name specified for the connection is
131131
used. The <replaceable>dbname</replaceable> can be a <link
@@ -317,10 +317,16 @@ PostgreSQL documentation
317317
<term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></></term>
318318
<listitem>
319319
<para>
320-
Specifies the name of the database to connect to discover what other
321-
databases should be reindexed. If not specified, the
322-
<literal>postgres</literal> database will be used,
323-
and if that does not exist, <literal>template1</literal> will be used.
320+
Specifies the name of the database to connect to to discover which
321+
databases should be reindexed,
322+
when <option>-a</option>/<option>--all</option> is used.
323+
If not specified, the <literal>postgres</literal> database will be used,
324+
or if that does not exist, <literal>template1</literal> will be used.
325+
This can be a <link linkend="libpq-connstring">connection
326+
string</link>. If so, connection string parameters will override any
327+
conflicting command line options. Also, connection string parameters
328+
other than the database name itself will be re-used when connecting
329+
to other databases.
324330
</para>
325331
</listitem>
326332
</varlistentry>

‎doc/src/sgml/ref/vacuumdb.sgml

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ PostgreSQL documentation
9292
<term><option><optional>--dbname=</><replaceable class="parameter">dbname</replaceable></option></term>
9393
<listitem>
9494
<para>
95-
Specifies the name of the database to be cleaned or analyzed.
96-
If this is not specified and <option>-a</option> (or
97-
<option>--all</option>) is not used, the database name is read
95+
Specifies the name of the database to be cleaned or analyzed,
96+
when <option>-a</option>/<option>--all</option> is not used.
97+
If this is not specified, the database name is read
9898
from the environment variable <envar>PGDATABASE</envar>. If
9999
that is not set, the user name specified for the connection is
100100
used. The <replaceable>dbname</replaceable> can be a <link
@@ -342,10 +342,16 @@ PostgreSQL documentation
342342
<term><option>--maintenance-db=<replaceable class="parameter">dbname</replaceable></></term>
343343
<listitem>
344344
<para>
345-
Specifies the name of the database to connect to discover what other
346-
databases should be vacuumed. If not specified, the
347-
<literal>postgres</literal> database will be used,
348-
and if that does not exist, <literal>template1</literal> will be used.
345+
Specifies the name of the database to connect to to discover which
346+
databases should be vacuumed,
347+
when <option>-a</option>/<option>--all</option> is used.
348+
If not specified, the <literal>postgres</literal> database will be used,
349+
or if that does not exist, <literal>template1</literal> will be used.
350+
This can be a <link linkend="libpq-connstring">connection
351+
string</link>. If so, connection string parameters will override any
352+
conflicting command line options. Also, connection string parameters
353+
other than the database name itself will be re-used when connecting
354+
to other databases.
349355
</para>
350356
</listitem>
351357
</varlistentry>

‎src/bin/scripts/clusterdb.c

Lines changed: 29 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,10 @@
1414
#include "dumputils.h"
1515

1616

17-
static void cluster_one_database(const char *dbname, bool verbose, const char *table,
18-
const char *host, const char *port,
19-
const char *username, enum trivalue prompt_password,
20-
const char *progname, bool echo);
21-
static void cluster_all_databases(bool verbose, const char *maintenance_db,
22-
const char *host, const char *port,
23-
const char *username, enum trivalue prompt_password,
24-
const char *progname, bool echo, bool quiet);
25-
17+
static void cluster_one_database(const ConnParams *cparams, const char *table,
18+
const char *progname, bool verbose, bool echo);
19+
static void cluster_all_databases(ConnParams *cparams, const char *progname,
20+
bool verbose, bool echo, bool quiet);
2621
static void help(const char *progname);
2722

2823

@@ -55,6 +50,7 @@ main(int argc, char *argv[])
5550
char *port = NULL;
5651
char *username = NULL;
5752
enum trivalue prompt_password = TRI_DEFAULT;
53+
ConnParams cparams;
5854
bool echo = false;
5955
bool quiet = false;
6056
bool alldb = false;
@@ -130,6 +126,13 @@ main(int argc, char *argv[])
130126
exit(1);
131127
}
132128

129+
/* fill cparams except for dbname, which is set below */
130+
cparams.pghost = host;
131+
cparams.pgport = port;
132+
cparams.pguser = username;
133+
cparams.prompt_password = prompt_password;
134+
cparams.override_dbname = NULL;
135+
133136
setup_cancel_handler();
134137

135138
if (alldb)
@@ -148,8 +151,9 @@ main(int argc, char *argv[])
148151
exit(1);
149152
}
150153

151-
cluster_all_databases(verbose, maintenance_db, host, port, username, prompt_password,
152-
progname, echo, quiet);
154+
cparams.dbname = maintenance_db;
155+
156+
cluster_all_databases(&cparams, progname, verbose, echo, quiet);
153157
}
154158
else
155159
{
@@ -163,39 +167,36 @@ main(int argc, char *argv[])
163167
dbname = get_user_name_or_exit(progname);
164168
}
165169

170+
cparams.dbname = dbname;
171+
166172
if (tables.head != NULL)
167173
{
168174
SimpleStringListCell *cell;
169175

170176
for (cell = tables.head; cell; cell = cell->next)
171177
{
172-
cluster_one_database(dbname, verbose, cell->val,
173-
host, port, username, prompt_password,
174-
progname, echo);
178+
cluster_one_database(&cparams, cell->val,
179+
progname, verbose, echo);
175180
}
176181
}
177182
else
178-
cluster_one_database(dbname, verbose, NULL,
179-
host, port, username, prompt_password,
180-
progname, echo);
183+
cluster_one_database(&cparams, NULL,
184+
progname, verbose, echo);
181185
}
182186

183187
exit(0);
184188
}
185189

186190

187191
static void
188-
cluster_one_database(const char *dbname, bool verbose, const char *table,
189-
const char *host, const char *port,
190-
const char *username, enum trivalue prompt_password,
191-
const char *progname, bool echo)
192+
cluster_one_database(const ConnParams *cparams, const char *table,
193+
const char *progname, bool verbose, bool echo)
192194
{
193195
PQExpBufferData sql;
194196

195197
PGconn *conn;
196198

197-
conn = connectDatabase(dbname, host, port, username, prompt_password,
198-
progname, echo, false, false);
199+
conn = connectDatabase(cparams, progname, echo, false, false);
199200

200201
initPQExpBuffer(&sql);
201202

@@ -226,22 +227,17 @@ cluster_one_database(const char *dbname, bool verbose, const char *table,
226227

227228

228229
static void
229-
cluster_all_databases(bool verbose, const char *maintenance_db,
230-
const char *host, const char *port,
231-
const char *username, enum trivalue prompt_password,
232-
const char *progname, bool echo, bool quiet)
230+
cluster_all_databases(ConnParams *cparams, const char *progname,
231+
bool verbose, bool echo, bool quiet)
233232
{
234233
PGconn *conn;
235234
PGresult *result;
236-
PQExpBufferData connstr;
237235
int i;
238236

239-
conn = connectMaintenanceDatabase(maintenance_db, host, port, username,
240-
prompt_password, progname, echo);
237+
conn = connectMaintenanceDatabase(cparams, progname, echo);
241238
result = executeQuery(conn, "SELECT datname FROM pg_database WHERE datallowconn ORDER BY 1;", progname, echo);
242239
PQfinish(conn);
243240

244-
initPQExpBuffer(&connstr);
245241
for (i = 0; i < PQntuples(result); i++)
246242
{
247243
char *dbname = PQgetvalue(result, i, 0);
@@ -252,15 +248,10 @@ cluster_all_databases(bool verbose, const char *maintenance_db,
252248
fflush(stdout);
253249
}
254250

255-
resetPQExpBuffer(&connstr);
256-
appendPQExpBuffer(&connstr, "dbname=");
257-
appendConnStrVal(&connstr, dbname);
251+
cparams->override_dbname = dbname;
258252

259-
cluster_one_database(connstr.data, verbose, NULL,
260-
host, port, username, prompt_password,
261-
progname, echo);
253+
cluster_one_database(cparams, NULL, progname, verbose, echo);
262254
}
263-
termPQExpBuffer(&connstr);
264255

265256
PQclear(result);
266257
}

0 commit comments

Comments
 (0)