sql: Add INET column type and IPAddr datum

[lgo]

This is an implementation for the INET postgres column type, found at Network Address Types. Related to #6981. At the moment this PR does not contain functions or operators found on Network Address Functions and Operators, but this is being done in another PR.

I've added pkg/util/ipaddr/ipaddr.go to provide utility functions as a wrapper around net.IPNet. This includes marshaling to/from binary and parsing INET addresses (and in the future parsing CIDR).

The parsing functions were made as Go's net.ParseIP and net.ParseCIDR don't suffice for postgres compatibility. Postgres INET values can have partial octets and a trailing .. For example these values are valid,

192.168/16
196.168./16
192.168.0.1.

Another key factor is that under Go an IPv4-mapped IPv6 address is equal to it's mapped IPv4 address, while in postgres this is not the case. This presents a problem, so we need to differentiate between the families explicitly. This is also important for ordering properties seen below.

Go

ip1 := net.ParseIP("127.0.0.1")
ip2 := net.ParseIP("::ffff:127.0.0.1")
ip1.Equal(ip2)
> true

Postgres

select '::ffff:127.0.0.1'::inet = '127.0.0.1'::inet;
----------
 f

select * from test order by ip;
-----------------------
190.0.0.0/1
192.168.0.0/1
192.168.0.1/1
192.168.0.1/30
192.168.0.1
192.168.0.2
::ffff:192.168.0.1/29
::ffff:192.168.0.1/30
::
::1
::f
::ffff:192.168.0.1
f:f:f:f:f:f:f:f

Encoding

The encoding scheme is

Family	Mask	IP
1-byte	1-byte	{4,16}-byte

The family can be 4 or 6, and the mask can be 1-32 for IPv4, or 1-128 for IPv6.

If the IP is IPv6, it will be 16-bytes. This falls close to Postgres, where they will use 1 additional byte in total[1].

[1]: I've yet to figure out why they have one extra byte. See their Network Address Types

[cockroach-teamcity]

This change is 

[benesch]

Re [1]: https://github.com/postgres/postgres/blob/c7b8998ebbf310a156aa38022555a24d98fdbfb4/src/include/utils/inet.h#L42-L51

[knz]

You also need to test the comparison of '192.168.0.2/24'::inet < '192.168.0.1/25'::inet. It's not trivial and motivates why the mask should be encoded before the address for ordering.

[lgo]

Yep, those comparisons are tested at a few levels but there aren't too many in the logic tests (or they are incorrect). I haven't made the logic tests thorough because storing and retrieving the values isn't working.

[lgo]

Also a follow up @knz. After adding the IPNet datum, how easy will it be to add the CIDR column type to use this also? I noticed the issue #8318 which talks about "compound types". Would the lack of any true compound type make it more difficult to add CIDR and re-use this code?

[knz]

CIDR has nothing to do with compound types. I think what you should be looking at to find something to reuse is the difference between DName and DString.

[knz]

Also, talk to Jordan.

[lgo]

@jordanlewis @knz please take a look :). This PR is done and working 🎉 . I've left comments in for places to add the CIDR column, which I can remove before merging if desired as I'll defer that to another PR.

A few questions I had:

• Where can I find specs on pgwire? I was uncertain about the pgwire encoding/decoding, so that's noted in the PR and currently stubbed out with the internal binary encode/decode.
• What is Datum.Prev and Datum.Next used for? Do you think they will be needed for INET/CIDR columns? Some basic logic is there but commented out as the definition is ambiguous when you consider different family and mask values.

[benesch]

@LEGO Postgres's wire protocol is documented in Chapter 51 of the PostgreSQL docs. The morpheme "pgwire" is a Cockroachism, which is why Googling it is so useless. The rest of the internet calls pgwire the "Postgres protocol" or similar.

All that said, the Postgres docs will usually just direct you to the source to determine the encoding for a particular datum.

The text representation of values is whatever strings are produced and accepted by the input/output conversion functions for the particular data type. In the transmitted representation, there is no trailing null character; the frontend must add one to received values if it wants to process them as C strings. (The text format does not allow embedded nulls, by the way.)

Binary representations for integers use network byte order (most significant byte first). For other data types consult the documentation or source code to learn about the binary representation. Keep in mind that binary representations for complex data types might change across server versions; the text format is usually the more portable choice.

https://www.postgresql.org/docs/current/static/protocol-overview.html

