From a92f323842403375913e12a939ee92c010ee4155 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Sun, 5 May 2024 23:23:38 +0200 Subject: [PATCH 1/5] Implement `MIN/MAX` functions for `decimal` in SQLite --- ...qliteQueryableAggregateMethodTranslator.cs | 24 +++++++++++++++++-- .../Internal/SqliteRelationalConnection.cs | 16 +++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteQueryableAggregateMethodTranslator.cs b/src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteQueryableAggregateMethodTranslator.cs index 00e35af7457..b6d412879b5 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteQueryableAggregateMethodTranslator.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/Translators/SqliteQueryableAggregateMethodTranslator.cs @@ -70,13 +70,23 @@ public SqliteQueryableAggregateMethodTranslator(ISqlExpressionFactory sqlExpress && source.Selector is SqlExpression maxSqlExpression: var maxArgumentType = GetProviderType(maxSqlExpression); if (maxArgumentType == typeof(DateTimeOffset) - || maxArgumentType == typeof(decimal) || maxArgumentType == typeof(TimeSpan) || maxArgumentType == typeof(ulong)) { throw new NotSupportedException( SqliteStrings.AggregateOperationNotSupported(nameof(Queryable.Max), maxArgumentType.ShortDisplayName())); } + else if (maxArgumentType == typeof(decimal)) + { + maxSqlExpression = CombineTerms(source, maxSqlExpression); + return _sqlExpressionFactory.Function( + "ef_max", + [maxSqlExpression], + nullable: true, + argumentsPropagateNullability: [false], + maxSqlExpression.Type, + maxSqlExpression.TypeMapping); + } break; @@ -86,13 +96,23 @@ public SqliteQueryableAggregateMethodTranslator(ISqlExpressionFactory sqlExpress && source.Selector is SqlExpression minSqlExpression: var minArgumentType = GetProviderType(minSqlExpression); if (minArgumentType == typeof(DateTimeOffset) - || minArgumentType == typeof(decimal) || minArgumentType == typeof(TimeSpan) || minArgumentType == typeof(ulong)) { throw new NotSupportedException( SqliteStrings.AggregateOperationNotSupported(nameof(Queryable.Min), minArgumentType.ShortDisplayName())); } + else if (minArgumentType == typeof(decimal)) + { + minSqlExpression = CombineTerms(source, minSqlExpression); + return _sqlExpressionFactory.Function( + "ef_min", + [minSqlExpression], + nullable: true, + argumentsPropagateNullability: [false], + minSqlExpression.Type, + minSqlExpression.TypeMapping); + } break; diff --git a/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs b/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs index 44daa250463..61d27375771 100644 --- a/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs +++ b/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs @@ -154,6 +154,22 @@ private void InitializeDbConnection(DbConnection connection) : acc.sum / acc.count, isDeterministic: true); + sqliteConnection.CreateAggregate( + "ef_max", + seed: null, + (decimal? max, decimal? value) => max is null + ? value + : value is null ? max : decimal.Max(max.Value, value.Value), + isDeterministic: true); + + sqliteConnection.CreateAggregate( + "ef_min", + seed: null, + (decimal? min, decimal? value) => min is null + ? value + : value is null ? min : decimal.Min(min.Value, value.Value), + isDeterministic: true); + sqliteConnection.CreateAggregate( "ef_sum", seed: null, From cbcd1431f2b7a1710e948feca347b743a41b11b3 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Sun, 5 May 2024 23:24:26 +0200 Subject: [PATCH 2/5] Update tests --- .../BuiltInDataTypesSqliteTest.cs | 10 ++----- .../Query/Ef6GroupBySqliteTest.cs | 28 +++++++++++++------ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs index 7799bcfa415..f503df97a6a 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs @@ -862,10 +862,7 @@ public virtual void Cant_query_Min_of_converted_types() .Where(e => e.PartitionId == 200) .GroupBy(_ => true); - Assert.Equal( - SqliteStrings.AggregateOperationNotSupported(nameof(Queryable.Min), typeof(decimal).ShortDisplayName()), - Assert.Throws( - () => query.Select(g => g.Min(e => e.TestNullableDecimal)).ToList()).Message); + Assert.Equal(2.000000000000001m, query.Select(g => g.Min(e => e.TestNullableDecimal)).Single()); Assert.Equal( SqliteStrings.AggregateOperationNotSupported(nameof(Queryable.Min), typeof(DateTimeOffset).ShortDisplayName()), @@ -915,10 +912,7 @@ public virtual void Cant_query_Max_of_converted_types() .Where(e => e.PartitionId == 201) .GroupBy(_ => true); - Assert.Equal( - SqliteStrings.AggregateOperationNotSupported(nameof(Queryable.Max), typeof(decimal).ShortDisplayName()), - Assert.Throws( - () => query.Select(g => g.Max(e => e.TestNullableDecimal)).ToList()).Message); + Assert.Equal(10.000000000000001m, query.Select(g => g.Max(e => e.TestNullableDecimal)).Single()); Assert.Equal( SqliteStrings.AggregateOperationNotSupported(nameof(Queryable.Max), typeof(DateTimeOffset).ShortDisplayName()), diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/Ef6GroupBySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/Ef6GroupBySqliteTest.cs index cda96d9fcd8..ffce9f09eda 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/Ef6GroupBySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/Ef6GroupBySqliteTest.cs @@ -29,16 +29,28 @@ GROUP BY "p"."Category" } public override async Task Max_Grouped_from_LINQ_101(bool async) - => Assert.Equal( - SqliteStrings.AggregateOperationNotSupported("Max", "decimal"), - (await Assert.ThrowsAsync( - () => base.Max_Grouped_from_LINQ_101(async))).Message); + { + await base.Max_Grouped_from_LINQ_101(async); + + AssertSql( + """ +SELECT "p"."Category", ef_max("p"."UnitPrice") AS "MostExpensivePrice" +FROM "ProductForLinq" AS "p" +GROUP BY "p"."Category" +"""); + } public override async Task Min_Grouped_from_LINQ_101(bool async) - => Assert.Equal( - SqliteStrings.AggregateOperationNotSupported("Min", "decimal"), - (await Assert.ThrowsAsync( - () => base.Min_Grouped_from_LINQ_101(async))).Message); + { + await base.Min_Grouped_from_LINQ_101(async); + + AssertSql( + """ +SELECT "p"."Category", ef_min("p"."UnitPrice") AS "CheapestPrice" +FROM "ProductForLinq" AS "p" +GROUP BY "p"."Category" +"""); + } public override async Task Whats_new_2021_sample_3(bool async) => await base.Whats_new_2021_sample_3(async); From 61b7a88942401acf17e36bf19405e4dc0e326e31 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Sun, 9 Feb 2025 00:55:07 +0100 Subject: [PATCH 3/5] Implement collation for `decimal` in SQLite --- ...yableMethodTranslatingExpressionVisitor.cs | 50 +++++++++++-------- .../Internal/SqliteRelationalConnection.cs | 4 ++ 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs index 768f147b38f..b841caf5bc6 100644 --- a/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Sqlite.Core/Query/Internal/SqliteQueryableMethodTranslatingExpressionVisitor.cs @@ -133,24 +133,28 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis LambdaExpression keySelector, bool ascending) { - var translation = base.TranslateOrderBy(source, keySelector, ascending); + var translation = TranslateLambdaExpression(source, keySelector); if (translation == null) { return null; } - var orderingExpression = ((SelectExpression)translation.QueryExpression).Orderings.Last(); - var orderingExpressionType = GetProviderType(orderingExpression.Expression); + var orderingExpressionType = GetProviderType(translation); if (orderingExpressionType == typeof(DateTimeOffset) - || orderingExpressionType == typeof(decimal) || orderingExpressionType == typeof(TimeSpan) || orderingExpressionType == typeof(ulong)) { throw new NotSupportedException( SqliteStrings.OrderByNotSupported(orderingExpressionType.ShortDisplayName())); } + else if (orderingExpressionType == typeof(decimal)) + { + translation = new CollateExpression(translation, "EF_DECIMAL"); + } - return translation; + ((SelectExpression)source.QueryExpression).ApplyOrdering(new OrderingExpression(translation, ascending)); + + return source; } /// @@ -164,24 +168,28 @@ protected override QueryableMethodTranslatingExpressionVisitor CreateSubqueryVis LambdaExpression keySelector, bool ascending) { - var translation = base.TranslateThenBy(source, keySelector, ascending); + var translation = TranslateLambdaExpression(source, keySelector); if (translation == null) { return null; } - var orderingExpression = ((SelectExpression)translation.QueryExpression).Orderings.Last(); - var orderingExpressionType = GetProviderType(orderingExpression.Expression); + var orderingExpressionType = GetProviderType(translation); if (orderingExpressionType == typeof(DateTimeOffset) - || orderingExpressionType == typeof(decimal) || orderingExpressionType == typeof(TimeSpan) || orderingExpressionType == typeof(ulong)) { throw new NotSupportedException( SqliteStrings.OrderByNotSupported(orderingExpressionType.ShortDisplayName())); } + else if (orderingExpressionType == typeof(decimal)) + { + translation = new CollateExpression(translation, "EF_DECIMAL"); + } + + ((SelectExpression)source.QueryExpression).AppendOrdering(new OrderingExpression(translation, ascending)); - return translation; + return source; } /// @@ -467,9 +475,9 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr Tables: [ TableValuedFunctionExpression - { - Name: "json_each", Schema: null, IsBuiltIn: true, Arguments: [var jsonArrayColumn] - } jsonEachExpression + { + Name: "json_each", Schema: null, IsBuiltIn: true, Arguments: [var jsonArrayColumn] + } jsonEachExpression ], Predicate: null, GroupBy: [], @@ -529,16 +537,16 @@ protected override ShapedQueryExpression TransformJsonQueryToTable(JsonQueryExpr protected override bool IsNaturallyOrdered(SelectExpression selectExpression) { return selectExpression is - { - Tables: [var mainTable, ..], - Orderings: + { + Tables: [var mainTable, ..], + Orderings: [ - { - Expression: ColumnExpression { Name: JsonEachKeyColumnName } orderingColumn, - IsAscending: true - } + { + Expression: ColumnExpression { Name: JsonEachKeyColumnName } orderingColumn, + IsAscending: true + } ] - } + } && orderingColumn.TableAlias == mainTable.Alias && IsJsonEachKeyColumn(selectExpression, orderingColumn); diff --git a/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs b/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs index 61d27375771..f0c1e8d244b 100644 --- a/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs +++ b/src/EFCore.Sqlite.Core/Storage/Internal/SqliteRelationalConnection.cs @@ -179,6 +179,10 @@ private void InitializeDbConnection(DbConnection connection) ? value : sum.Value + value.Value, isDeterministic: true); + + sqliteConnection.CreateCollation( + "EF_DECIMAL", + (x, y) => decimal.Compare(decimal.Parse(x), decimal.Parse(y))); } else { From c8c0647b42fa5a618a78932d4de93562cb57ef02 Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Sun, 9 Feb 2025 00:55:42 +0100 Subject: [PATCH 4/5] Update tests --- .../BuiltInDataTypesSqliteTest.cs | 125 ++++++++++++++++-- 1 file changed, 113 insertions(+), 12 deletions(-) diff --git a/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs index f503df97a6a..614f53a34cf 100644 --- a/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/BuiltInDataTypesSqliteTest.cs @@ -1400,12 +1400,6 @@ public virtual void Cant_query_OrderBy_of_converted_types() .Where(e => e.PartitionId == 205); var ex = Assert.Throws( - () => query - .OrderBy(e => e.TestNullableDecimal) - .First()); - Assert.Equal(SqliteStrings.OrderByNotSupported("decimal"), ex.Message); - - ex = Assert.Throws( () => query .OrderBy(e => e.TestNullableDateTimeOffset) .First()); @@ -1457,12 +1451,6 @@ public virtual void Cant_query_ThenBy_of_converted_types() .OrderBy(e => e.PartitionId); var ex = Assert.Throws( - () => query - .ThenBy(e => e.TestNullableDecimal) - .First()); - Assert.Equal(SqliteStrings.OrderByNotSupported("decimal"), ex.Message); - - ex = Assert.Throws( () => query .ThenBy(e => e.TestNullableDateTimeOffset) .First()); @@ -1481,6 +1469,119 @@ public virtual void Cant_query_ThenBy_of_converted_types() Assert.Equal(SqliteStrings.OrderByNotSupported("ulong"), ex.Message); } + + [ConditionalFact] + public virtual void Can_query_OrderBy_of_converted_types() + { + using var context = CreateContext(); + var min = new BuiltInNullableDataTypes + { + Id = 221, + PartitionId = 207, + TestNullableDecimal = 2.000000000000001m, + TestNullableDateTimeOffset = new DateTimeOffset(2018, 1, 1, 12, 0, 0, TimeSpan.Zero), + TestNullableTimeSpan = TimeSpan.FromDays(2), + TestNullableUnsignedInt64 = 0 + }; + context.Add(min); + + var max = new BuiltInNullableDataTypes + { + Id = 222, + PartitionId = 207, + TestNullableDecimal = 10.000000000000001m, + TestNullableDateTimeOffset = new DateTimeOffset(2018, 1, 1, 11, 0, 0, TimeSpan.FromHours(-2)), + TestNullableTimeSpan = TimeSpan.FromDays(10), + TestNullableUnsignedInt64 = long.MaxValue + 1ul + }; + context.Add(max); + + context.SaveChanges(); + + Fixture.TestSqlLoggerFactory.Clear(); + + var query = context.Set() + .Where(e => e.PartitionId == 207); + + var results = query + .OrderBy(e => e.TestNullableDecimal) + .Select(e => e.Id) + .First(); + + AssertSql( + """ +SELECT "b"."Id" +FROM "BuiltInNullableDataTypes" AS "b" +WHERE "b"."PartitionId" = 207 +ORDER BY "b"."TestNullableDecimal" COLLATE EF_DECIMAL +LIMIT 1 +"""); + + var expectedResults = query.AsEnumerable() + .OrderBy(e => e.TestNullableDecimal) + .Select(e => e.Id) + .First(); + + Assert.Equal(expectedResults, results); + } + + [ConditionalFact] + public virtual void Can_query_ThenBy_of_converted_types() + { + using var context = CreateContext(); + var min = new BuiltInNullableDataTypes + { + Id = 223, + PartitionId = 208, + TestNullableDecimal = 2.000000000000001m, + TestNullableDateTimeOffset = new DateTimeOffset(2018, 1, 1, 12, 0, 0, TimeSpan.Zero), + TestNullableTimeSpan = TimeSpan.FromDays(2), + TestNullableUnsignedInt64 = 0 + }; + context.Add(min); + + var max = new BuiltInNullableDataTypes + { + Id = 224, + PartitionId = 208, + TestNullableDecimal = 10.000000000000001m, + TestNullableDateTimeOffset = new DateTimeOffset(2018, 1, 1, 11, 0, 0, TimeSpan.FromHours(-2)), + TestNullableTimeSpan = TimeSpan.FromDays(10), + TestNullableUnsignedInt64 = long.MaxValue + 1ul + }; + context.Add(max); + + context.SaveChanges(); + + Fixture.TestSqlLoggerFactory.Clear(); + + var query = context.Set() + .Where(e => e.PartitionId == 208); + + var results = query + .OrderBy(e => e.PartitionId) + .ThenBy(e => e.TestNullableDecimal) + .Select(e => e.Id) + .First(); + + AssertSql( + """ +SELECT "b"."Id" +FROM "BuiltInNullableDataTypes" AS "b" +WHERE "b"."PartitionId" = 208 +ORDER BY "b"."PartitionId", "b"."TestNullableDecimal" COLLATE EF_DECIMAL +LIMIT 1 +"""); + + var expectedResults = query.AsEnumerable() + .OrderBy(e => e.PartitionId) + .ThenBy(e => e.TestNullableDecimal) + .Select(e => e.Id) + .First(); + + Assert.Equal(expectedResults, results); + } + [ConditionalFact] public virtual void Can_query_using_char_ToLower() { From b23c8bb42f2496d963b39157a143d24d231fb30b Mon Sep 17 00:00:00 2001 From: Andrea Canciani Date: Sun, 9 Feb 2025 00:56:46 +0100 Subject: [PATCH 5/5] Use `decimal` directly in Northwind SQLite tests They are now supported sufficiently well. --- .../Query/NorthwindFunctionsQuerySqliteTest.cs | 2 +- .../Query/NorthwindQuerySqliteFixture.cs | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs index 33e82557551..900b986995f 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindFunctionsQuerySqliteTest.cs @@ -28,7 +28,7 @@ public override async Task Client_evaluation_of_uncorrelated_method_call(bool as """ SELECT "o"."OrderID", "o"."ProductID", "o"."Discount", "o"."Quantity", "o"."UnitPrice" FROM "Order Details" AS "o" -WHERE "o"."UnitPrice" < 7.0 AND 10 < "o"."ProductID" +WHERE ef_compare("o"."UnitPrice", '7.0') < 0 AND 10 < "o"."ProductID" """); } diff --git a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindQuerySqliteFixture.cs b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindQuerySqliteFixture.cs index 5bc28e08eca..88feedba963 100644 --- a/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindQuerySqliteFixture.cs +++ b/test/EFCore.Sqlite.FunctionalTests/Query/NorthwindQuerySqliteFixture.cs @@ -13,15 +13,6 @@ public class NorthwindQuerySqliteFixture : NorthwindQueryRelat protected override ITestStoreFactory TestStoreFactory => SqliteNorthwindTestStoreFactory.Instance; - protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context) - { - base.OnModelCreating(modelBuilder, context); - - // NB: SQLite doesn't support decimal very well. Using double instead - modelBuilder.Entity().Property(o => o.UnitPrice).HasConversion(); - modelBuilder.Entity().Property(o => o.UnitPrice).HasConversion(); - } - protected override Type ContextType => typeof(NorthwindSqliteContext); }