Mop-up around psql's \connect behavior

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Mop-up around psql's \connect behavior
Date: 2020-10-21 22:59:04
Message-ID: 235210.1603321144@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

While working on commit 85c54287a, I noticed a few things I did not
much care for in do_connect(). These don't quite seem to rise to
the level of back-patchable bugs, but they're still not great:

* The initial stanza that complains about

if (!o_conn && (!dbname || !user || !host || !port))

seems woefully obsolete. In the first place, it's pretty silly
to equate a "complete connection specification" with having just
those four values; the whole point of 85c54287a and predecessors
is that other settings such as sslmode may be just as important.
In the second place, this fails to consider the possibility that
we only have a connstring parameter --- which may nonetheless
provide all the required settings. And in the third place,
this clearly wasn't revisited when we added explicit control of
whether or not we're supposed to re-use parameters from the old
connection. It's very silly to insist on having an o_conn if we're
going to ignore it anyway.

I think the reason we've not had complaints about this is that the
situation normally doesn't arise in interactive sessions (since we
won't release the old connection voluntarily), while scripts are
likely not designed to cope with connection losses anyway. These
facts militate against spending a whole lot of effort on a fix,
but still we ought to reduce the silliness factor. What I propose
is to complain if we have no o_conn *and* we are asked to re-use
parameters from it. Otherwise, it's fine.

* I really don't like the bit about silently ignoring user, host,
and port parameters if we see that the first parameter is a connstring.
That's as user-unfriendly as can be. It should be a syntax error
to specify both; the documentation certainly implies that it is.

* The old-style-syntax code path understands that it should re-use
the old password (if any) when the user, host, and port settings
haven't changed. The connstring code path was too lazy to make
that work, but now that we're deconstructing the connstring there's
very little excuse for not having it act the same way.

The attached patch fixes these things and documents the password
behavior, which for some reason went unmentioned before. Along
the way I simplified the mechanism for re-using a password a bit;
there's no reason to treat it so much differently from re-using
other parameters.

Any objections?

regards, tom lane

Attachment Content-Type Size
minor-fixes-for-psql-connect.patch text/x-diff 6.6 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Anastasia Lubennikova 2020-10-21 22:59:07 Re: Allow some recovery parameters to be changed with reload
Previous Message John Naylor 2020-10-21 22:45:44 Re: speed up unicode decomposition and recomposition