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

Unable to map inet column to (IPAddress, int) tuple #1158

Closed
markron opened this issue Dec 9, 2019 · 2 comments · May be fixed by #2698
Closed

Unable to map inet column to (IPAddress, int) tuple #1158

markron opened this issue Dec 9, 2019 · 2 comments · May be fixed by #2698

Comments

@markron
Copy link

markron commented Dec 9, 2019

I need to store an IP address along with its netmask in an inet column.
I would like to map the value of the column to a tuple of type (IPAddress, int), as I already do with columns of type cidr.

I tried to explicitly specify the column type, as in the following example:

class Entity
{
	// ...
	[Column(TypeName = "inet")]
	public (IPAddress, int) Address { get; set; }
	// ...
}

However, adding the migration fails with the error The property 'Entity.Address' is of type 'ValueTuple<IPAddress, int>' which is not supported by current database provider. Either change the property CLR type or ignore the property using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.

The cause seems to be that a RelationalTypeMapping for the mapping between inet and a tuple is not defined.
The only RelationalTypeMapping that is defined for inet columns and configured in NpgsqlTypeMappingSource is for the mapping between inet and IPAddress.

I currently resolved with a custom type mapping source and an additional type mapper:

public class CustomNpgsqlTypeMappingSource : NpgsqlTypeMappingSource
{
    public CustomNpgsqlTypeMappingSource(
            TypeMappingSourceDependencies dependencies,
            RelationalTypeMappingSourceDependencies relationalDependencies,
            ISqlGenerationHelper sqlGenerationHelper,
            INpgsqlOptions npgsqlOptions = null)
            : base(dependencies, relationalDependencies, sqlGenerationHelper, npgsqlOptions)
    {
        StoreTypeMappings["inet"] =
            new RelationalTypeMapping[] {
                new NpgsqlInetWithMaskTypeMapping(),
                new NpgsqlInetTypeMapping() };
    }
}

// Basically copied from NpgsqlCidrTypeMapping
public class NpgsqlInetWithMaskTypeMapping : NpgsqlTypeMapping
{
    public NpgsqlInetWithMaskTypeMapping() : base("inet", typeof((IPAddress, int)), NpgsqlDbType.Inet) {}

    protected NpgsqlInetWithMaskTypeMapping(RelationalTypeMappingParameters parameters)
        : base(parameters, NpgsqlDbType.Inet) {}

    protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
        => new NpgsqlInetWithMaskTypeMapping(parameters);

    protected override string GenerateNonNullSqlLiteral(object value)
    {
        var cidr = ((IPAddress Address, int Subnet))value;
        return $"INET '{cidr.Address}/{cidr.Subnet}'";
    }

    public override Expression GenerateCodeLiteral(object value)
    {
        var cidr = ((IPAddress Address, int Subnet))value;
        return Expression.New(
            Constructor,
            Expression.Call(ParseMethod, Expression.Constant(cidr.Address.ToString())),
            Expression.Constant(cidr.Subnet));
    }

    static readonly MethodInfo ParseMethod = typeof(IPAddress).GetMethod("Parse", new[] { typeof(string) });

    static readonly ConstructorInfo Constructor =
        typeof((IPAddress, int)).GetConstructor(new[] { typeof(IPAddress), typeof(int) });
}

and in Startup:

services.AddDbContext<MyContext>(c => {
                c.UseNpgsql(Configuration.GetConnectionString("Default"));
                c.ReplaceService<IRelationalTypeMappingSource, CustomNpgsqlTypeMappingSource>();
            });

In this way, a property with type (IPAddress, int) is correctly mapped to the inet column type when an attribute [Column(TypeName = "inet")] is specified.

Can I try to make a PR with this additional type mapper defined and configured in NpgsqlTypeMappingSource? Or are there some drawbacks that I have not considered?

@roji
Copy link
Member

roji commented Dec 9, 2019

Good catch, we currently map (IPAddress, int) only to the PostgreSQL cidr type, which can only represent networks (unlike inet which can represent both hosts and networks). I think it makes sense to allow this.

Sure, feel free to submit a PR adding support for this. To avoid duplication, this should be done by making the existing NpgsqlInetTypeMapping more flexible, and probably merging it with the existing NpgsqlCidrTypeMapping into a new merged mapping which would support IPAddress->inet, (IPAddress, int)->inet and (IPAddress, int)->cidr. The default for (IPAddress, int) should stay cidr to avoid a breaking change on this.

@roji roji added this to the Backlog milestone May 24, 2020
PJB3005 added a commit to PJB3005/efcore.pg that referenced this issue Mar 19, 2023
Fixes npgsql#1158

Unified cidr and inet handling code since they're similar.
@roji roji modified the milestones: Backlog, 8.0.0 Sep 23, 2023
@roji
Copy link
Member

roji commented Nov 13, 2023

The mapping of inet/cidr as .NET value tuples is being removed in 8.0; instead, simply NpgsqlInet and NpgsqlCidr structs can be used. The work to support these has already been done (see #2882 and npgsql/npgsql#5136).

@roji roji closed this as not planned Won't fix, can't repro, duplicate, stale Nov 13, 2023
@roji roji removed this from the 8.0.0 milestone Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants