Re: Mop-up around psql's \connect behavior

At Wed, 21 Oct 2020 18:59:04 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in
> 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.

Sounds reasonable.

> 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.

The reason I haven't complain about this is I don't reconnect by \c
after involuntary disconnection. (That is, C-d then psql again:p) But
once it got on my mind, it might be strange that just \c or \c
-reuse-previous=y doesn't reconnect a broken session. It might be
better we reuse the previous connection parameter even if the
connection has been lost, but this would be another issue.

> * 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.

+1

> * 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.

+1 (I thought sslmode might affect but that is wrong since cert
authenticaion cannot be turned off from command line.)

> 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.

Looks fine.

> Any objections?

Nope from me.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center