Support different SQL translation when store representation has changed due to value conversion

[ajcvickers]

Consider cases like #10427 which involves SQL translations for DateTime calculations. If the property has been mapped to some non-datetime type, then this kind of translation will not work anymore. Instead, query should either:

• Do a different translation
• Revert to client evaluation

Probably client eval is what we can realistically do in the short term, but in the future it would be good to have the type mapping/value conversion participate in generating the translations.

[ajcvickers]

Triage: client eval for now.

[ajcvickers]

Triage: For 2.1, we will:

• Detect places where query is going to use a converted type in a place "that matters"--for example, Where clause--and output a warning.
• The warning will be for every operation other than equality/inequality
• The warning will direct people to consider client eval

For 2.1, we will not:

• Force client eval in these cases--for example, forcing client eval on all ordering would be bad since often only a stable ordering is needed and forcing client eval would kill perf
• Try to add API around this area since there are many considerations and there is not enough time to bake it for 2.1. See #10265

[ajcvickers]

See also #11156

[anpete]

Warning is done. Clearing milestone to track further improvements.

[michaldudak]

What about equality/inequality in Where clause? It seems that when there is a converted type in a Where clause (something like .Where(b => b.BlogId == new BlogId("8807e48e-1236-4ef0-a8c9-3de730e52e1b") in my case, where BlogId is a struct with a type conversion to/from a Guid) the client-side evaluation kicks in. So in this case, all Blogs would get fetched from the database, which is far from acceptable.

[divega]

@ajcvickers @bricelam If this is necessary for spatial, I think we missed that spatial is in 2.2 and this in 3.0.

[bricelam]

I actually don't think this is necessary, but we'll pull in parts of it if needed

[bricelam]

To elaborate, there will just be one mapping of IGeometry, and while it may go through some kind of conversion, the translations will always make sense after the conversion.

This issue is for when the translations no longer make sense after conversion.

[divega]

Thanks @bricelam. I was confusing this issue with the general ability to push a type conversion down to SQL, e.g. in the case of spatial for SQL Server, to get the WKT or WKB representation, so that types like sqlgeography and sqlgeometry that cannot be represented in Core CLR are not present in the results. Do you know if we are tracking this elsewhere?

[ajcvickers]

@divega I think #10861 is the issue you were looking for. It's currently in backlog.

[smitpatel]

~Blocked in #13192~

This issue is about making translation depend on StoreType regardless of what is the CLR type and function used on client.

[MovGP0]

I found that in my codebases there are only a few use cases. They are replicated thousands(!) of times, but EF can't handle them. Most importantly are type aliases and time conversions (using NodaTime).

Type Alias

A type alias (there might be a better name for it) is something that wraps a scalar primitive type like a string, int, guid, or something similar in a struct: This is mosly done for type savety when writing a LINQ query, which can drastically reduce bugs in an large codebase.

You can do something like that easlily in languages like F#, where you can use union types to do the job:

type OrderId = OrderId of System.Guid;
type CustomerId = CustomerId of System.Guid;

This prevents you from using an CustomerId where one would need an OrderId and vice-versa. Unfortunately, there is no such thing in C#, and I am unsure how well the support by EF is for F#, so one must rely on custom structs.

Example: Custom Struct

Instead of using a plain Guid type for an order's id, you could write something like:

    public struct OrderId : IEquatable<OrderId>, IComparable<OrderId>
    {
        private Guid Id { get; }

        public OrderId(Guid id)
        {
            Id = id;
        }

    public OrderId(string id)
        {
            Id = new Guid(id);
        }

        public static explicit operator Guid(OrderId id)
        {
            return id.Id;
        }

        public static explicit operator OrderId(Guid id)
        {
            return new OrderId(id);
        }

    public bool Equals(OrderId other)
        {
            return Id.Equals(other.Id);
        }

        public override bool Equals(object obj)
        {
            if (ReferenceEquals(null, obj)) return false;
            return obj is OrderId id && Equals(id);
        }

        public override int GetHashCode()
        {
            return Id.GetHashCode();
        }

    public static bool operator ==(OrderId x, OrderId y)
        {
            return x.Id == y.Id;
        }

        public static bool operator !=(OrderId x, OrderId y)
        {
            return !(x == y);
        }

    public int CompareTo(OrderId other)
    {
        return Id.CompareTo(other.Id);
    }

    public override string ToString()
    {
        return Id.ToString();
    }

    public static OrderId Empty => new OrderId(Guid.Empty);
    }

Hint: since I need hundreds of those types and they work mosly in the same manner, I use the Text Template Transformation Toolkit (T4) to create those classes.

Example Use-Case

Now lets have a class like the following:

public sealed class Order
{
    public OrderId Id { get; set; }

    // Navigational property for n:m relations
    public CustomerId CustomerId { get; set; }
    public Customer Customer { get; set; }

    // Navigational property gor 1:n relations 
    public ICollection<Item> Items { get; set; } = new HashSet<Item>();

    public NodaTime.Instant OrderDate { get; set; }
    public NodaTime.Instant? DeliveryDate { get; set; }

    // additional properties... 
}

Also assume that I did implement ValueConverters and register those with the ModelBulder:

var entity = modelBuilder.Entity<Order>();

entity.ToTable("Order");
entity.HasKey(e => e.Id);

entity.Property(e => e.Id)
    .HasColumnName("id")
    .HasColumnType("uniqueidentifier")
    .IsRequired()
    .HasConversion<OrderId>(id => (OrderId)id, oid => (Guid)oid);

/* other properties */

So now I want to do something like getting the order details of a set of selected orders within a time range:

// using an injected NodaTime IClock to get the current time 
var now = Clock.GetCurrentInstant();
var threeDaysAgo = now.Minus(Duration.FromDays(3));

var result = await Context.Orders
    .Where(order => order.OrderDate >= threeDaysAgo) // doesn't work
    .Where(order => order.DeliveryDate == null || order.DeliveryDate > threeDaysAgo) // doesn't work
    .Where(order => orderIds.Contains(order.Id)) // doesn't work
    .Where(order => nullableCustomerId == null || order.CustomerId == nullableCustomerId) // doesn't work
    .Include(order => order.Items)
    .Select(order => new OrderDetail { OrderId = order.Id, /* ... */ })
    .AsNoTracking()
    .ToListAsync(cancellationToken)
    .ConfigureAwait(false);

Remarks

I guess it would be possible to write custom extension methods for IQueryable<T> which build the proper expression tree. I am just not sure how that would work. So any hint would be very welcome.

References

• Designing with types: Single case union types
• How to: Use Expression Trees to Build Dynamic Queries (C#)
• C# Language: Discussion: Union and Intersection types
• EFCore Linq Query Could not be translated and will be evaluated locally

[ajcvickers]

@MovGP0 For the cases where you comment "doesn't work" do you get an exception, bad SQL, or does it fail in some other way? Can you please post details of the stracktraces, bad SQL, or other failures?

[MovGP0]

@ajcvickers those cases work in memory and not in the SQL Server.

If I get an exception or not depends on the Entity Framework configuration. By default it's just a warning and very slow.

[StevenRasmussen]

NodaTime is exactly where I'm encountering this issue too. I was super excited with the release of ValueConverters thinking I could now use NodaTime types for all of my datetime properties, but alas... I ran into this issue where the query would run in-memory after returning all the results from the table when I had a 'where' clause that involved a NodaTime type. When dealing with 100k+ records the performance is just not acceptable... had to revert to using DateTime again. Sigh...

[MovGP0]

I guess that there might be the need to register type conversions globally in the DbContext, such that the expression tree builder in the LINQ clauses can access it (ie. by trying to cast the IQueryable object to some new IConversionProvider base type) and generate the proper expression tree.

Registering the type conversions in the DataProvider might also be a possibility.

[NickCraver]

Not sure if related, but we're hitting similar, a simple example of:

var u = DB.Users.Single(u => u.GUID == new Guid(guidString));

...won't translate, where as:

var guid = new Guid(guidString);
var u = DB.Users.Single(u => u.GUID == guid);

...works fine. This was a regression from Linq2SQL where it was a non-issue and translated to a server-side SQL query.

[smitpatel]

@NickCraver - It is slightly unrelated to this issue but on my radar. I filed #13549 for tracking & visibility.

[divega]

@MovGP0 FYI, the limitation you describe is different from the one covered by this issue. We are tracking it in https://github.com/aspnet/EntityFrameworkCore/issues/12045.

[smitpatel]

The SQL translator would have access to type mapping of each translated fragment, allowing it to inspect the type mapping and call appropriate function on server side.

[apavelm]

@ajcvickers Is any progress on this?

I'm using NodaTime converters, but still unable to execute filtering/sorting on the server side.

The latest version of EF Core.

I see a warning, but cannot filter resultset by NodaTime's LocalDate/LocalDateTime/Instant

The LINQ expression 'where ([s].StartTime >= __dateStart_1)' could not be translated and will be evaluated locally.

Are any plans to implement this in further releases, or at least ability how to help SqlTranslator to do that?

I tried to add SqlColumnType for an EntityTypeBuilder config.Property(t => t.StartTime).HasColumnType("datetime").HasConversion(ConverterHelpers.LocalDateTimeConverter);

public static ValueConverter LocalDateTimeConverter => new ValueConverter<LocalDateTime, DateTime>(v => v.ToDateTimeUnspecified(), v => LocalDateTime.FromDateTime(v)); But it doesn't work. Any ETA on this?

Thank you.

UPDATED This and other NodaTime cases works fine for InMemoryDB and even PostgreSQL, but not for MS SQL Server.

[ajcvickers]

@apavelm This is in the 3.0 milestone, which means we still plan to look at it for EF Core 3.0. I don't expect every case of this to be fixed for that release, but I do expect we will make progress.

[apavelm]

@ajcvickers Thank you for info!

[ajcvickers]

Notes from triage:

• Cases where the representation in the database has not changed (e.g. some type that is not DateTime, but still translated to datetime in the database) should work for 3.0 with the query pipeline changes
• Cases where the representation is different (e.g. DateTime to strings in the database) is dependent on #15979 and may be punted. depending on what we see from feedback and testing.

[ajcvickers]

See also scenario in #16639

[ajcvickers]

See also the case here: #17879

[OskarKlintrot]

Will this make it to 5.0? Would love to be able to combine NodaTime with EF Core + SQL Server!

[ajcvickers]

@OskarKlintrot We're not currently scheduling this for 5.0. However, I will discuss with team. I don't expect we can fit it in, but we may make it a stretch goal.

[ajcvickers]

Update from triage: we will look at this as a stretch goal for 5.0. Specifically, we may be able to identify some relatively cheap things we can do to enable common scenarios without the API becoming blocking for later work on more advanced scenarios.

[ajcvickers]

See also case here: #20094

[Lobosque]

Another use case that might be worth sharing: https://github.com/Lobosque/ef-value-object-poc

[ajcvickers]

See example in #21956

[ajcvickers]

See https://github.com/dotnet/efcore/issues/10434

[AndriySvyryd]

See #10434

This is #10434

[OpenSpacesAndPlaces]

Ran into this running contains on a Int32[] that is value converting from a string field. Translation of method 'System.Linq.Enumerable.Contains' failed.

[AndriySvyryd]

A design proposal is to have two store-side value converters on the type mapping that wouldn't impact the store type used for migrations:

• One for serialization/deserialization, it can affect the provider type
• One for query when using type inference. Composition can be used to construct the appropriate converter chain depending on the context (e.g. when used as an argument of a function that expects a different store type)

[voroninp]

Will it be planned for EF 9?

[ajcvickers]

@voroninp Too early to say yet, but I think it's probably quite unlikely that we'll fully implement this in EF Core 9.

[voroninp]

@ajcvickers , if I wanted to extend that part, where should I look at?

In most cases I need translation of comparison operators (>, >=, <, <=) and in most cases it's just the same operation applied to the converted value.

[roji]

@voroninp you'd have to provide more context on exactly which (converted) types you want to add comparison operators to.

[voroninp]

For example we have ints which are time HHmmss, we convert them to our own type RadioTime (don't ask why not TimeOnly) which is effectively just a wrapper. And we need queries where we compare times.

[roji]

You'll have to define functions representing operations over RadioTime (e.g. GreaterThan), and then use user-defined function mapping to map them to the SQL operators.

[voroninp]

@roji

System.InvalidOperationException Message=The parameter 'left' for the DbFunction DateIndex.op_GreaterThan(DateIndex, DateIndex)' has an invalid type 'DateIndex'. Ensure the parameter type can be mapped by the current provider.

And in ConfigureConventions I have this:

configurationBuilder.Properties<DateIndex>(b =>
{
    b.HaveConversion<DateIndexToIntConverter>();
});

[voroninp]

@roji, I need some help:

UPD: I got it. It's an expression tree, so it will be Expression.LessThan, so there won't be any call to op_LessThan. ='( There should be additional check on EF Core side.

modelBuilder.HasDbFunction(
    typeof(SqlTranslation).GetMethod(
        nameof(SqlTranslation.LessThan), 
        BindingFlags.Public | BindingFlags.Static, 
        new[] { typeof(DateTimeOffset), typeof(DateTimeOffset) })!)
    .HasTranslation(
        args => new SqlBinaryExpression(
            ExpressionType.LessThan,
            args[0],
            args[1],
            typeof(bool),
            new BoolTypeMapping("boolean", DbType.Boolean)));

This works, but if I replace with typeof(DateTimeOffset).GetMethod("op_LessThan", BindingFlags.Public | BindingFlags.Static)! it fails. Are special methods not checked for custom mapping?

And what type of SQL expression should I use, if I want to define it as an instance function on spot?

bool IsOlderThan(Classification classification) => this.Timestamp < classification.Timestamp;

[roji]

@voroninp can you please open a separate issue with the precise repro code sample that shows what additional check you're proposing?

[soroshsabz]

ITNOA

Hi,

How many votes needed to start implementation of this feature?