Re: [PATCH] pgpassfile connection option

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: Julian Markwort <julian(dot)markwort(at)uni-muenster(dot)de>, Mithun Cy <mithun(dot)cy(at)enterprisedb(dot)com>, PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] pgpassfile connection option
Date: 2017-01-24 22:17:36
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> writes:
> I've switch in the CF to "ready for committer", and we'll see what the
> next level thinks about it:-)

Pushed with a few adjustments:

* It seemed to me that the appropriate parameter name is "passfile"
not "pgpassfile". In all but a couple of legacy cases, the environment
variable's name is the parameter name plus a PG prefix; therefore,
with the environment variable already named PGPASSFILE, we have to use
"passfile" for consistency. Putting a PG prefix on a connection parameter
name is rather pointless anyway, since there's no larger namespace that
it might have a conflict in.

* The error handling in the submitted patch was pretty shoddy; it didn't
check for malloc failure at all, and it didn't report a suitable error on
pqGetHomeDirectory failure either (which is worth doing, if it's going to
be a reason for connection failure). I ended up concluding that the
easiest way to fix that involved just inlining getPgPassFilename into the
one remaining call site.

* I also got rid of the assignment of DefaultPassword to conn->pgpass;
that wasn't doing anything very useful anymore. AFAICS, the only visible
effect was to make PQpass() return an empty string not NULL, but we can
make that happen without paying a malloc/free cycle for it.

* Minor comment and docs cleanups.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2017-01-24 22:24:36 Re: [COMMITTERS] pgsql: Add pg_sequence system catalog
Previous Message Fujii Masao 2017-01-24 22:05:34 Re: Re: [bug fix] Cascading standby cannot catch up and get stuck emitting the same message repeatedly