I think the file you want to look at here is https://github.com/postgres/postgres/blob/81c5e46c490e2426db243eada186995da5bb0ba7/src/backend/utils/adt/network.c.

[lgo]

Was that the pg_catalog failure? It was the type length changed during the net.IP to uint128 change, all of the other logic tests are now passing :)

---

Review status: 44 of 48 files reviewed at latest revision, 5 unresolved discussions.

---

c-deps/rocksdb, line 1 at r9 (raw file):

Previously, knz (kena) wrote…

I don't see any change.

Okay, got it after the gitmodules were being tricky :)

(It keeps auto-changing even when it was reset, but the proper one was getting staged and the other wasn't but I kept adding it.

---

pkg/cli/dump_test.go, line 61 at r11 (raw file):

Previously, knz (kena) wrote…

duplicate column name

Done.

---

Comments from Reviewable

[knz]

nice work!

---

Reviewed 4 of 4 files at r12.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

Comments from Reviewable

[knz]

you forgot to run build/builder.sh make generate

---

Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

Comments from Reviewable

[lgo]

👍 thanks. Also realized where the rowsort fix is needed. Churning through the final test failures then it should be good :)

---

Review status: 47 of 48 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[lgo]

Ah something I left as a TODO, currently, the encoding is using 16-bytes for an IPv4 address. I'm going to swap that back to being more optimal for IPv4. I dropped that when I first made the switch to uint128.

I reckon it's no easy matter to change this after the initial feature.

---

Review status: 42 of 48 files reviewed at latest revision, 3 unresolved discussions, all commit checks successful.

---

Comments from Reviewable

[knz]

Okay let's do that. Let me know when it's ready.

[lgo]

👍 just pushed it up

---

Review status: 37 of 48 files reviewed at latest revision, 3 unresolved discussions, some commit checks pending.

---

Comments from Reviewable

[knz]

Reviewed 2 of 21 files at r5, 11 of 11 files at r13.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks pending.

---

pkg/util/ipaddr/ipaddr.go, line 65 at r13 (raw file):

const IPv6size = net.IPv6len + 2

// IPv4mappedIPv6prefix is the byte prefix for IPv4-mapped IPv6

Period missing at end of sentence.

---

pkg/util/ipaddr/ipaddr.go, line 73 at r13 (raw file):

	appendTo = append(appendTo, byte(ipAddr.Family), ipAddr.Mask)
	if ipAddr.Family == IPv4family {
		var tmp [4]byte

appendTo = append(appendTo, 0, 0, 0, 0)
binary.BigEndian.PutUint32(tmp[len(tmp)-5:len(tmp)], ...)

---

pkg/util/ipaddr/ipaddr.go, line 78 at r13 (raw file):

	} else {
		var tmp [16]byte
		binary.BigEndian.PutUint64(tmp[:8], ipAddr.Addr.Hi)

ditto

no need to copy the data twice.

---

Comments from Reviewable

[lgo]

Review status: 47 of 48 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

pkg/util/ipaddr/ipaddr.go, line 65 at r13 (raw file):

Previously, knz (kena) wrote…

Period missing at end of sentence.

Done.

---

pkg/util/ipaddr/ipaddr.go, line 73 at r13 (raw file):

binary.BigEndian.PutUint32(tmp[len(tmp)-5:len(tmp)], ...)
Right, so this ensures that the slice has enough space immediately(and/or allocates it)? I was looking into pre-allocating space in the slice via. a new make and copy but that was obviously not ideal, so this is good :)

---

pkg/util/ipaddr/ipaddr.go, line 78 at r13 (raw file):

Previously, knz (kena) wrote…

ditto

no need to copy the data twice.

Done.

---

Comments from Reviewable

[knz]

thanks 🍻

---

Reviewed 1 of 21 files at r5, 1 of 1 files at r14.
Review status: all files reviewed at latest revision, 4 unresolved discussions, all commit checks successful.

---

pkg/util/ipaddr/ipaddr.go, line 73 at r13 (raw file):

Previously, lego (Joey Pereira) wrote…

binary.BigEndian.PutUint32(tmp[len(tmp)-5:len(tmp)], ...)
Right, so this ensures that the slice has enough space immediately(and/or allocates it)? I was looking into pre-allocating space in the slice via. a new make and copy but that was obviously not ideal, so this is good :)

Yes append of these zeros will allocate the extra space.

---

Comments from Reviewable