On Fri, Nov 6, 2015 at 12:23 AM, Marko Tiikkaja <marko(at)joh(dot)to> wrote:
> On 11/5/15 4:11 PM, Zeus Kronion wrote:
>>
>> On Nov 1, 2015 5:04 PM, "Marko Tiikkaja" <marko(at)joh(dot)to> wrote:
>>>
>>> However, I don't quite like the way the password cache is kept up to date
>>
>> in the old *or* the new code. It seems to me that it should instead look
>> like:
>>>
>>>
>>> if (PQconnectionUsedPassword(AH->connection))
>>> AH->savedPassword = PQpass(AH->connection);
>>>
>>> What do you think?
>>
>>
>> I don't understand why this logic is preferable. Is your concern that
>> AH->savedPassword may contain a password even when none is needed?
>
>
> The opposite, sort of. If the first connection uses a password, the second
> one doesn't, and the third one does again, you need to ask for a password
> again because you emptied the cache on the second attempt since it didn't
> use a password. Granted, this situation is quite unlikely to occur in
> practice, but I find the "correct" code *also* more readable. To me it reads
> like "if the what we're caching was applied during the connection attempt,
> update the cache; otherwise keep the previous value in case it's useful in
> the future".
OK, I think that we had better address this bug for this CF. I am
attaching an updated patch following Marko's suggestion, and marking
this patch as ready for committer. I have noticed that the fix was
actually not complete: ConnectDatabase needs a similar fix, this is a
code path when a connection string like "dbname=db user=foo" is used.
And this was failing as well when the password is passed directly in
the connection string for multiple jobs.
--
Michael