Fix SQLite shenanigans

src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs

@@ -17,7 +17,7 @@ namespace Microsoft.EntityFrameworkCore.Sqlite.Query.Internal;
 public class SqliteQueryableMethodTranslatingExpressionVisitor : RelationalQueryableMethodTranslatingExpressionVisitor
 {
     private readonly IRelationalTypeMappingSource _typeMappingSource;
-    private readonly ISqlExpressionFactory _sqlExpressionFactory;
+    private readonly SqliteSqlExpressionFactory _sqlExpressionFactory;
     private readonly bool _areJsonFunctionsSupported;
 
     /// <summary>
@@ -33,7 +33,7 @@ public SqliteQueryableMethodTranslatingExpressionVisitor(
         : base(dependencies, relationalDependencies, queryCompilationContext)
     {
         _typeMappingSource = relationalDependencies.TypeMappingSource;
-        _sqlExpressionFactory = relationalDependencies.SqlExpressionFactory;
+        _sqlExpressionFactory = (SqliteSqlExpressionFactory)relationalDependencies.SqlExpressionFactory;
 
         _areJsonFunctionsSupported = new Version(new SqliteConnection().ServerVersion) >= new Version(3, 38);
     }
@@ -227,7 +227,7 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis
         // the JSON value out to its relational counterpart (e.g. datetime() for timestamps, see ApplyJsonSqlConversion).
         //
         // However, doing it here would interfere with pattern matching in e.g. TranslateElementAtOrDefault, where we specifically check
-        // for a bare column being projected out out of the table - if the user composed any operators over the collection, it's no longer
+        // for a bare column being projected out of the table - if the user composed any operators over the collection, it's no longer
         // possible to apply a specialized translation via the -> operator. We could add a way to recognize the special conversions we
         // compose on top, but instead of going into that complexity, we'll just apply the SQL conversion later, in
         // SqliteInferredTypeMappingApplier, as if we had a parameter collection.
@@ -314,7 +314,8 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis
                 // conversions.
                 if (projectionColumn.TypeMapping is not null)
                 {
-                    translation = ApplyJsonSqlConversion(translation, projectionColumn.TypeMapping, projectionColumn.IsNullable);
+                    translation = ApplyJsonSqlConversion(
+                        translation, _sqlExpressionFactory, projectionColumn.TypeMapping, projectionColumn.IsNullable);
                 }
 
                 return source.UpdateQueryExpression(_sqlExpressionFactory.Select(translation));
@@ -349,6 +350,7 @@ protected override Expression ApplyInferredTypeMappings(
     protected class SqliteInferredTypeMappingApplier : RelationalInferredTypeMappingApplier
     {
         private readonly IRelationalTypeMappingSource _typeMappingSource;
+        private readonly SqliteSqlExpressionFactory _sqlExpressionFactory;
         private Dictionary<TableExpressionBase, RelationalTypeMapping>? _currentSelectInferredTypeMappings;
 
         /// <summary>
@@ -359,10 +361,10 @@ protected class SqliteInferredTypeMappingApplier : RelationalInferredTypeMapping
         /// </summary>
         public SqliteInferredTypeMappingApplier(
             IRelationalTypeMappingSource typeMappingSource,
-            ISqlExpressionFactory sqlExpressionFactory,
+            SqliteSqlExpressionFactory sqlExpressionFactory,
             IReadOnlyDictionary<(TableExpressionBase, string), RelationalTypeMapping?> inferredTypeMappings)
             : base(sqlExpressionFactory, inferredTypeMappings)
-            => _typeMappingSource = typeMappingSource;
+            => (_typeMappingSource, _sqlExpressionFactory) = (typeMappingSource, sqlExpressionFactory);
 
         /// <summary>
         ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -415,6 +417,7 @@ when TryGetInferredTypeMapping(jsonEachExpression, "value", out var typeMapping)
                     when _currentSelectInferredTypeMappings?.TryGetValue(columnExpression.Table, out var inferredTypeMapping) is true:
                     return ApplyJsonSqlConversion(
                         columnExpression.ApplyTypeMapping(inferredTypeMapping),
+                        _sqlExpressionFactory,
                         inferredTypeMapping,
                         columnExpression.IsNullable);
 
@@ -460,16 +463,45 @@ protected virtual TableValuedFunctionExpression ApplyTypeMappingsOnJsonEachExpre
     /// Wraps the given expression with any SQL logic necessary to convert a value coming out of a JSON document into the relational value
     /// represented by the given type mapping.
     /// </summary>
-    private static SqlExpression ApplyJsonSqlConversion(SqlExpression expression, RelationalTypeMapping typeMapping, bool isNullable)
+    private static SqlExpression ApplyJsonSqlConversion(
+        SqlExpression expression,
+        SqliteSqlExpressionFactory sqlExpressionFactory,
+        RelationalTypeMapping typeMapping,
+        bool isNullable)
         => typeMapping switch
         {
+            // The "default" JSON representation of a GUID is a lower-case string, but we do upper-case GUIDs in our non-JSON
+            // implementation.
+            SqliteGuidTypeMapping
+                => sqlExpressionFactory.Function("upper", new[] { expression }, isNullable, new[] { true }, typeof(Guid), typeMapping),
+
             // The "standard" JSON timestamp representation is ISO8601, with a T between date and time; but SQLite's representation has
-            // no T. Apply a conversion on the value coming out of json_each.
+            // no T. The following performs a reliable conversions on the string values coming out of json_each.
+            // Unfortunately, the SQLite datetime() function doesn't present fractional seconds, so we generate the following lovely thing:
+            // rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', $value), '0'), '.')
             SqliteDateTimeTypeMapping
-                => new SqlFunctionExpression("datetime", new[] { expression }, isNullable, new[] { true }, typeof(DateTime), typeMapping),
-
-            SqliteGuidTypeMapping
-                => new SqlFunctionExpression("upper", new[] { expression }, isNullable, new[] { true }, typeof(Guid), typeMapping),
+                => sqlExpressionFactory.Function(
+                    "rtrim",
+                    new SqlExpression[]
+                    {
+                        sqlExpressionFactory.Function(
+                            "rtrim",
+                            new SqlExpression[]
+                            {
+                                sqlExpressionFactory.Function(
+                                    "strftime",
+                                    new[]
+                                    {
+                                        sqlExpressionFactory.Constant("%Y-%m-%d %H:%M:%f"),
+                                        expression
+                                    },
+                                    isNullable, new[] { true }, typeof(DateTime), typeMapping),
+                                sqlExpressionFactory.Constant("0")
+                            },
+                            isNullable, new[] { true }, typeof(DateTime), typeMapping),
+                        sqlExpressionFactory.Constant(".")
+                    },
+                    isNullable, new[] { true }, typeof(DateTime), typeMapping),
 
             // The JSON representation for decimal is e.g. 1 (JSON int), whereas our literal representation is "1.0" (string).
             // We can cast the 1 to TEXT, but we'd still get "1" not "1.0".

test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs

@@ -45,6 +45,14 @@ public virtual Task Array_of_decimal()
     public virtual Task Array_of_DateTime()
         => TestArray(new DateTime(2023, 1, 1, 12, 30, 0), new DateTime(2023, 1, 2, 12, 30, 0));
 
+    [ConditionalFact]
+    public virtual Task Array_of_DateTime_with_milliseconds()
+        => TestArray(new DateTime(2023, 1, 1, 12, 30, 0, 123), new DateTime(2023, 1, 1, 12, 30, 0, 124));
+
+    [ConditionalFact]
+    public virtual Task Array_of_DateTime_with_microseconds()
+        => TestArray(new DateTime(2023, 1, 1, 12, 30, 0, 123, 456), new DateTime(2023, 1, 1, 12, 30, 0, 123, 457));
+
     [ConditionalFact]
     public virtual Task Array_of_DateOnly()
         => TestArray(new DateOnly(2023, 1, 1), new DateOnly(2023, 1, 2));
@@ -53,6 +61,14 @@ public virtual Task Array_of_DateOnly()
     public virtual Task Array_of_TimeOnly()
         => TestArray(new TimeOnly(12, 30, 0), new TimeOnly(12, 30, 1));
 
+    [ConditionalFact]
+    public virtual Task Array_of_TimeOnly_with_milliseconds()
+        => TestArray(new TimeOnly(12, 30, 0, 123), new TimeOnly(12, 30, 0, 124));
+
+    [ConditionalFact]
+    public virtual Task Array_of_TimeOnly_with_microseconds()
+        => TestArray(new TimeOnly(12, 30, 0, 123, 456), new TimeOnly(12, 30, 0, 124, 457));
+
     [ConditionalFact]
     public virtual Task Array_of_DateTimeOffset()
         => TestArray(

test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs

@@ -136,6 +136,36 @@ FROM OPENJSON([t].[SomeArray]) WITH ([value] datetime2 '$') AS [s]
 """);
     }
 
+    public override async Task Array_of_DateTime_with_milliseconds()
+    {
+        await base.Array_of_DateTime_with_milliseconds();
+
+        AssertSql(
+"""
+SELECT TOP(2) [t].[Id], [t].[Ints], [t].[SomeArray]
+FROM [TestEntity] AS [t]
+WHERE (
+    SELECT COUNT(*)
+    FROM OPENJSON([t].[SomeArray]) WITH ([value] datetime2 '$') AS [s]
+    WHERE [s].[value] = '2023-01-01T12:30:00.1230000') = 2
+""");
+    }
+
+    public override async Task Array_of_DateTime_with_microseconds()
+    {
+        await base.Array_of_DateTime_with_microseconds();
+
+        AssertSql(
+            """
+SELECT TOP(2) [t].[Id], [t].[Ints], [t].[SomeArray]
+FROM [TestEntity] AS [t]
+WHERE (
+    SELECT COUNT(*)
+    FROM OPENJSON([t].[SomeArray]) WITH ([value] datetime2 '$') AS [s]
+    WHERE [s].[value] = '2023-01-01T12:30:00.1234560') = 2
+""");
+    }
+
     public override async Task Array_of_DateOnly()
     {
         await base.Array_of_DateOnly();
@@ -166,6 +196,36 @@ FROM OPENJSON([t].[SomeArray]) WITH ([value] time '$') AS [s]
 """);
     }
 
+    public override async Task Array_of_TimeOnly_with_milliseconds()
+    {
+        await base.Array_of_TimeOnly_with_milliseconds();
+
+        AssertSql(
+"""
+SELECT TOP(2) [t].[Id], [t].[Ints], [t].[SomeArray]
+FROM [TestEntity] AS [t]
+WHERE (
+    SELECT COUNT(*)
+    FROM OPENJSON([t].[SomeArray]) WITH ([value] time '$') AS [s]
+    WHERE [s].[value] = '12:30:00.123') = 2
+""");
+    }
+
+    public override async Task Array_of_TimeOnly_with_microseconds()
+    {
+        await base.Array_of_TimeOnly_with_microseconds();
+
+        AssertSql(
+"""
+SELECT TOP(2) [t].[Id], [t].[Ints], [t].[SomeArray]
+FROM [TestEntity] AS [t]
+WHERE (
+    SELECT COUNT(*)
+    FROM OPENJSON([t].[SomeArray]) WITH ([value] time '$') AS [s]
+    WHERE [s].[value] = '12:30:00.123456') = 2
+""");
+    }
+
     public override async Task Array_of_DateTimeOffset()
     {
         await base.Array_of_DateTimeOffset();

test/EFCore.Sqlite.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqliteTest.cs

@@ -115,6 +115,27 @@ LIMIT 2
 """);
     }
 
+    public override async Task Array_of_DateTime_with_milliseconds()
+    {
+        await base.Array_of_DateTime_with_milliseconds();
+
+        AssertSql(
+"""
+SELECT "t"."Id", "t"."Ints", "t"."SomeArray"
+FROM "TestEntity" AS "t"
+WHERE (
+    SELECT COUNT(*)
+    FROM json_each("t"."SomeArray") AS "s"
+    WHERE rtrim(rtrim(strftime('%Y-%m-%d %H:%M:%f', "s"."value"), '0'), '.') = '2023-01-01 12:30:00.123') = 2
+LIMIT 2
+""");
+    }
+
+    // SQLite strftime, which we use to convert JSON strings to a relational representation (essentially strip the T in the middle) does
+    // not support microsecond precision.
+    public override Task Array_of_DateTime_with_microseconds()
+        => Assert.ThrowsAsync<InvalidOperationException>(() => base.Array_of_DateTime_with_microseconds());
+
     public override async Task Array_of_DateOnly()
     {
         await base.Array_of_DateOnly();
@@ -147,6 +168,38 @@ LIMIT 2
 """);
     }
 
+    public override async Task Array_of_TimeOnly_with_milliseconds()
+    {
+        await base.Array_of_TimeOnly_with_milliseconds();
+
+        AssertSql(
+"""
+SELECT "t"."Id", "t"."Ints", "t"."SomeArray"
+FROM "TestEntity" AS "t"
+WHERE (
+    SELECT COUNT(*)
+    FROM json_each("t"."SomeArray") AS "s"
+    WHERE "s"."value" = '12:30:00.1230000') = 2
+LIMIT 2
+""");
+    }
+
+    public override async Task Array_of_TimeOnly_with_microseconds()
+    {
+        await base.Array_of_TimeOnly_with_microseconds();
+
+        AssertSql(
+"""
+SELECT "t"."Id", "t"."Ints", "t"."SomeArray"
+FROM "TestEntity" AS "t"
+WHERE (
+    SELECT COUNT(*)
+    FROM json_each("t"."SomeArray") AS "s"
+    WHERE "s"."value" = '12:30:00.1234560') = 2
+LIMIT 2
+""");
+    }
+
     public override async Task Array_of_DateTimeOffset()
     {
         // The JSON representation for DateTimeOffset is ISO8601 (2023-01-01T12:30:00+02:00), but our SQL literal representation