From 8d61e5ac406ef87d6ac48ecabf93f1bcd2affd3a Mon Sep 17 00:00:00 2001 From: Victor Irzak Date: Thu, 20 Aug 2026 11:43:17 -0400 Subject: [PATCH 1/2] Fix analyzer diagnostics failing the build The build failed with 19 analyzer diagnostics promoted to errors by TreatWarningsAsErrors. All are style/quality rules; no behavior changes. - IDE0046: fold the DeclaringType guard into the switch expression in BinaryTranslator and WindowFunctionsTranslator. - IDE0028: back ChainedSqlExpression.List with a List field instead of casting the IReadOnlyList back to List on every Add. - CA1860: use Count > 0 instead of Any() on Partitions/Orderings. - IDE0031: use null-conditional assignment (x?[i] = v) in WindowFunctionsSqlNullabilityProcessorHelper. - IDE0370: drop null-forgiving operators that are no longer needed on WindowFrame.ToString() and TypeBuilder.CreateType(). - IDE0305: use a collection expression instead of ToList(). - IDE0290: suppress on constructors inside #if branches, which cannot be primary constructors, matching the existing suppressions in those files. - IDE0120: use Sum(g => g.Count()) instead of Select(...).Sum(). Build is clean (0 warnings, 0 errors). The three SQLite-backed test projects pass; the SQL Server and Npgsql ones need live databases and were not run. --- .../Query/Internal/BinaryTranslator.cs | 27 +++---- ...ctionsNpgsqlEvaluatableExpressionFilter.cs | 1 + ...ServerParameterBasedSqlProcessorFactory.cs | 1 + ...nctionsSqlServerSqlNullabilityProcessor.cs | 1 + .../Internal/ExpressionVisitorExtensions.cs | 6 +- .../WindowFunctionInsideWhereDetector.cs | 4 +- ...wFunctionsSqlNullabilityProcessorHelper.cs | 15 +--- .../Internal/WindowFunctionsTranslator.cs | 77 +++++++++---------- .../SqlExpressions/ChainedSqlExpression.cs | 6 +- .../SqlExpressions/RowOrRangeExpression.cs | 2 +- .../WindowFunctionExpression.cs | 4 +- .../RankTests.cs | 8 +- 12 files changed, 70 insertions(+), 82 deletions(-) diff --git a/src/Zomp.EFCore.BinaryFunctions/Query/Internal/BinaryTranslator.cs b/src/Zomp.EFCore.BinaryFunctions/Query/Internal/BinaryTranslator.cs index 11171d6..f4d6fa6 100644 --- a/src/Zomp.EFCore.BinaryFunctions/Query/Internal/BinaryTranslator.cs +++ b/src/Zomp.EFCore.BinaryFunctions/Query/Internal/BinaryTranslator.cs @@ -19,21 +19,18 @@ public class BinaryTranslator(ISqlExpressionFactory sqlExpressionFactory, IRelat { ArgumentNullException.ThrowIfNull(method); - if (method.DeclaringType != typeof(DbFunctionsExtensions)) - { - return null; - } - - return method.Name switch - { - nameof(DbFunctionsExtensions.GetBytes) => GetBytes(arguments[1]), - nameof(DbFunctionsExtensions.Concat) => Concat(arguments), - nameof(DbFunctionsExtensions.Substring) => Substring(arguments[1], arguments[2], arguments[3]), - nameof(DbFunctionsExtensions.ToValue) when arguments.Count > 2 => ToValue(arguments[1], arguments[2], method.GetGenericArguments()[0]), - nameof(DbFunctionsExtensions.ToValue) => ToValue(arguments[1], method.GetGenericArguments()[0]), - nameof(DbFunctionsExtensions.BinaryCast) => BinaryCast(arguments[1], method.GetGenericArguments()[1]), - _ => null, - }; + return method.DeclaringType != typeof(DbFunctionsExtensions) + ? null + : method.Name switch + { + nameof(DbFunctionsExtensions.GetBytes) => GetBytes(arguments[1]), + nameof(DbFunctionsExtensions.Concat) => Concat(arguments), + nameof(DbFunctionsExtensions.Substring) => Substring(arguments[1], arguments[2], arguments[3]), + nameof(DbFunctionsExtensions.ToValue) when arguments.Count > 2 => ToValue(arguments[1], arguments[2], method.GetGenericArguments()[0]), + nameof(DbFunctionsExtensions.ToValue) => ToValue(arguments[1], method.GetGenericArguments()[0]), + nameof(DbFunctionsExtensions.BinaryCast) => BinaryCast(arguments[1], method.GetGenericArguments()[1]), + _ => null, + }; } /// diff --git a/src/Zomp.EFCore.WindowFunctions.Npgsql/Query/Internal/WindowFunctionsNpgsqlEvaluatableExpressionFilter.cs b/src/Zomp.EFCore.WindowFunctions.Npgsql/Query/Internal/WindowFunctionsNpgsqlEvaluatableExpressionFilter.cs index d1728f8..ab05a46 100644 --- a/src/Zomp.EFCore.WindowFunctions.Npgsql/Query/Internal/WindowFunctionsNpgsqlEvaluatableExpressionFilter.cs +++ b/src/Zomp.EFCore.WindowFunctions.Npgsql/Query/Internal/WindowFunctionsNpgsqlEvaluatableExpressionFilter.cs @@ -15,6 +15,7 @@ public class WindowFunctionsNpgsqlEvaluatableExpressionFilter : NpgsqlEvaluatabl /// Service dependencies. /// Relational service dependencies. /// NpgSql Singleton Options. + [SuppressMessage("Style", "IDE0290:Use primary constructor", Justification = "Conditional compilation")] public WindowFunctionsNpgsqlEvaluatableExpressionFilter(EvaluatableExpressionFilterDependencies dependencies, RelationalEvaluatableExpressionFilterDependencies relationalDependencies, INpgsqlSingletonOptions npgsqlSingletonOptions) : base(dependencies, relationalDependencies, npgsqlSingletonOptions) { diff --git a/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerParameterBasedSqlProcessorFactory.cs b/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerParameterBasedSqlProcessorFactory.cs index d2c2c4f..fe4ccb8 100644 --- a/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerParameterBasedSqlProcessorFactory.cs +++ b/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerParameterBasedSqlProcessorFactory.cs @@ -15,6 +15,7 @@ public class WindowFunctionsSqlServerParameterBasedSqlProcessorFactory : SqlServ /// /// Service dependencies. /// The singleton option. + [SuppressMessage("Style", "IDE0290:Use primary constructor", Justification = "Conditional compilation")] public WindowFunctionsSqlServerParameterBasedSqlProcessorFactory(RelationalParameterBasedSqlProcessorDependencies dependencies, ISqlServerSingletonOptions sqlServerSingletonOptions) : base(dependencies, sqlServerSingletonOptions) { diff --git a/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerSqlNullabilityProcessor.cs b/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerSqlNullabilityProcessor.cs index d980b5e..22a2c07 100644 --- a/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerSqlNullabilityProcessor.cs +++ b/src/Zomp.EFCore.WindowFunctions.SqlServer/Query/Internal/WindowFunctionsSqlServerSqlNullabilityProcessor.cs @@ -15,6 +15,7 @@ public class WindowFunctionsSqlServerSqlNullabilityProcessor : SqlServerSqlNulla /// Relational Parameter Based Sql Processor Dependencies. /// Relational parameters. /// The singleton option. + [SuppressMessage("Style", "IDE0290:Use primary constructor", Justification = "Conditional compilation")] public WindowFunctionsSqlServerSqlNullabilityProcessor(RelationalParameterBasedSqlProcessorDependencies dependencies, RelationalParameterBasedSqlProcessorParameters parameters, ISqlServerSingletonOptions sqlServerSingletonOptions) : base(dependencies, parameters, sqlServerSingletonOptions) { diff --git a/src/Zomp.EFCore.WindowFunctions/Query/Internal/ExpressionVisitorExtensions.cs b/src/Zomp.EFCore.WindowFunctions/Query/Internal/ExpressionVisitorExtensions.cs index b759130..27d3316 100644 --- a/src/Zomp.EFCore.WindowFunctions/Query/Internal/ExpressionVisitorExtensions.cs +++ b/src/Zomp.EFCore.WindowFunctions/Query/Internal/ExpressionVisitorExtensions.cs @@ -47,14 +47,14 @@ public static Expression VisitWindowFunction(this ExpressionVisitor expressionVi } _ = relationalCommandBuilder.Append("OVER("); - if (windowFunctionExpression.Partitions.Any()) + if (windowFunctionExpression.Partitions.Count > 0) { _ = relationalCommandBuilder.Append("PARTITION BY "); GenerateList(relationalCommandBuilder, windowFunctionExpression.Partitions, e => expressionVisitor.Visit(e)); _ = relationalCommandBuilder.Append(" "); } - if (windowFunctionExpression.Orderings.Any()) + if (windowFunctionExpression.Orderings.Count > 0) { _ = relationalCommandBuilder.Append("ORDER BY "); GenerateList(relationalCommandBuilder, windowFunctionExpression.Orderings, e => expressionVisitor.Visit(e)); @@ -92,7 +92,7 @@ private static void ProcessRowOrRange(WindowFunctionExpression windowFunctionExp private static void ProcessWindowFrame(IRelationalCommandBuilder relationalCommandBuilder, WindowFrame windowFrame, bool isStart) { - _ = relationalCommandBuilder.Append(windowFrame.ToString()!); + _ = relationalCommandBuilder.Append(windowFrame.ToString()); if (windowFrame.IsDirectional) { diff --git a/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionInsideWhereDetector.cs b/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionInsideWhereDetector.cs index ddde0ef..8615c0a 100644 --- a/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionInsideWhereDetector.cs +++ b/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionInsideWhereDetector.cs @@ -229,7 +229,7 @@ private static LambdaExpression BuildSubqueries( } var replacing = windowFunctions[level]; - subqueryList = replacing.Select(z => (MethodCallExpression)wfr.Visit(z)).ToList(); + subqueryList = [.. replacing.Select(z => (MethodCallExpression)wfr.Visit(z))]; } var newBody = wfr.Visit(lambda.Body); @@ -302,7 +302,7 @@ private static Type CreateNewType(IEnumerable info) cil.Emit(OpCodes.Ret); // Return the type to the caller - return dynamicAnonymousType.CreateType()!; + return dynamicAnonymousType.CreateType(); } private static MethodInfo GetMethod(string name, int genericParameterCount, Func parameterGenerator) diff --git a/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsSqlNullabilityProcessorHelper.cs b/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsSqlNullabilityProcessorHelper.cs index 291c578..efd6f1d 100644 --- a/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsSqlNullabilityProcessorHelper.cs +++ b/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsSqlNullabilityProcessorHelper.cs @@ -33,10 +33,7 @@ public static WindowFunctionExpression VisitWindowFunction( } } - if (arguments is not null) - { - arguments[i] = visitedArgument; - } + arguments?[i] = visitedArgument; } SqlExpression[]? partitions = null; @@ -54,10 +51,7 @@ public static WindowFunctionExpression VisitWindowFunction( } } - if (partitions is not null) - { - partitions[i] = visitedPartition!; - } + partitions?[i] = visitedPartition!; } OrderingExpression[]? orderings = null; @@ -75,10 +69,7 @@ public static WindowFunctionExpression VisitWindowFunction( } } - if (orderings is not null) - { - orderings[i] = visitedOrdering; - } + orderings?[i] = visitedOrdering; } return arguments is not null || orderings is not null || partitions is not null diff --git a/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsTranslator.cs b/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsTranslator.cs index cef4fe8..e4b0c4a 100644 --- a/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsTranslator.cs +++ b/src/Zomp.EFCore.WindowFunctions/Query/Internal/WindowFunctionsTranslator.cs @@ -14,46 +14,43 @@ public class WindowFunctionsTranslator(ISqlExpressionFactory sqlExpressionFactor /// public SqlExpression? Translate(SqlExpression? instance, MethodInfo method, IReadOnlyList arguments, IDiagnosticsLogger logger) { - if (method.DeclaringType != typeof(DbFunctionsExtensions)) - { - return null; - } - - return method.Name switch - { - nameof(DbFunctionsExtensions.Min) => Parse(arguments, "MIN"), - nameof(DbFunctionsExtensions.Max) => Parse(arguments, "MAX"), - nameof(DbFunctionsExtensions.Lead) => Parse(arguments, "LEAD"), - nameof(DbFunctionsExtensions.Lag) => Parse(arguments, "LAG"), - nameof(DbFunctionsExtensions.Sum) => Parse(arguments, "SUM"), - nameof(DbFunctionsExtensions.Avg) => Parse(arguments, "AVG"), - nameof(DbFunctionsExtensions.Count) => Parse(arguments, "COUNT"), - nameof(DbFunctionsExtensions.RowNumber) => Parse(arguments, "ROW_NUMBER"), - nameof(DbFunctionsExtensions.Rank) => Parse(arguments, "RANK"), - nameof(DbFunctionsExtensions.DenseRank) => Parse(arguments, "DENSE_RANK"), - nameof(DbFunctionsExtensions.PercentRank) => Parse(arguments, "PERCENT_RANK"), - - nameof(DbFunctionsExtensions.OrderBy) => OrderBy(arguments, true), - nameof(DbFunctionsExtensions.OrderByDescending) => OrderBy(arguments, false), - nameof(DbFunctionsExtensions.PartitionBy) => PartitionBy(arguments), - nameof(DbFunctionsExtensions.ThenBy) => ThenBy(arguments, true), - nameof(DbFunctionsExtensions.ThenByDescending) => ThenBy(arguments, false), - - nameof(DbFunctionsExtensions.Rows) => RowsOrRange(arguments, true), - nameof(DbFunctionsExtensions.Range) => RowsOrRange(arguments, false), - - nameof(DbFunctionsExtensions.FromPreceding) => From(arguments, false), - nameof(DbFunctionsExtensions.FromFollowing) => From(arguments, true), - nameof(DbFunctionsExtensions.FromCurrentRow) => FromWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.CurrentRow), - nameof(DbFunctionsExtensions.FromUnbounded) => FromWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.Unbounded), - - nameof(DbFunctionsExtensions.ToFollowing) => To(arguments, true), - nameof(DbFunctionsExtensions.ToCurrentRow) => ToWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.CurrentRow), - nameof(DbFunctionsExtensions.ToUnbounded) => ToWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.Unbounded), - nameof(DbFunctionsExtensions.ToPreceding) => To(arguments, false), - - _ => null, - }; + return method.DeclaringType != typeof(DbFunctionsExtensions) + ? null + : method.Name switch + { + nameof(DbFunctionsExtensions.Min) => Parse(arguments, "MIN"), + nameof(DbFunctionsExtensions.Max) => Parse(arguments, "MAX"), + nameof(DbFunctionsExtensions.Lead) => Parse(arguments, "LEAD"), + nameof(DbFunctionsExtensions.Lag) => Parse(arguments, "LAG"), + nameof(DbFunctionsExtensions.Sum) => Parse(arguments, "SUM"), + nameof(DbFunctionsExtensions.Avg) => Parse(arguments, "AVG"), + nameof(DbFunctionsExtensions.Count) => Parse(arguments, "COUNT"), + nameof(DbFunctionsExtensions.RowNumber) => Parse(arguments, "ROW_NUMBER"), + nameof(DbFunctionsExtensions.Rank) => Parse(arguments, "RANK"), + nameof(DbFunctionsExtensions.DenseRank) => Parse(arguments, "DENSE_RANK"), + nameof(DbFunctionsExtensions.PercentRank) => Parse(arguments, "PERCENT_RANK"), + + nameof(DbFunctionsExtensions.OrderBy) => OrderBy(arguments, true), + nameof(DbFunctionsExtensions.OrderByDescending) => OrderBy(arguments, false), + nameof(DbFunctionsExtensions.PartitionBy) => PartitionBy(arguments), + nameof(DbFunctionsExtensions.ThenBy) => ThenBy(arguments, true), + nameof(DbFunctionsExtensions.ThenByDescending) => ThenBy(arguments, false), + + nameof(DbFunctionsExtensions.Rows) => RowsOrRange(arguments, true), + nameof(DbFunctionsExtensions.Range) => RowsOrRange(arguments, false), + + nameof(DbFunctionsExtensions.FromPreceding) => From(arguments, false), + nameof(DbFunctionsExtensions.FromFollowing) => From(arguments, true), + nameof(DbFunctionsExtensions.FromCurrentRow) => FromWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.CurrentRow), + nameof(DbFunctionsExtensions.FromUnbounded) => FromWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.Unbounded), + + nameof(DbFunctionsExtensions.ToFollowing) => To(arguments, true), + nameof(DbFunctionsExtensions.ToCurrentRow) => ToWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.CurrentRow), + nameof(DbFunctionsExtensions.ToUnbounded) => ToWindowFrame(GetOrderingSqlExpression(arguments), WindowFrame.Unbounded), + nameof(DbFunctionsExtensions.ToPreceding) => To(arguments, false), + + _ => null, + }; } /// diff --git a/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/ChainedSqlExpression.cs b/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/ChainedSqlExpression.cs index 2229bd6..1abda0e 100644 --- a/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/ChainedSqlExpression.cs +++ b/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/ChainedSqlExpression.cs @@ -3,9 +3,11 @@ internal abstract class ChainedSqlExpression(T first) : SqlExpression(typeof(ChainedSqlExpression), null) where T : Expression { - public IReadOnlyList List { get; } = new List([first]); + private readonly List list = [first]; - public void Add(T item) => ((List)List).Add(item); + public IReadOnlyList List => list; + + public void Add(T item) => list.Add(item); protected override void Print(ExpressionPrinter expressionPrinter) => expressionPrinter.VisitCollection(List); } \ No newline at end of file diff --git a/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/RowOrRangeExpression.cs b/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/RowOrRangeExpression.cs index 371fcfd..6ccd61d 100644 --- a/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/RowOrRangeExpression.cs +++ b/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/RowOrRangeExpression.cs @@ -72,7 +72,7 @@ protected override void Print(ExpressionPrinter expressionPrinter) private static void ProcessWindowFrame(ExpressionPrinter expressionPrinter, WindowFrame windowFrame, bool isStart) { - _ = expressionPrinter.Append(windowFrame.ToString()!); + _ = expressionPrinter.Append(windowFrame.ToString()); if (windowFrame.IsDirectional) { diff --git a/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/WindowFunctionExpression.cs b/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/WindowFunctionExpression.cs index 3e087c5..858b9a3 100644 --- a/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/WindowFunctionExpression.cs +++ b/src/Zomp.EFCore.WindowFunctions/Query/SqlExpressions/WindowFunctionExpression.cs @@ -169,14 +169,14 @@ protected override void Print(ExpressionPrinter expressionPrinter) _ = expressionPrinter.Append("OVER("); - if (Partitions.Any()) + if (Partitions.Count > 0) { _ = expressionPrinter.Append("PARTITION BY "); expressionPrinter.VisitCollection(Partitions); _ = expressionPrinter.Append(" "); } - if (Orderings.Any()) + if (Orderings.Count > 0) { _ = expressionPrinter.Append("ORDER BY "); expressionPrinter.VisitCollection(Orderings); diff --git a/tests/Zomp.EFCore.WindowFunctions.Testing/RankTests.cs b/tests/Zomp.EFCore.WindowFunctions.Testing/RankTests.cs index 156c045..8513d27 100644 --- a/tests/Zomp.EFCore.WindowFunctions.Testing/RankTests.cs +++ b/tests/Zomp.EFCore.WindowFunctions.Testing/RankTests.cs @@ -1,4 +1,4 @@ -namespace Zomp.EFCore.WindowFunctions.Testing; +namespace Zomp.EFCore.WindowFunctions.Testing; public partial class RankTests { @@ -46,8 +46,7 @@ public void RankBasic() .Select(r => r.Id / 10) .Select(v => (long)groups .Where(g => g.Key < v) - .Select(g => g.Count()) - .Sum() + 1); + .Sum(g => g.Count()) + 1); Assert.Equal(expectedSequence, result); } @@ -86,8 +85,7 @@ public void PercentRankBasic() .OrderBy(x => x, comparer) .Select(v => groups .Where(g => comparer.Compare(g.Key, v) < 0) - .Select(g => g.Count()) - .Sum() / (double)(TestRows.Length - 1)); + .Sum(g => g.Count()) / (double)(TestRows.Length - 1)); Assert.Equal(expectedSequence, result.Select(r => r)); } From 578c241ecef108f95e420f166f73d141c80371c4 Mon Sep 17 00:00:00 2001 From: Victor Irzak Date: Sun, 2 Aug 2026 14:13:46 -0400 Subject: [PATCH 2/2] Fix #23 Use SQLite's query SQL generator for window functions UseWindowFunctions() registered the provider agnostic WindowQuerySqlGeneratorFactory for SQLite, so queries compiled with the generic relational generator instead of SqliteQuerySqlGenerator. That produced ANSI OFFSET/FETCH paging, which SQLite rejects with 'near "FETCH": syntax error', breaking First/Single/Take/Skip on any query once window functions were enabled - even queries containing no window function at all. Add WindowFunctionsSqliteQuerySqlGenerator deriving from SqliteQuerySqlGenerator, matching what the SQL Server and Npgsql packages already do, and register its factory instead. Add PagingTests to the shared testing project so First/Take/Skip are covered on all three providers, plus a SQLite specific assertion that the generated SQL uses LIMIT and not FETCH. SubQueryTests.RowNumberWithSingle was skipped on SQLite with a "Fixme: investigate why this fails" note. It failed for this same reason and now passes, so the skip is removed. --- ...SqliteDbContextOptionsBuilderExtensions.cs | 2 +- .../WindowFunctionsSqliteQuerySqlGenerator.cs | 25 +++++++++ ...FunctionsSqliteQuerySqlGeneratorFactory.cs | 18 ++++++ .../Partials.cs | 3 + .../Partials.cs | 3 + .../Partials.cs | 3 + .../SqliteSpecificTests.cs | 22 ++++++++ .../PagingTests.cs | 55 +++++++++++++++++++ .../SubQueryTests.cs | 4 +- ...p.EFCore.WindowFunctions.Testing.projitems | 1 + 10 files changed, 132 insertions(+), 4 deletions(-) create mode 100644 src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGenerator.cs create mode 100644 src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGeneratorFactory.cs create mode 100644 tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/SqliteSpecificTests.cs create mode 100644 tests/Zomp.EFCore.WindowFunctions.Testing/PagingTests.cs diff --git a/src/Zomp.EFCore.WindowFunctions.Sqlite/Extensions/SqliteDbContextOptionsBuilderExtensions.cs b/src/Zomp.EFCore.WindowFunctions.Sqlite/Extensions/SqliteDbContextOptionsBuilderExtensions.cs index 8e9a93b..f8dae89 100644 --- a/src/Zomp.EFCore.WindowFunctions.Sqlite/Extensions/SqliteDbContextOptionsBuilderExtensions.cs +++ b/src/Zomp.EFCore.WindowFunctions.Sqlite/Extensions/SqliteDbContextOptionsBuilderExtensions.cs @@ -28,7 +28,7 @@ private static SqliteDbContextOptionsBuilder AddOrUpdateExtension( IRelationalParameterBasedSqlProcessorFactory, WindowFunctionsSqliteParameterBasedSqlProcessorFactory >() - .ReplaceService() + .ReplaceService() .ReplaceService() .ReplaceService() .ReplaceService() diff --git a/src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGenerator.cs b/src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGenerator.cs new file mode 100644 index 0000000..585f586 --- /dev/null +++ b/src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGenerator.cs @@ -0,0 +1,25 @@ +namespace Zomp.EFCore.WindowFunctions.Sqlite.Query.Internal; + +/// +/// A query SQL generator for window functions to get for given . +/// +[System.Diagnostics.CodeAnalysis.SuppressMessage("Style", "IDE0290:Use primary constructor", Justification = "Multiple versions")] +public class WindowFunctionsSqliteQuerySqlGenerator : SqliteQuerySqlGenerator +{ + /// + /// Initializes a new instance of the class. + /// + /// Service dependencies. + public WindowFunctionsSqliteQuerySqlGenerator(QuerySqlGeneratorDependencies dependencies) + : base(dependencies) + { + } + + /// + protected override Expression VisitExtension(Expression extensionExpression) + => extensionExpression switch + { + WindowFunctionExpression windowFunctionExpression => this.VisitWindowFunction(windowFunctionExpression), + _ => base.VisitExtension(extensionExpression), + }; +} diff --git a/src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGeneratorFactory.cs b/src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGeneratorFactory.cs new file mode 100644 index 0000000..d7441ac --- /dev/null +++ b/src/Zomp.EFCore.WindowFunctions.Sqlite/Query/Internal/WindowFunctionsSqliteQuerySqlGeneratorFactory.cs @@ -0,0 +1,18 @@ +namespace Zomp.EFCore.WindowFunctions.Sqlite.Query.Internal; + +/// +/// A factory for creating instances. +/// +/// +/// Initializes a new instance of the class. +/// +/// Query Sql Generator Dependencies. +public class WindowFunctionsSqliteQuerySqlGeneratorFactory(QuerySqlGeneratorDependencies dependencies) + : SqliteQuerySqlGeneratorFactory(dependencies) +{ + private readonly QuerySqlGeneratorDependencies dependencies = dependencies; + + /// + public override QuerySqlGenerator Create() + => new WindowFunctionsSqliteQuerySqlGenerator(dependencies); +} diff --git a/tests/Zomp.EFCore.WindowFunctions.Npgsql.Tests/Partials.cs b/tests/Zomp.EFCore.WindowFunctions.Npgsql.Tests/Partials.cs index 38ecb1f..eb91abd 100644 --- a/tests/Zomp.EFCore.WindowFunctions.Npgsql.Tests/Partials.cs +++ b/tests/Zomp.EFCore.WindowFunctions.Npgsql.Tests/Partials.cs @@ -32,4 +32,7 @@ public partial class AnalyticTests(ITestOutputHelper output) : TestBase(output) [Collection(nameof(NpgsqlCollection))] public partial class SubQueryTests(ITestOutputHelper output) : TestBase(output) { } + +[Collection(nameof(NpgsqlCollection))] +public partial class PagingTests(ITestOutputHelper output) : TestBase(output) { } #pragma warning restore SA1402 // File may only contain a single type diff --git a/tests/Zomp.EFCore.WindowFunctions.SqlServer.Tests/Partials.cs b/tests/Zomp.EFCore.WindowFunctions.SqlServer.Tests/Partials.cs index 016e518..dc7c79d 100644 --- a/tests/Zomp.EFCore.WindowFunctions.SqlServer.Tests/Partials.cs +++ b/tests/Zomp.EFCore.WindowFunctions.SqlServer.Tests/Partials.cs @@ -32,4 +32,7 @@ public partial class AnalyticTests(ITestOutputHelper output) : TestBase(output) [Collection(nameof(SqlServerCollection))] public partial class SubQueryTests(ITestOutputHelper output) : TestBase(output) { } + +[Collection(nameof(SqlServerCollection))] +public partial class PagingTests(ITestOutputHelper output) : TestBase(output) { } #pragma warning restore SA1402 // File may only contain a single type diff --git a/tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/Partials.cs b/tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/Partials.cs index 445e140..877400b 100644 --- a/tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/Partials.cs +++ b/tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/Partials.cs @@ -32,4 +32,7 @@ public partial class AnalyticTests(ITestOutputHelper output) : TestBase(output) [Collection(nameof(SqliteCollection))] public partial class SubQueryTests(ITestOutputHelper output) : TestBase(output) { } + +[Collection(nameof(SqliteCollection))] +public partial class PagingTests(ITestOutputHelper output) : TestBase(output) { } #pragma warning restore SA1402 // File may only contain a single type diff --git a/tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/SqliteSpecificTests.cs b/tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/SqliteSpecificTests.cs new file mode 100644 index 0000000..c77e3f7 --- /dev/null +++ b/tests/Zomp.EFCore.WindowFunctions.Sqlite.Tests/SqliteSpecificTests.cs @@ -0,0 +1,22 @@ +namespace Zomp.EFCore.WindowFunctions.Sqlite.Tests; + +[Collection(nameof(SqliteCollection))] +public class SqliteSpecificTests(ITestOutputHelper output) : TestBase(output) +{ + /// + /// Ensures paging is generated with SQLite's LIMIT rather than ANSI OFFSET ... FETCH. + /// + /// + /// https://github.com/zompinc/efcore-extensions/issues/23. Enabling window functions used to + /// replace the SQLite query SQL generator with the provider agnostic one, which broke every + /// query using Take / Skip / First, whether or not it contained a window function. + /// + [Fact] + public void Issue23PagingUsesLimit() + { + var sql = DbContext.TestRows.OrderBy(r => r.Id).Take(1).ToQueryString(); + + Assert.Contains("LIMIT", sql, StringComparison.Ordinal); + Assert.DoesNotContain("FETCH", sql, StringComparison.Ordinal); + } +} diff --git a/tests/Zomp.EFCore.WindowFunctions.Testing/PagingTests.cs b/tests/Zomp.EFCore.WindowFunctions.Testing/PagingTests.cs new file mode 100644 index 0000000..bd760fe --- /dev/null +++ b/tests/Zomp.EFCore.WindowFunctions.Testing/PagingTests.cs @@ -0,0 +1,55 @@ +namespace Zomp.EFCore.WindowFunctions.Testing; + +/// +/// Verifies that enabling window functions leaves provider specific paging SQL intact. +/// +/// +/// Regression tests for https://github.com/zompinc/efcore-extensions/issues/23, where the SQLite +/// provider fell back to the generic query SQL generator and emitted ANSI +/// OFFSET ... FETCH NEXT instead of SQLite's LIMIT. +/// +public partial class PagingTests +{ + [Fact] + public void First() + { + var result = DbContext.TestRows.OrderBy(r => r.Id).First(); + + var expected = TestRows.OrderBy(r => r.Id).First(); + + Assert.Equal(expected, result, TestRowEqualityComparer.Default); + } + + [Fact] + public void Take() + { + var result = DbContext.TestRows.OrderBy(r => r.Id).Take(3).ToList(); + + var expected = TestRows.OrderBy(r => r.Id).Take(3); + + Assert.Equal(expected, result, TestRowEqualityComparer.Default); + } + + [Fact] + public void SkipAndTake() + { + var result = DbContext.TestRows.OrderBy(r => r.Id).Skip(2).Take(3).ToList(); + + var expected = TestRows.OrderBy(r => r.Id).Skip(2).Take(3); + + Assert.Equal(expected, result, TestRowEqualityComparer.Default); + } + + [Fact] + public void TakeWithWindowFunction() + { + var query = DbContext.TestRows + .OrderBy(r => r.Id) + .Select(r => EF.Functions.RowNumber(EF.Functions.Over().OrderBy(r.Id))) + .Take(3); + + var result = query.ToList(); + + Assert.Equal([1L, 2L, 3L], result); + } +} diff --git a/tests/Zomp.EFCore.WindowFunctions.Testing/SubQueryTests.cs b/tests/Zomp.EFCore.WindowFunctions.Testing/SubQueryTests.cs index 58012eb..ea74781 100644 --- a/tests/Zomp.EFCore.WindowFunctions.Testing/SubQueryTests.cs +++ b/tests/Zomp.EFCore.WindowFunctions.Testing/SubQueryTests.cs @@ -15,11 +15,9 @@ public void RowNumberWithWhere() Assert.Equal(expected, result.Single(), TestRowEqualityComparer.Default); } - [SkippableFact] + [Fact] public void RowNumberWithSingle() { - // Fixme: investigate why this fails. - Skip.If(DbContext.IsSqlite); var result = DbContext.TestRows .Single(t => EF.Functions.RowNumber(EF.Functions.Over().OrderBy(t.Id)) == 1); diff --git a/tests/Zomp.EFCore.WindowFunctions.Testing/Zomp.EFCore.WindowFunctions.Testing.projitems b/tests/Zomp.EFCore.WindowFunctions.Testing/Zomp.EFCore.WindowFunctions.Testing.projitems index edcbf2c..332f3c2 100644 --- a/tests/Zomp.EFCore.WindowFunctions.Testing/Zomp.EFCore.WindowFunctions.Testing.projitems +++ b/tests/Zomp.EFCore.WindowFunctions.Testing/Zomp.EFCore.WindowFunctions.Testing.projitems @@ -17,6 +17,7 @@ +