Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes to networking cidr/inet mapping #5136

Closed
roji opened this issue Jun 30, 2023 · 1 comment · Fixed by #5123
Closed

Changes to networking cidr/inet mapping #5136

roji opened this issue Jun 30, 2023 · 1 comment · Fixed by #5123

Comments

@roji
Copy link
Member

roji commented Jun 30, 2023

Npgsql currently PG cidr to .NET ValueTuple<IPAddress, int> by default, and also allows mapping inet to it. There's also an obsolete NpgsqlInet mutable struct. For 8.0, we'll do the following changes:

  • We'll un-obsolete NpgsqlInet, make it an immutable struct and clean it up.
    • IPAddress will remain the default mapping for inet, as that's what most users use it for.
    • Note that as before, reading an inet with a non-full netmask silently truncates the value. This seems reasonable as the primary usage of inet is to store simple host IPs (i.e. IPAddress). If the netmask component is significant, NpgsqlInet mapping can be used instead.
  • We'll also introduce a similar immutable NpgsqlCidr struct, and remove the value tuple mapping.
  • NpgsqlCidr will be the default mapping for cidr, as that type is used to represent networks, where both the IP and netmask components are important.
  • The ValueTuple mapping will be removed.

Some notes on why it makes sense to have separate NpgsqlInet and NpgsqlCidr types for PG inet and cidr:

  • inet can represent a single IP Address, and so should be constructible/castable from one; that's not the case for cidr.
  • inet ToString doesn't output the netmask if it has all bits (32 for IPv4/128 for IPv6). This doesn't really make sense for cidr.
  • Having a separate CLR type for cidr would allow mapping it by default without requiring explicit specification of NpgsqlDbType/DataTypeName.
@roji
Copy link
Member Author

roji commented Jul 3, 2023

Done as part of NinoFloris#2 (comment), which merged into #5123.

@roji roji added this to the 8.0.0 milestone Jul 3, 2023
@NinoFloris NinoFloris mentioned this issue Aug 15, 2023
8 tasks
@roji roji changed the title Changes to cidr/inet mapping Changes to networking cidr/inet mapping Oct 11, 2023
@Brar Brar assigned NinoFloris and unassigned roji Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants