Add support for SQL window functions in ExpressiveSharp.EntityFrameworkCore#10
Add support for SQL window functions in ExpressiveSharp.EntityFrameworkCore#10
Conversation
…pressiveSharp.EntityFrameworkCore
There was a problem hiding this comment.
Pull request overview
Adds a new relational extension package to the ExpressiveSharp EF Core integration, enabling SQL window functions and indexed Select support via an opt-in plugin model (UseExpressives(o => o.UseRelationalExtensions())).
Changes:
- Introduces
ExpressiveSharp.EntityFrameworkCore.Relationalwith window function stubs (WindowFunction,Window) and EF Core translation/rendering infrastructure. - Adds a plugin system to
ExpressiveSharp.EntityFrameworkCore(IExpressivePlugin+ExpressiveOptionsBuilder) so relational features can be enabled selectively. - Extends the interceptor generator and stubs to support
Select((elem, index) => ...), plus a transformer that rewrites indexed selects toROW_NUMBER().
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ExpressiveSharp.EntityFrameworkCore.Relational.Tests/WindowFunctionTests.cs | New SQL-shape + integration tests for window functions and indexed Select. |
| tests/ExpressiveSharp.EntityFrameworkCore.Relational.Tests/Models/WindowTestDbContext.cs | Minimal EF Core model/context for relational window-function tests. |
| tests/ExpressiveSharp.EntityFrameworkCore.Relational.Tests/Models/TestModels.cs | Test entities (Customer, Order). |
| tests/ExpressiveSharp.EntityFrameworkCore.Relational.Tests/ExpressiveSharp.EntityFrameworkCore.Relational.Tests.csproj | New MSTest project wiring for relational tests. |
| src/ExpressiveSharp/Extensions/RewritableQueryableLinqExtensions.cs | Adds delegate-stub overload for indexed Select on IRewritableQueryable<T>. |
| src/ExpressiveSharp.Generator/PolyfillInterceptorGenerator.cs | Adds interceptor emission for indexed Select(Func<T,int,TResult>). |
| src/ExpressiveSharp.EntityFrameworkCore/Infrastructure/Internal/ExpressiveOptionsExtension.cs | Adds plugin application and plugin-based EF internal service provider hashing. |
| src/ExpressiveSharp.EntityFrameworkCore/IExpressivePlugin.cs | New plugin interface for registering services + transformers. |
| src/ExpressiveSharp.EntityFrameworkCore/Extensions/DbContextOptionsExtensions.cs | Adds UseExpressives(Action<ExpressiveOptionsBuilder>) overload for plugin configuration. |
| src/ExpressiveSharp.EntityFrameworkCore/ExpressiveOptionsBuilder.cs | New builder used by UseExpressives(...) to register plugins. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/WindowFunctions/WindowFunction.cs | Public window function stubs (translated to SQL). |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/WindowFunctions/WindowDefinition.cs | Marker/fluent API for building window specs in expression trees. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/WindowFunctions/Window.cs | Static entry point for window spec creation. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Transformers/RewriteIndexedSelectToRowNumber.cs | Rewrites indexed Queryable.Select to ROW_NUMBER() - 1. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/RelationalExpressivePlugin.cs | Registers relational translation services + transformers via plugin. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowSpecSqlExpression.cs | Intermediate SQL node carrying PARTITION/ORDER clauses. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowSpecMethodCallTranslator.cs | Translates Window/WindowDefinition calls into WindowSpecSqlExpression. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowFunctionTranslatorPlugin.cs | Registers method-call translators into EF Core pipeline. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowFunctionSqlNullabilityProcessor.cs | Marks window function SQL nodes as non-nullable during nullability processing. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowFunctionSqlExpression.cs | Custom SQL expression type for RANK/DENSE_RANK/NTILE window functions. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowFunctionParameterBasedSqlProcessorFactory.cs | Factory wiring for custom parameter-based SQL processor. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowFunctionParameterBasedSqlProcessor.cs | Hooks custom nullability processor into EF’s relational SQL processing. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowFunctionMethodCallTranslator.cs | Translates WindowFunction.* calls into EF SQL expressions. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/WindowFunctionEvaluatableExpressionFilter.cs | Prevents client-evaluation of window-function marker methods. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/ExpressiveRelationalQuerySqlGeneratorFactory.cs | Attempts to provide a SQL generator that can render custom window-function SQL nodes. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/Infrastructure/Internal/ExpressiveRelationalQuerySqlGenerator.cs | Adds SQL rendering for WindowFunctionSqlExpression. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/ExpressiveSharp.EntityFrameworkCore.Relational.csproj | New relational package project with EF Core Relational references per TFM. |
| src/ExpressiveSharp.EntityFrameworkCore.Relational/ExpressiveOptionsBuilderExtensions.cs | Adds UseRelationalExtensions() plugin registration extension. |
| README.md | Documents relational package installation and window function usage. |
| ExpressiveSharp.slnx | Adds the new relational project and its test project to the solution. |
| Directory.Packages.props | Adds central version for Microsoft.EntityFrameworkCore.Relational. |
| CLAUDE.md | Updates repo/module map and test listing to include relational project/tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Prices: 10, 20, 20, 30, 50 (note duplicates for tie-testing) | ||
| ctx.Orders.AddRange( | ||
| new Order { Id = 1, Price = 50, Quantity = 1, CustomerId = 1 }, | ||
| new Order { Id = 2, Price = 20, Quantity = 2, CustomerId = 1 }, | ||
| new Order { Id = 3, Price = 10, Quantity = 3, CustomerId = 2 }, | ||
| new Order { Id = 4, Price = 30, Quantity = 1, CustomerId = 2 }, | ||
| new Order { Id = 5, Price = 20, Quantity = 5, CustomerId = 1 }, | ||
| new Order { Id = 6, Price = 40, Quantity = 2, CustomerId = 2 }, | ||
| new Order { Id = 7, Price = 15, Quantity = 1, CustomerId = 1 }, | ||
| new Order { Id = 8, Price = 25, Quantity = 3, CustomerId = 2 }, | ||
| new Order { Id = 9, Price = 35, Quantity = 2, CustomerId = 1 }, | ||
| new Order { Id = 10, Price = 45, Quantity = 1, CustomerId = 2 }); |
There was a problem hiding this comment.
The comment says Prices: 10, 20, 20, 30, 50, but the seeded data actually contains 10 orders with additional prices (15, 25, 35, 40, 45, etc.). Update the comment to match the seeded dataset to avoid misleading future readers.
| Assert.AreEqual(10, results.Count); | ||
| // NTILE(4) with 10 rows: buckets of 3,3,2,2 | ||
| Assert.IsTrue(results.All(r => r.Quartile >= 1 && r.Quartile <= 4), | ||
| "All quartile values should be between 1 and 4"); | ||
| // Count per bucket | ||
| var bucketCounts = results.GroupBy(r => r.Quartile).OrderBy(g => g.Key).Select(g => g.Count()).ToList(); | ||
| Assert.AreEqual(4, bucketCounts.Count, "Should have exactly 4 buckets"); | ||
| } |
There was a problem hiding this comment.
Ntile_DistributesIntoBuckets computes bucket counts but doesn't assert the expected distribution (the comment says 3,3,2,2 for 10 rows into 4 buckets). As written, the test would still pass even if NTILE were translated incorrectly as long as values stay within 1..4 and all buckets appear. Add assertions for the expected per-bucket counts (and/or expected ordering) to make this test effective.
| return method.Name switch | ||
| { | ||
| nameof(WindowFunction.RowNumber) when arguments.Count == 1 && arguments[0] is WindowSpecSqlExpression spec | ||
| => new RowNumberExpression(spec.Partitions, spec.Orderings, longTypeMapping), | ||
|
|
||
| nameof(WindowFunction.RowNumber) when arguments.Count == 0 | ||
| => new RowNumberExpression( | ||
| [], | ||
| [new OrderingExpression( | ||
| _sqlExpressionFactory.Fragment("(SELECT NULL)"), | ||
| ascending: true)], | ||
| longTypeMapping), | ||
|
|
||
| nameof(WindowFunction.Rank) when arguments[0] is WindowSpecSqlExpression spec | ||
| => new WindowFunctionSqlExpression("RANK", [], spec.Partitions, spec.Orderings, typeof(long), longTypeMapping), | ||
|
|
||
| nameof(WindowFunction.DenseRank) when arguments[0] is WindowSpecSqlExpression spec | ||
| => new WindowFunctionSqlExpression("DENSE_RANK", [], spec.Partitions, spec.Orderings, typeof(long), longTypeMapping), | ||
|
|
||
| nameof(WindowFunction.Ntile) when arguments[1] is WindowSpecSqlExpression spec | ||
| => new WindowFunctionSqlExpression("NTILE", | ||
| [_sqlExpressionFactory.ApplyDefaultTypeMapping(arguments[0])], | ||
| spec.Partitions, spec.Orderings, typeof(long), longTypeMapping), | ||
|
|
There was a problem hiding this comment.
WindowFunctionMethodCallTranslator indexes into arguments for Rank, DenseRank, and Ntile without checking arguments.Count first. If EF calls the translator with an unexpected argument shape (or if a user constructs an invalid expression tree), this will throw IndexOutOfRangeException during translation rather than returning null/a helpful error. Add arguments.Count guards to these switch arms before reading arguments[0]/arguments[1].
| public QuerySqlGenerator Create() => | ||
| new ExpressiveRelationalQuerySqlGenerator(_dependencies); |
There was a problem hiding this comment.
ExpressiveRelationalQuerySqlGeneratorFactory captures the provider's IQuerySqlGeneratorFactory in _inner but never uses it; Create() always returns new ExpressiveRelationalQuerySqlGenerator(_dependencies). This effectively replaces the provider-specific SQL generator (e.g., SqlServer/Sqlite) with the base QuerySqlGenerator, which can break provider-specific SQL rendering (TOP/LIMIT/OFFSET, provider-specific expression types, etc.). Update the factory to delegate to the inner generator and only extend it to handle WindowFunctionSqlExpression (or otherwise preserve provider-specific generator behavior).
| public QuerySqlGenerator Create() => | |
| new ExpressiveRelationalQuerySqlGenerator(_dependencies); | |
| public QuerySqlGenerator Create() | |
| { | |
| var innerGenerator = _inner.Create(); | |
| // If the inner factory already returns our custom generator, preserve it. | |
| if (innerGenerator is ExpressiveRelationalQuerySqlGenerator) | |
| { | |
| return innerGenerator; | |
| } | |
| // Otherwise, fall back to our expressive generator. | |
| return new ExpressiveRelationalQuerySqlGenerator(_dependencies); | |
| } |
| /// Plugin that registers window function services into the EF Core service provider. | ||
| /// Auto-discovered via <see cref="ExpressivePluginAttribute"/> when the assembly is loaded. | ||
| /// </summary> | ||
| /// <summary> | ||
| /// Plugin that registers window function services into the EF Core service provider. | ||
| /// Activated via <c>.UseExpressives(o => o.UseRelationalExtensions())</c>. | ||
| /// </summary> |
There was a problem hiding this comment.
The XML docs on RelationalExpressivePlugin have two consecutive <summary> blocks, and the first one mentions auto-discovery via ExpressivePluginAttribute even though no such attribute is present/used in this PR. This will generate malformed/unhelpful docs and is misleading for consumers. Collapse to a single <summary> and describe the actual activation mechanism (UseExpressives(o => o.UseRelationalExtensions())).
| /// Plugin that registers window function services into the EF Core service provider. | |
| /// Auto-discovered via <see cref="ExpressivePluginAttribute"/> when the assembly is loaded. | |
| /// </summary> | |
| /// <summary> | |
| /// Plugin that registers window function services into the EF Core service provider. | |
| /// Activated via <c>.UseExpressives(o => o.UseRelationalExtensions())</c>. | |
| /// </summary> | |
| /// Plugin that registers window function services into the EF Core service provider, | |
| /// activated via <c>.UseExpressives(o => o.UseRelationalExtensions())</c>. | |
| /// </summary> |
| foreach (var plugin in plugins) | ||
| hash.Add(plugin.GetType().FullName); |
There was a problem hiding this comment.
ExpressiveOptionsExtension computes the service-provider hash based only on plugin.GetType().FullName. If plugins are ever stateful (configuration stored on the instance) or order-dependent, EF Core may incorrectly reuse the same internal service provider for different plugin configurations. Consider either constraining plugins to be stateless/deduped, or include a plugin-provided hash (and/or plugin order) in _pluginHash and ShouldUseSameServiceProvider.
| foreach (var plugin in plugins) | |
| hash.Add(plugin.GetType().FullName); | |
| foreach (var plugin in plugins) | |
| { | |
| hash.Add(plugin.GetType().FullName); | |
| hash.Add(plugin.GetHashCode()); | |
| } |
| public DbSet<Order> Orders => Set<Order>(); | ||
| public ExpressiveDbSet<Order> ExpressiveOrders => this.ExpressiveSet<Order>(); | ||
| public DbSet<Customer> Customers => Set<Customer>(); |
There was a problem hiding this comment.
WindowTestDbContext references ExpressiveDbSet<Order> but the file doesn't import the ExpressiveSharp.EntityFrameworkCore namespace (and the type isn't in Microsoft.EntityFrameworkCore). This should not compile with the current usings; add the missing using or fully-qualify the type name.
| using ExpressiveSharp.EntityFrameworkCore.Relational.Tests.Models; | ||
| using ExpressiveSharp.EntityFrameworkCore.Relational.WindowFunctions; | ||
| using Microsoft.Data.Sqlite; | ||
| using Microsoft.EntityFrameworkCore; | ||
| using Microsoft.Extensions.DependencyInjection; |
There was a problem hiding this comment.
Microsoft.Extensions.DependencyInjection is imported but not used in this test file. With TreatWarningsAsErrors enabled, this unused using will fail the build; remove it or use it.
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'ExpressiveSharp Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 712f946 | Previous: ecfbc5e | Ratio |
|---|---|---|---|
ExpressiveSharp.Benchmarks.ExpressionReplacerBenchmarks.Replace_BlockBody |
6304.290918986003 ns (± 2891.9515004907116) |
3140.7159576416016 ns (± 64.07807556687932) |
2.01 |
ExpressiveSharp.Benchmarks.PolyfillGeneratorBenchmarks.RunGenerator(CallSiteCount: 100) |
124563286.83333333 ns (± 18630333.699155636) |
42829457.25 ns (± 223322.78085870645) |
2.91 |
ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_NoiseChange(ExpressiveCount: 100) |
143672424.55555555 ns (± 1949313.3288649952) |
114098720.1111111 ns (± 11565259.954403723) |
1.26 |
This comment was automatically generated by workflow using github-action-benchmark.
…nhance WindowFunctionSqlExpression documentation
- Add arguments.Count guards to Rank/DenseRank/Ntile translator arms - Fix stale comment about seeded test data prices - Strengthen Ntile test with per-bucket count assertions - Remove unused using directive Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Adds SQL window function support (ROW_NUMBER, RANK, DENSE_RANK, NTILE) and indexed
Selectto ExpressiveSharp via a newExpressiveSharp.EntityFrameworkCore.Relationalpackage.Setup
What this enables
Ranking rows within a result set
Ranking with ties
Dense ranking (no gaps after ties)
Distributing rows into buckets
Indexed Select (zero-based row index)
Combining with ExpressiveSharp features
Architecture
ExpressiveSharp.EntityFrameworkCore.Relational— depends onMicrosoft.EntityFrameworkCore.Relational, keeping the base EF Core package provider-agnosticExpressiveOptionsBuilderwithUseRelationalExtensions()— explicit opt-in, no fragile assembly scanningRowNumberExpression; RANK/DENSE_RANK/NTILE use a self-renderingWindowFunctionSqlExpressionthat works with any provider'sQuerySqlGenerator(no generator replacement)IExpressionTreeTransformerthat rewritesQueryable.Select((item, index) => ...)to injectROW_NUMBER() - 1Test plan
🤖 Generated with Claude Code