Re: WIP: Fix parallel workers connection bug in pg_dump (Bug #13727)

<p dir="ltr"><br /> On Nov 1, 2015 5:04 PM, "Marko Tiikkaja" <<a href="mailto:marko@joh.to">marko@joh.to</a>>
wrote:<br/> ><br /> > On 10/25/15 10:55 PM, Zeus Kronion wrote:<br /> >><br /> >> Parallel workers
werefailing to connect to the database when running<br /> >> pg_dump with a connection string. The first of the
followingtwo commands<br /> >> runs without errors, while the second one fails:<br /> >> pg_dump
"postgres://<a
href="http://my-user:my-password@my.hostname.com:5432/my-db">my-user:my-password@my.hostname.com:5432/my-db</a>"-Fd
-f<br/> >> my-dump<br /> >> pg_dump "postgres://<a
href="http://my-user:my-password@my.hostname.com:5432/my-db">my-user:my-password@my.hostname.com:5432/my-db</a>"-Fd<br
/>>> --jobs=9 -f my-dump<br /> >><br /> >> The error message:<br /> >> pg_dump: [parallel
archiver]connection to database "my-db" failed:<br /> >> fe_sendauth: no password supplied<br /> >><br />
>>The password is not being stored correctly in the PGconn object when<br /> >> connecting with a
connectionstring.<br /> ><br /> ><br /> > Yeah, the current code is definitely broken for this case.  However,
Idon't feel like this patch is quite there yet, either.  _connectDB has similar logic in it which might be hit in case
e.g.a a user's HBA is changed from a non-password-requiring method to a password-requiring one after the one or more
connectionshas been initiated.  That one needs changing as well.<p dir="ltr">I wasn't aware of that case. Should be an
easyfix to make this weekend.<p dir="ltr">> However, I don't quite like the way the password cache is kept up to
datein the old *or* the new code.  It seems to me that it should instead look like:<br /> ><br /> >    if
(PQconnectionUsedPassword(AH->connection))<br/> >        AH->savedPassword = PQpass(AH->connection);<br />
><br/> > What do you think?<br /><br /> I don't understand why this logic is preferable. Is your concern that
AH->savedPasswordmay contain a password even when none is needed? Or is the change simply meant to give the reader a
bettersense of what is actually going on?<p dir="ltr">-CS<br />