Joe Conway <m...@joeconway.com> writes:
> The attached patch set moves the guts of \password from psql into the
> libpq client side -- PQchangePassword() (patch 0001).
Haven't really read the patch, just looked at the docs, but here's
a bit of bikeshedding:
* This seems way too eager to promote the use of md5. Surely the
default ought to be SCRAM, full stop. I question whether we even
need an algorithm parameter. Perhaps it's a good idea for
future-proofing, but we could also plan that the function would
make its own decisions based on noting the server's version.
(libpq is far more likely to be au courant about what to do than
the calling application, IMO.)
* Parameter order seems a bit odd: to me it'd be natural to write
user before password.
* Docs claim return type is char *, but then say bool (which is
also what the code says). We do not use bool in libpq's API;
the admittedly-hoary convention is to use int with 1/0 values.
Rather than quibble about that, though, I wonder if we should
make the result be the PGresult from the command, which the
caller would be responsible to free. That would document error
conditions much more flexibly. On the downside, client-side
errors would be harder to report, particularly OOM, but I think
we have solutions for that in existing functions.
* The whole business of nonblock mode seems fairly messy here,
and I wonder whether it's worth the trouble to support. If we
do want to claim it works then it ought to be tested.
> One thing I have not done but, considered, is adding an additional
> optional parameter to allow "VALID UNTIL" to be set. Seems like it would
> be useful to be able to set an expiration when setting a new password.
No strong opinion about that.
regards, tom lane
