Support for inet data type.

[rikvdkleij]

Implemented Encoder and Decoder for inet PostgreSQL data type.

Created Inet type in Data.hs which has constructors for IPv4 and IPv6, with or without netmask.

Added roundtrip test for Inet in Main.hs of Tasty test.

[rikvdkleij]

@nikita-volkov
When will you review this PR?

[nikita-volkov]

It's released. Thanks for the contribution!

[rikvdkleij]

It's released. Thanks for the contribution!

My pleasure. So you don't have review comments about my PR code 😄 I'm asking this because I want to learn from reviews.

I will also create PR for hasql to support inet.

[nikita-volkov]

Better move this into the where clause of inet for two reasons:

1. This function only exists for inet
2. The API is always cleaner, when all the top-level functions operate on the same level of abstraction

Same applies to inetIPv6FromBytes.

[rikvdkleij]

Okay, thanks, I will change that.

I also noticed that I defined in Data module type Netmask = Word8 but I do not use this in encoder or decoder code itself. It's more documentation but I will change that also so Netmask is used.

[nikita-volkov]

I've reviewed your according "hasql" PR. Thanks for your patience.

At first I didn't pay much attention to this PR, because "postgesql-binary" doesn't have a wide audience and the cost of API-breaking changes is negligible. It's kinda like an internal library. However none of that is the case of the "hasql" library, from which in your PR you reexported the datatypes you defined here. That made me pay closer attention to them and I discovered flaws.

First, there was an obvious redundancy in the Inet definition, so I redefined it like this:

data Inet =
  Inet !IP !Word8

data IP =
  IPv4IP !IPv4 |
  IPv6IP !IPv6

I didn't find any point in encoding the netmask as optional, so I made it permanent.

It's idiomatic to give the public types identity, which is why I redefined the IP types like this:

data IPv4 =
  IPv4 !Word8 !Word8 !Word8 !Word8

data IPv6 =
  IPv6 !Word16 !Word16 !Word16 !Word16 !Word16 !Word16 !Word16 !Word16

Then I thought, what is 4x Word8 if not Word32? Why waste space then? So:

newtype IPv4 =
  IPv4 Word32

data IPv6 =
  IPv6 !Word64 !Word64

Finally, I've almost pushed those changes, but then I've discovered the "network-ip" package, which focuses on exactly our use case and does everything exactly the same way, while providing a lot of extras for the application of those types. So I suggest to migrate to that package. Are you willing to do the update?

[rikvdkleij]

Yes, I will do the update. I already considered to use package for these data types, see nikita-volkov/hasql#68 (comment)

I didn't find any point in encoding the netmask as optional, so I made it permanent.

But I would still like to make distinction between subnet and host. I think in most cases you will store host address and do not want be bothered with netmask. It's your library so let me know if you still do not want to mark netmask as optional.

[nikita-volkov]

I believe that making the netmask optional is overthinking this. Besides, if we migrate to the "network-ip" package, then we only get the NetAddr IP for the Inet type and there's no way of specifying it without the netmask anyway.