Allow HasConversion/ValueConverters to convert nulls

[MarkGodwin]

There are significant problems when executing queries that either convert nulls in the database to non-nulls in code or vice-versa. Therefore, we have marked this feature as internal for EF Core 6.0. You can still use it, but you will get a compiler warning. The warning can be disabled with a #pragma. See #26230 for more information.

---

When using FromSql to create a custom filtered data set, any column conversions defined on the DbQuery object don't get applied.

I think the column conversions should be included automatically around the sub-query supplied to FromSql, similar to how a sub-query is used if more predicates are added to the IQueryable.

    public class Model
    {
        public int Id { get; set; }
        public bool Flag { get; set; }
    }

    public partial class MyDbContext : DbContext
    {
        public DbQuery<Model> Models { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
                    modelBuilder.Query<Models>(
                        entity =>
                        {
                            entity.ToView("dbo.Models");
                            // Convert null Flag to false on way in
                            entity.Property(e => e.Flag).HasConversion<bool?>(f => f, t => t ?? false);
                        });
        }
    }

    // ...

    var models = dbContext.Models.FromSql("SELECT * FROM dbo.Models").ToList();

This, I think, should result in SQL similar to this being executed...

SELECT [e].[Id], COALESCE([e].[Flag], 0) AS [Flag]
FROM (
    SELECT * FROM dbo.Models) AS [e];

But, in reality, the plain SQL from FromSql is executed

SELECT * FROM dbo.Models

EF Core version: 2.1.4
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Windows 7
IDE: Visual Studio 2017 15.8.5

[ajcvickers]

@MarkGodwin Value conversions are not translated to SQL--that feature is tracked by #10861. Instead, the value is read from the database and then converted. This is because the primary purpose of this feature it to allow use of types that cannot be directly read from or written to the database.

Also, value converters do not generally allow the conversion of null to some other value. This is because the same value converter can be used for both nullable and non-nullable types, which is very useful for PK/FK combinations where the FK is often nullable and the PK is not. We will discuss whether or not to add support for converting nulls.

[MarkGodwin]

Sorry, I can see that I am guilty of making assumptions that I didn't verify before raising the issue. I misinterpreted the documentation about where value converters are executed, and then jumped to conclusions about why it wasn't working in my particular case.

The inability to translate null is also very clearly described in the documentation.

I... assume... a correctly implemented value converter would be executed when mapping the results of a FromSql. So I'll close this off immediately.

Apologies again.

[space-alien]

The inability to translate nulls leads to some pretty unintuitive behaviour:

modelBuilder.Entity<Person>()
    .Property(p => p.Address).HasConversion(
        address => JsonConvert.SerializeObject(address),     // Address is a value object
        str => JsonConvert.DeserializeObject<Address>(str));

The above is mapped to a nullable string column in the DB. If Address is null, a NULL is inserted into the row and vice versa. So far so good.

But the results of following query include entities where Address is null!

_context.People.Where(c => c.Address != null).ToList()

[Skulblaka]

I ran into a problem when I tried to add a ValueConverter<float, float?> that converts float.NaN to null and vice versa since SQL Server's real data type does not support NaN. The generated columns for the properties with the converter did not allow null values; I guess this is related?

[ajcvickers]

@Skulblaka Yes.

[UdiAzulay]

any success to use conversion from non null to nullable type? (without excluding the property using NonMapped)

[UdiAzulay]

found an ugly solution using ValueConverter<T, T?>(optionally) and the below reflection field update

prop.Metadata.GetType().GetField("_isNullable", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic).SetValue(prop.Metadata, true);

[AndriySvyryd]

After this is fixed columns with a non-nullable value converter should be marked as non-nullable:

        public static bool IsColumnNullable([NotNull] this IProperty property)
            => !property.IsPrimaryKey()
               && (property.DeclaringEntityType.BaseType != null
                   || (property.IsNullable
                       && property.GetValueConverter()?.ProviderClrType.IsNullableType() != false)
                   || IsTableSplitting(property.DeclaringEntityType));

See #18592 (comment)

Also consider setting the database default to the converted null

[UdiAzulay]

