From d0a8f59f934fbc7aa4b83c5c346abe0d5f4a3d5c Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Tue, 18 Jul 2023 13:00:05 +0200 Subject: [PATCH] Fix SQLite shenanigans --- ...yableMethodTranslatingExpressionVisitor.cs | 56 +++++++++++++---- ...SharedPrimitiveCollectionsQueryTestBase.cs | 16 +++++ ...dPrimitiveCollectionsQuerySqlServerTest.cs | 60 +++++++++++++++++++ ...aredPrimitiveCollectionsQuerySqliteTest.cs | 53 ++++++++++++++++ 4 files changed, 173 insertions(+), 12 deletions(-) diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs index 641ee1735f2..c6c1de9929e 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs +++ b/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; /// @@ -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? _currentSelectInferredTypeMappings; /// @@ -359,10 +361,10 @@ protected class SqliteInferredTypeMappingApplier : RelationalInferredTypeMapping /// public SqliteInferredTypeMappingApplier( IRelationalTypeMappingSource typeMappingSource, - ISqlExpressionFactory sqlExpressionFactory, + SqliteSqlExpressionFactory sqlExpressionFactory, IReadOnlyDictionary<(TableExpressionBase, string), RelationalTypeMapping?> inferredTypeMappings) : base(sqlExpressionFactory, inferredTypeMappings) - => _typeMappingSource = typeMappingSource; + => (_typeMappingSource, _sqlExpressionFactory) = (typeMappingSource, sqlExpressionFactory); /// /// 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. /// - 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". diff --git a/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs b/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs index 1fda743eaba..a66bda115a9 100644 --- a/test/EFCore.Specification.Tests/Query/NonSharedPrimitiveCollectionsQueryTestBase.cs +++ b/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( diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs index cfadd539dfa..cc0c0463f56 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqlServerTest.cs +++ b/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(); diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqliteTest.cs index 8e2084f7e62..33058522aaa 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/NonSharedPrimitiveCollectionsQuerySqliteTest.cs +++ b/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(() => 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