Re: Patch for SortSupport implementation on inet/cdir

Поиск
Список
Период
Сортировка
От Brandur Leach
Тема Re: Patch for SortSupport implementation on inet/cdir
Дата
Msg-id CABR_9B8hCZKBNzvAcKt8sjVHWJnPbogOuJLB=ybyYRXmosMU3g@mail.gmail.com
обсуждение исходный текст
Ответ на Re: Patch for SortSupport implementation on inet/cdir  (Peter Geoghegan <pg@bowt.ie>)
Ответы Re: Patch for SortSupport implementation on inet/cdir  (Peter Geoghegan <pg@bowt.ie>)
Список pgsql-hackers
Thanks Peter. V6 is pretty uncontroversial by me — the new
conditional ladder broken cleanly into cases of (1) all
subnet, (2) network/subnet mix, and (3) all network is a
little more verbose, but all in all makes things easier to
reason about.

> Do you have any final input on the testing, Brandur? I
> would like to hear your thoughts on the possibility of
> edge cases that still don't have coverage. The tests will
> break if the new "if (ip_bits(authoritative) == 0)"
> branch is removed, but only at one exact point. I'm
> pretty sure that there are no remaining subtleties like
> that one, but two heads are better than one.

(Discussed a little offline already, but) the new, more
exhaustive test suite combined with your approach of
synthesizing many random values and comparing before and
after sorting seems sufficient to me. The approach was
meant to suss out any remaining edge cases, and seems to
have done its job.

Brandur

On Wed, Jul 31, 2019 at 10:49 AM Peter Geoghegan <pg@bowt.ie> wrote:
On Fri, Jul 26, 2019 at 7:25 PM Peter Geoghegan <pg@bowt.ie> wrote:
> I guess that the idea here was to prevent masking on ipv6 addresses,
> though not on ipv4 addresses. Obviously we're only dealing with a
> prefix with ipv6 addresses, whereas we usually have the whole raw
> ipaddr with ipv4. Not sure if I'm doing the right thing there in v3,
> even though the tests pass. In any case, this will need to be a lot
> clearer in the final version.

This turned out to be borked for certain IPv6 cases, as suspected.
Attached is a revised v6, which fixes the issue by adding the explicit
handling needed when ipaddr_datum is just a prefix of the full ipaddr
from the authoritative representation. Also made sure that the tests
will catch issues like this. Separately, it occurred to me that it's
probably not okay to do straight type punning of the ipaddr unsigned
char array to a Datum on alignment-picky platforms. Using a memcpy()
seems like the right approach, which is what we do in the latest
revision.

I accepted almost all of Brandur's comment revisions from v5 for v6.

I'm probably going to commit this tomorrow morning Pacific time. Do
you have any final input on the testing, Brandur? I would like to hear
your thoughts on the possibility of edge cases that still don't have
coverage. The tests will break if the new "if (ip_bits(authoritative)
== 0)" branch is removed, but only at one exact point. I'm pretty sure
that there are no remaining subtleties like that one, but two heads
are better than one.

--
Peter Geoghegan

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Julien Rouhaud
Дата:
Сообщение: Re: Avoid full GIN index scan when possible
Следующее
От: Robert Haas
Дата:
Сообщение: Re: tableam vs. TOAST