seems that the above suggested ugly workaround only works for "_fastQuery" and fails with "_shapedQuery" so i had to put another ugly solution for that:

    static EntityContext()
    {
        var localMethod = typeof(EntityContext).GetMethod("TryReadValue", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic);
        typeof(Microsoft.EntityFrameworkCore.Metadata.Internal.EntityMaterializerSource).GetField("TryReadValueMethod", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.Public)
            .SetValue(null, localMethod);
    }

    private static TValue TryReadValue<TValue>(in Microsoft.EntityFrameworkCore.Storage.ValueBuffer valueBuffer, int index, Microsoft.EntityFrameworkCore.Metadata.IPropertyBase property)
    {
        var ret = valueBuffer[index];
        if (ret == null && property is Microsoft.EntityFrameworkCore.Metadata.Internal.PropertyBase ip)
            return default(TValue);
        return (TValue)ret;
    }

now add the bellow line for each db null field (which is not null in application level)
prop.Metadata.GetType().GetField("_isNullable", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic).SetValue(prop.Metadata, true);

and the null values will be converted to default(type) in all cases.

please add supported solution for nullable to non nullable conversion for cases like i have (400 poorly designed tables which i am not allowed to change )

[ajcvickers]

See also #22542

[ajcvickers]

For anyone watching this issue: there are significant problems when executing queries that either convert nulls in the database to non-nulls in code or vice-versa. Therefore, we have marked this feature as internal for EF Core 6.0. You can still use it, but you will get a compiler warning. The warning can be disabled with a #pragma. See #26230 for more information.

[SF-Simon]

Hello @ajcvickers , I'm trying to write a process to save any field type with JSON text fields.

However, to be compatible with nullable types, this HasConversion does not support the practice of empty types, and I cannot complete the work.

But I found that IConventionProperty.SetValueConverter is also set value conversion, and it also supports returning null.

            // propertyBuilder.HasConversion(converter);
            propertyBuilder.Metadata.SetValueConverter(converter);
            propertyBuilder.Metadata.SetValueComparer(comparer);

I don't use the A function, I use SetValueConverter and SetValueComparer separately.

I would like to ask, is this a solution? Or what is the difference between them? Is there anything else I need to pay attention to?

the API document:
https://learn.microsoft.com/zh-cn/dotnet/api/microsoft.entityframeworkcore.metadata.imutableproperty.setvalueconverter?view=efcore-7.0

[douglasg14b]

When using strongly typed Ids this is REALLY important. This means we will have null Ids that are meant to be a strongly typed Id that can internally handle nulls.

I hope we seen this soon!!

[douglasg14b]

This post seems to imply that this is possible, but it's internal, but I'm not finding code examples for this internal API? How do we actually achieve this? Is this only through what @SF-Simon showed?

How can this be achieved in ConfigureConventions?

        protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
        {
            configurationBuilder
                .Properties<EventPlatform>()
                .HaveConversion<EventPlatformConverter>();
            
            configurationBuilder
                .Properties<EventPartner>()
                .HaveConversion<EventPartnerConverter>();
        }

public class EventPlatformConverter : ValueConverter<EventPlatform, string?>
{
    public EventPlatformConverter()
        : base(
            v => v.ToString(),
            v => new EventPlatform(v)) 
    { }
}

[ajcvickers]

@douglasg14b The API hasn't changed, it's just marked as internal because converting nulls only works correctly in very limited cases. This is because the database, .NET, and EF Core all treat nulls differently to other values, so if a null is converted then it stops behaving like a null. We may make this a non-internal API if we can find some other way to let people know it is a pit of failure, but it is unlikely we will do any other work in this area.

[chris-pie]

@douglasg14b The API hasn't changed, it's just marked as internal because converting nulls only works correctly in very limited cases. This is because the database, .NET, and EF Core all treat nulls differently to other values, so if a null is converted then it stops behaving like a null. We may make this a non-internal API if we can find some other way to let people know it is a pit of failure, but it is unlikely we will do any other work in this area.

Would you be interested in accepting PRs for this at all or no? I'm wondering if I should try tackling it but if it's not going to get merged anyway I'd rather work on something else.

[ajcvickers]

@chris-pie In the general case, this is a very involved fix that requires understanding how the query pipeline handles nulls and null semantics. Realistically, this would be a big, complex item to undertake for somebody on the team with a very good understanding of how both type mapping and queries work. That being said, if there is something specific you are planning to do, then let us know, and we will discuss.