|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
|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|