How does translation of comparison operators work for converted values?

[voroninp]

I used a DateTime to long conversion for Sqlite database:

internal sealed class UtcDateTimeConverter : ValueConverter<DateTime, long>
{
    public UtcDateTimeConverter()
        : base(model => model.Ticks, dbValue => new DateTime(dbValue, DateTimeKind.Utc), convertsNulls: false)
    {
    }
}

I was expecting the query which compares two timestamps to fail, yet it works. But how? How does EF know that it should compare underlying longs? What if I decided to write negative values:

internal sealed class UtcDateTimeConverter : ValueConverter<DateTime, long>
{
    public UtcDateTimeConverter()
        : base(model => -model.Ticks, dbValue => new DateTime(-dbValue, DateTimeKind.Utc), convertsNulls: false)
    {
    }
}

For example, for DateTimeOffset with conversion to string EF could not translate comparison to SQL.

[ajcvickers]

Duplicate of #10434

[voroninp]

@ajcvickers , I know that issue. But how/why does it work now?

[ajcvickers]

I suspect it's just comparing the values in the same way it would with the default mapping. What does the SQL look like?

[voroninp]

@ajcvickers , yes, exactly as you say:

SELECT "r"."Id", "r"."ActualDurationInSeconds", "r"."BreakBroadcastId", "r"."BreakCode", "r"."ChannelId", "r"."ClassificationId", "r"."CreativeTitle", "r"."Date", "r"."HasMatchedBreak", "r"."NextSpotIdInTrain", "r"."NmoCreativeId", "r"."PayingAgencyHarmonizedName", "r"."PayingAgencyIdFromSalesHouse", "r"."PlannedDate", "r"."PlannedDurationInSeconds", "r"."PlanningAgencyHarmonizedName", "r"."PlanningAgencyIdFromSalesHouse", "r"."PositionInBreak", "r"."PreviousSpotIdInTrain", "r"."SalesHouseCampaignContractNumber", "r"."SalesHouseCreativeId", "r"."SourceFileId", "r"."SpotId", "r"."SpotType", "r"."StartTimeMinutesPrecision", "r"."StartTimeSecondsPrecision", "r"."TimestampUtc"
FROM "RadioSpots" AS "r"
INNER JOIN "CreativeClassifications" AS "c" ON "r"."NmoCreativeId" = "c"."NmoCreativeId"
WHERE "r"."NmoCreativeId" NOT IN (-1, 100008760) AND "r"."TimestampUtc" < "c"."TimestampUtc"

var query =
    from spot in dbContext.Set<RadioSpotRaw>()
        .Where(s => s.NmoCreativeId != NloCreativeId.Unknown && s.NmoCreativeId != NloCreativeId.RegionalWithoutClassification)
    join classification in dbContext.Set<RadioCreativeClassification>()
    on spot.NmoCreativeId equals classification.NmoCreativeId
    where spot.TimestampUtc < classification.TimestampUtc
    select spot;

Then why does it fail for DateTimeOffset type when it's converted to string? Comparison is still a valid operation for strings.

[roji]

We specifically block those comparison operations for Sqlite (code), since on Sqlite DateTimeOffset (a commonly-used type) is by default value-converted to string, so it's a big pit of failure lots of people would fall into.

But at the end of the day, the moment you go into value conversion, you have to be very careful with any operators/function calls you do.

[voroninp]

But at the end of the day, the moment you go into value conversion, you have to be very careful with any operators/function calls you do.

I've noticed =D

Though I am a bit surprised that you decided to explicitly block some translations, yet by default EF just applies the same operators to converted values. I guess it's just more or less safe default as it's very unlikely conversion will change order relationship.

I hope #10434 makes into EF 9 ;-)

[roji]

Though I am a bit surprised that you decided to explicitly block some translations, yet by default EF just applies the same operators to converted values. I guess it's just more or less safe default as it's very unlikely conversion will change order relationship.

It's just about DateTimeOffset and Sqlite being a particular pit of failure, specifically because it's a default value conversion; the user doesn't opt in, and may have no idea that value conversion is even involved. If you explicitly configure value conversions, it makes a bit more sense that you're responsible (or hopefully at least considering) what happens with comparisons etc.

[voroninp]

DateTime also has its own issues with Kind ='(

[roji]

Kind doesn't participate in comparisons so it's less of a problem.

[voroninp]

Btw, that's how I hacked custom comparison for DateTimeOffset (safe for me because it's always UTC):

internal static class SqliteTranslation
{
    public static bool LessThan(DateTimeOffset left, DateTimeOffset right) => left < right;

