Instead of immediately PQfinish'ing a dead connection, save it aside
so that we can still extract its parameters for \connect attempts.
(This works because PQconninfo doesn't care whether the PGconn is in
CONNECTION_BAD state.) This allows developers to reconnect with
just \c after a database crash and restart.
It's tempting to use the same approach instead of closing the old
connection after a failed non-interactive \connect command. However,
that would not be very safe: consider a script containing
\c db1 user1 live_server
\c db2 user2 dead_server
\c db3
The script would be expecting to connect to db3 at dead_server, but
if we re-use parameters from the first connection then it might
successfully connect to db3 at live_server. This'd defeat the goal
of not letting a script accidentally execute commands against the
wrong database.
Discussion: https://postgr.es/m/38464.
1603394584@sss.pgh.pa.us
connection is closed.
If the connection attempt fails (wrong user name, access
denied, etc.), the previous connection will be kept if
- <application>psql</application> is in interactive mode. But when
- executing a non-interactive script, processing will
- immediately stop with an error. This distinction was chosen as
+ <application>psql</application> is in interactive mode. But when
+ executing a non-interactive script, the old connection is closed
+ and an error is reported. That may or may not terminate the
+ script; if it does not, all database-accessing commands will fail
+ until another <literal>\connect</literal> command is successfully
+ executed. This distinction was chosen as
a user convenience against typos on the one hand, and a safety
mechanism that scripts are not accidentally acting on the
wrong database on the other hand.
+ Note that whenever a <literal>\connect</literal> command attempts
+ to re-use parameters, the values re-used are those of the last
+ successful connection, not of any failed attempts made subsequently.
+ However, in the case of a
+ non-interactive <literal>\connect</literal> failure, no parameters
+ are allowed to be re-used later, since the script would likely be
+ expecting the values from the failed <literal>\connect</literal>
+ to be re-used.
</para>
<para>
break;
}
- /*
- * If we are to re-use parameters, there'd better be an old connection to
- * get them from.
- */
- if (reuse_previous && !o_conn)
- {
- pg_log_error("No database connection exists to re-use parameters from");
- return false;
- }
-
/*
* 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.
+ * old connection, then replace individual values as necessary. (We may
+ * need to resort to looking at pset.dead_conn, if the connection died
+ * previously.) 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);
+ {
+ if (o_conn)
+ cinfo = PQconninfo(o_conn);
+ else if (pset.dead_conn)
+ cinfo = PQconninfo(pset.dead_conn);
+ else
+ {
+ /* This is reachable after a non-interactive \connect failure */
+ pg_log_error("No database connection exists to re-use parameters from");
+ return false;
+ }
+ }
else
cinfo = PQconndefaults();
if (o_conn)
{
/*
- * Transition to having no connection. Keep this bit in sync
- * with CheckConnection().
+ * Transition to having no connection.
+ *
+ * Unlike CheckConnection(), we close the old connection
+ * immediately to prevent its parameters from being re-used.
+ * This is so that a script cannot accidentally reuse
+ * parameters it did not expect to. Otherwise, the state
+ * cleanup should be the same as in CheckConnection().
*/
PQfinish(o_conn);
pset.db = NULL;
ResetCancelConn();
UnsyncVariables();
}
+
+ /* On the same reasoning, release any dead_conn to prevent reuse */
+ if (pset.dead_conn)
+ {
+ PQfinish(pset.dead_conn);
+ pset.dead_conn = NULL;
+ }
}
return false;
PQdb(pset.db), PQuser(pset.db));
}
+ /* Drop no-longer-needed connection(s) */
if (o_conn)
PQfinish(o_conn);
+ if (pset.dead_conn)
+ {
+ PQfinish(pset.dead_conn);
+ pset.dead_conn = NULL;
+ }
+
return true;
}
fprintf(stderr, _("Failed.\n"));
/*
- * Transition to having no connection. Keep this bit in sync with
- * do_connect().
+ * Transition to having no connection; but stash away the failed
+ * connection so that we can still refer to its parameters in a
+ * later \connect attempt. Keep the state cleanup here in sync
+ * with do_connect().
*/
- PQfinish(pset.db);
+ if (pset.dead_conn)
+ PQfinish(pset.dead_conn);
+ pset.dead_conn = pset.db;
pset.db = NULL;
ResetCancelConn();
UnsyncVariables();
/* Show the stats target if it's not default */
if (strcmp(PQgetvalue(result, i, 8), "-1") != 0)
appendPQExpBuffer(&buf, "; STATISTICS %s",
- PQgetvalue(result, i, 8));
+ PQgetvalue(result, i, 8));
printTableAddFooter(&cont, buf.data);
}
VariableSpace vars; /* "shell variable" repository */
+ /*
+ * If we get a connection failure, the now-unusable PGconn is stashed here
+ * until we can successfully reconnect. Never attempt to do anything with
+ * this PGconn except extract parameters for a \connect attempt.
+ */
+ PGconn *dead_conn; /* previous connection to backend */
+
/*
* The remaining fields are set by assign hooks associated with entries in
* "vars". They should not be set directly except by those hook
pset.progname = get_progname(argv[0]);
pset.db = NULL;
+ pset.dead_conn = NULL;
setDecimalLocale();
pset.encoding = PQenv2encoding();
pset.queryFout = stdout;
/* clean up */
if (pset.logfile)
fclose(pset.logfile);
- PQfinish(pset.db);
+ if (pset.db)
+ PQfinish(pset.db);
+ if (pset.dead_conn)
+ PQfinish(pset.dead_conn);
setQFout(NULL);
return successResult;
projects / postgresql.git / commitdiff