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

Allow different types for FK/PK if a value converters to compatible provider types are used #22542

Open
Tracked by #240
alc86 opened this issue Sep 15, 2020 · 4 comments

Comments

@alc86
Copy link

alc86 commented Sep 15, 2020

We use a custom ValueConverter to map our Undefinable type into the database relevant type. This works as expected as long as TValue is not a reference or nullable type.

Currently EF Core generates two database columns for every reference or nullable type one is our defined and there is an additional shadow foreign key property.

Steps to reproduce

We have an Undefinable struct which tells us if the client provide a value for a property

public struct Undefinable<TValue>
{
    public Undefinable(TValue value)
    {
        IsDefined = true;
        Value = value;
    }

    public bool IsDefined { get; }

    public TValue Value { get; }
}

Using this ValueConverter EF Core uses the correct database type. (This works as expected as long as TValue is not a reference or nullable type)

public class UndefinableConverter<TValue> : ValueConverter<Undefinable<TValue>, TValue>
{
    public UndefinableConverter()
        : base(undefinable => undefinable.IsDefined ? undefinable.Value : default, value => new Undefinable<TValue>(value))
    {
    }
}

Look at the following relationsship

public class Person
{
    [Key]
    public Undefinable<Guid> Id { get; set; }

    public Undefinable<Guid?> AddressId { get; set; }

    public Address Address { get; set; }
}

public class Address
{
    [Key]
    public Undefinable<Guid> Id { get; set; }
}

At the DBContext we use the following snippet to add the UndefinableConverter

foreach (var property in properties)
{
    var propertyType = property.PropertyType;

    var underlyingType = UndefinableHelper.GetUnderlyingType(propertyType);

    if (underlyingType != null)
    {
        var valueConverter = (ValueConverter)Activator.CreateInstance(typeof(UndefinableConverter<>).MakeGenericType(underlyingType));
        entityTypeBuilder.Property(property.Name).HasConversion(valueConverter);
    }
}

EF Core creates a migration with two columns for the Person AddressId:

AddressId = table.Column<Guid>(nullable: false),
AddressId1 = table.Column<Guid>(nullable: true),

Further technical details

EF Core version: 3.1.7
Database provider: Npgsql.EntityFrameworkCore.PostgreSQL (3.1.4)
Target framework: .NET Core 3.1
IDE: Visual Studio 2019 16.3

@ajcvickers
Copy link
Member

Note for triage: this fails because the type of the FK is Undefinable<Nullable<Guid>>, while the PK is Undefinable<Guid>. We tallow nullable FKs against non-nullable PK by special handling of nullable types. Generalizing this could get tricky.

Model (EF Core 5.0):

Model: 
  EntityType: Address
    Properties: 
      Id (Undefinable<Guid>) Required PK AfterSave:Throw
    Keys: 
      Id PK
  EntityType: Person
    Properties: 
      Id (Undefinable<Guid>) Required PK AfterSave:Throw
      AddressId (Undefinable<Nullable<Guid>>) Required
      AddressId1 (no field, Nullable<Undefinable<Guid>>) Shadow FK Index
    Navigations: 
      Address (Address) ToPrincipal Address
    Keys: 
      Id PK
    Foreign keys: 
      Person {'AddressId1'} -> Address {'Id'} ToPrincipal: Address ClientSetNull
    Indexes: 
      AddressId1 

@alc86
Copy link
Author

alc86 commented Sep 15, 2020

I there any possibility to modify the special handling nullable FK against non-nullable PK?
In our case ist would be enough to use the TValue type of the Undefinable struct. This is a urgent problem from us which we need to solve

@ajcvickers
Copy link
Member

@alc86 I am not aware of any way to make this work.

@ajcvickers ajcvickers changed the title The provider type of the ValueConverter is not used for relationships Allow different types for FK/PK if a value converters to compatible provider types are used Sep 18, 2020
@ajcvickers
Copy link
Member

Related to #13850

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants