Re: Broken SSL tests in master - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Broken SSL tests in master
Date
Msg-id CAB7nPqTti-eP42XOTSnZHj5Ow5HPJztzKK47jbQz35Qyu_jttg@mail.gmail.com
Whole thread Raw
In response to Re: Broken SSL tests in master  (Andreas Karlsson <andreas@proxel.se>)
Responses Re: Broken SSL tests in master  ("Tsunakawa, Takayuki" <tsunakawa.takay@jp.fujitsu.com>)
List pgsql-hackers
On Fri, Nov 25, 2016 at 8:15 AM, Andreas Karlsson <andreas@proxel.se> wrote:
> On 11/24/2016 10:38 PM, Andreas Karlsson wrote:
>> To me it feels like the proper fix would be to make PQHost() return the
>> value of the host parameter rather than the hostaddr (maybe add a new
>> field in the pg_conn_host struct). But would be a behaviour change which
>> might break someones application. Thoughts?

Thanks for digging up the root cause. I can see the problem with HEAD as well.

> I have attached a proof of concept patch for this. Remaining work is
> investigating all the callers of PQhost() and see if any of them are
> negatively affected by this patch and cleaning up the code some.

This needs some thoughts, but first I need to understand the
whereabouts of this refactoring work in 9a1d0af4 as well as the
structures that 274bb2b3 has introduced. From what I can see you are
duplicating some logic parsing the pghost string when there is a
pghostaddr set, so my first guess is that you are breaking some of the
intentions behind this code by patching it this way... I am adding in
CC Robert, Mithun and Tsunakawa-san who worked on that. On my side,
I'll need some time to study and understand this new code, that's
necessary before looking at your patch in details, there could be a
more elegant solution. And we had better address this issue before
looking more into the SSL reload patch.
-- 
Michael



pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: Declarative partitioning - another take
Next
From: Amit Kapila
Date:
Subject: Re: [sqlsmith] Failed assertion in parallel worker in ExecInitSubPlan