Re: [PATCH] pgpassfile connection option

Julian Markwort <julian.markwort@uni-muenster.de>

From: Julian Markwort <julian.markwort@uni-muenster.de>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: PostgreSQL Developers <pgsql-hackers@postgresql.org>
Date: 2016-10-25T13:50:12Z
Lists: pgsql-hackers

Attachments

On 10/16/2016 12:09 PM, Fabien COELHO wrote:
> Patch applies cleanly, make check ok... however AFAICS it only means 
> that it compiles but it is not tested in anyway... This is is 
> annoying. Well I'm not sure whether other options are tested either, 
> but they should. 
Thanks for taking the time to review my patch! I haven't found much 
regarding the testing of connection options. However since there isn't 
anything fancy going on in PasswordFromFile() I'm not too worried about 
that.

> The documentation needs to be updated. 
I've written a couple lines now.
I aligned the definition of the connection option and the environment 
variable with that of other (conn.opt&env.var.) pairs and added mention 
of the different options to the doc of the "Password File".

> The reported password file is wrong. It is even more funny if 
> ~/.pgpass contains the right password: the pgpassfile option *is* 
> taken into account, ./foo is read and it fails, but the error message 
> is not updated and points to the wrong file. The error message stuff 
> should be updated to look for the pgpassfile connection string option... 
That was indeed an Error on my side, I hadn't updated the errormessages 
to inform you which file has been used.

So attached is an updated version of the patch.

I'd like to ask for some input on how to handle invalid files - right 
now no message is shown, the user just gets a password prompt as a 
result, however I think a message when the custom pgpassfile hasn't been 
found would be helpful.

Kind regards,
Julian

--

Julian Markwort
Westphalian Wilhelms-University in Münster
julian(dot)markwort(at)uni-muenster(dot)de

Commits

  1. Allow password file name to be specified as a libpq connection parameter.