Re: Patch: Implement failover on libpq connect level.

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Peter van Hardenberg <pvh(at)pvh(dot)ca>, Peter Eisentraut <peter_e(at)gmx(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch: Implement failover on libpq connect level.
Date: 2016-10-24 20:40:14
Message-ID: 20161024204014.gcu5ne5rbwk5yhva@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-jdbc

Robert Haas wrote:

> While I was experimenting with this today, I discovered a problem of
> interpretation related to IPv6 addresses. Internally, a postgresql://
> URL and a connection string are converted into the same format, so
> postgresql://a,b/ means just the same thing as host=a,b. I thought
> that could similarly decide that postgresql://a:123,b:456/ is going to
> get translated to host=a:123,b:456 and then there can be further code
> to parse that into a list of host-and-port pairs. However, if you do
> that, then something like host=1:2:3::4:5:6 is fundamentally
> ambiguous. That :6 is equally valid either as part of the IP address
> or as a trailing port number specification, and there's no a priori
> way to know which it is. Today, since the host part can't include a
> port specifier, it's regarded as part of the IP address, and I think
> it would probably be a bad idea to change that, as I believe Victor's
> patch would. He seems to have it in mind that we could allow things
> like host=[1:2:3::4:5:6] or host=[1:2:3::4:5]:6, which would might be
> helpful for the future but doesn't avoid changing the meaning of
> connection strings that work today.

Umm, my recollection regarding IPv6 parsing in the URI syntax is that
those must appear inside square brackets -- it's not valid to have the
IPv6 address outside brackets, and the port number is necessarily
outside the brackets. So there's no ambiguity. If the current patch is
allowing IPv6 address to appear outside of brackets, that seems like a
bug to me.

The string conninfo spec should not accept port numbers in the "host"
part. (Does it?)

> So now I think that to make this work correctly, we're going to need
> to change both the URL parser and also add parsing for the host and
> port. Let's say the user says this:
>
> postgresql://[1::2]:3,[4::5],[6::7]::8/

(There's a double colon before 8, I suppose that's just a typo.)

> What I think we need to do is translate that into this:
>
> host=1::2,4::5,6::7 port=3,,8
>
> Note the double-comma, indicating a blank port number for the second
> URL component.

Sounds reasonable.

> And then they might write one where the host and
> port parts don't have the same number of components, like this:
>
> host=a,b,c port=3,4
> or
> host=a,b port=3,4,5
>
> It is obvious what is meant if multiple hosts are given but only a
> single port number is specified; it is also obvious what is meant if
> the number of ports is equal to the number of hosts.

Agreed -- seems like we can accept both those cases.

> It is not obvious what it means if there are multiple ports but the
> number doesn't equal the number of hosts.

I think we should reject the case of differing number of elements and
neither host nor port is a singleton, as an error. The suggestion to
ignore some parts seems too error-prone.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2016-10-24 20:49:20 Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list
Previous Message Jonathan Katz 2016-10-24 20:39:26 Re: Press Release Draft - 2016-10-27 Cumulative Update

Browse pgsql-jdbc by date

  From Date Subject
Next Message Tsunakawa, Takayuki 2016-10-25 03:15:01 Re: [RFC] How about changing the default value of defaultRowFetchSize?
Previous Message Peter Eisentraut 2016-10-24 20:15:41 Re: Patch: Implement failover on libpq connect level.