    public static bool LessThanOrEqual(DateTimeOffset left, DateTimeOffset right) => left <= right;

    public static bool GreaterThan(DateTimeOffset left, DateTimeOffset right) => left > right;

    public static bool GreaterThanOrEqual(DateTimeOffset left, DateTimeOffset right) => left >= right;
}

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.HasDbFunction(
        typeof(SqliteTranslation).GetMethod(
            nameof(SqliteTranslation.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)));
    modelBuilder.HasDbFunction(
        typeof(SqliteTranslation).GetMethod(
            nameof(SqliteTranslation.LessThanOrEqual),
            BindingFlags.Public | BindingFlags.Static,
            new[] { typeof(DateTimeOffset), typeof(DateTimeOffset) })!)
        .HasTranslation(
            args => new SqlBinaryExpression(
                ExpressionType.LessThanOrEqual,
                args[0],
                args[1],
                typeof(bool),
                new BoolTypeMapping("boolean", DbType.Boolean)));
    modelBuilder.HasDbFunction(
        typeof(SqliteTranslation).GetMethod(
            nameof(SqliteTranslation.GreaterThan),
            BindingFlags.Public | BindingFlags.Static,
            new[] { typeof(DateTimeOffset), typeof(DateTimeOffset) })!)
        .HasTranslation(
            args => new SqlBinaryExpression(
                ExpressionType.GreaterThan,
                args[0],
                args[1],
                typeof(bool),
                new BoolTypeMapping("boolean", DbType.Boolean)));
    modelBuilder.HasDbFunction(
        typeof(SqliteTranslation).GetMethod(
            nameof(SqliteTranslation.GreaterThanOrEqual),
            BindingFlags.Public | BindingFlags.Static,
            new[] { typeof(DateTimeOffset), typeof(DateTimeOffset) })!)
        .HasTranslation(
            args => new SqlBinaryExpression(
                ExpressionType.GreaterThanOrEqual,
                args[0],
                args[1],
                typeof(bool),
                new BoolTypeMapping("boolean", DbType.Boolean)));
...
}

internal sealed class DateTimeOffsetComparisonInterceptorForSqlite : IQueryExpressionInterceptor
{
    private static readonly ExpressionVisitor Visitor = new DateTimeOffsetComparisonVisitor();

    public Expression QueryCompilationStarting(Expression queryExpression, QueryExpressionEventData eventData)
    {
        return Visitor.Visit(queryExpression);
    }

    private sealed class DateTimeOffsetComparisonVisitor : ExpressionVisitor
    {
        private static readonly System.Type DateTimeOffsetType = typeof(DateTimeOffset);

        private static readonly MethodInfo LessThanMethod = 
            typeof(SqliteTranslation).GetMethod(nameof(SqliteTranslation.LessThan), BindingFlags.Public | BindingFlags.Static)!;

        private static readonly MethodInfo LessThanOrEqualMethod = 
            typeof(SqliteTranslation).GetMethod(nameof(SqliteTranslation.LessThanOrEqual), BindingFlags.Public | BindingFlags.Static)!;

        private static readonly MethodInfo GreaterThanMethod = 
            typeof(SqliteTranslation).GetMethod(nameof(SqliteTranslation.GreaterThan), BindingFlags.Public | BindingFlags.Static)!;

        private static readonly MethodInfo GreaterThenOrEqualMethod = 
            typeof(SqliteTranslation).GetMethod(nameof(SqliteTranslation.GreaterThanOrEqual), BindingFlags.Public | BindingFlags.Static)!;

        protected override Expression VisitBinary(BinaryExpression node)
        {
            if (node.Left.Type != DateTimeOffsetType || node.Right.Type != DateTimeOffsetType)
            {
                return base.VisitBinary(node);
            }

            return node.NodeType switch
            {
                ExpressionType.LessThan => Expression.Call(null, LessThanMethod, node.Left, node.Right),
                ExpressionType.LessThanOrEqual => Expression.Call(null, LessThanOrEqualMethod, node.Left, node.Right),
                ExpressionType.GreaterThan => Expression.Call(null, GreaterThanMethod, node.Left, node.Right),
                ExpressionType.GreaterThanOrEqual => Expression.Call(null, GreaterThenOrEqualMethod, node.Left, node.Right),
                _ => base.VisitBinary(node),
            };
        }
    }
}

[roji]

If it's only UTC, why are you using DateTimeOffset in the first place (and always have +00:00 suffixed everywhere)? Why not just store DateTime, at which point you don't need all that code?

[voroninp]

Indeed, I switched to DateTime, but it was a nice experiment I learned a lot from.