diff --git a/src/EFCore.Relational/Query/SqlTreePruner.cs b/src/EFCore.Relational/Query/SqlTreePruner.cs index 7d99afde982..2e01bc03caf 100644 --- a/src/EFCore.Relational/Query/SqlTreePruner.cs +++ b/src/EFCore.Relational/Query/SqlTreePruner.cs @@ -273,10 +273,17 @@ protected virtual SelectExpression PruneSelect(SelectExpression select, bool pre /// that ordering isn't actually necessary. /// /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + /// This also visits the row/parameter contents of the , so that column + /// references embedded within it (e.g. a navigation column inlined into a row) get registered in + /// before the tables providing them are considered for pruning. + /// + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// /// [EntityFrameworkInternal] protected virtual ValuesExpression PruneValues(ValuesExpression values) @@ -301,7 +308,7 @@ protected virtual ValuesExpression PruneValues(ValuesExpression values) for (var j = 0; j < i; j++) { referencedColumns[j] = true; - newColumnNames.Add(columnName); + newColumnNames.Add(values.ColumnNames[j]); } } @@ -337,24 +344,32 @@ protected virtual ValuesExpression PruneValues(ValuesExpression values) } } - if (referencedColumns is null) - { - return values; - } - - // We know at least some columns are getting pruned. - Debug.Assert(newColumnNames is not null); - + // Always visit nested expressions so that column references inside VALUES cells (e.g. navigation + // columns embedded in an inline collection) are registered before outer tables are pruned (#38700). switch (values) { // If we have a value parameter (row values aren't specific in line), we still prune the column names. // Later in SqlNullabilityProcessor, when the parameterized collection is inline to constants, we'll take // the column names into account. case { ValuesParameter: not null }: - return new ValuesExpression(values.Alias, rowValues: null, values.ValuesParameter, newColumnNames); + { + var visitedParameter = (SqlParameterExpression)Visit(values.ValuesParameter); + + return referencedColumns is null + ? values.Update(visitedParameter) + : new ValuesExpression(values.Alias, rowValues: null, visitedParameter, newColumnNames!); + } // Go over the rows and create new ones without the pruned columns. case { RowValues: { } rowValues }: + { + if (referencedColumns is null) + { + return values.Update(this.VisitAndConvert(rowValues)); + } + + Debug.Assert(newColumnNames is not null); + var newRowValues = new RowValueExpression[rowValues.Count]; for (var i = 0; i < rowValues.Count; i++) @@ -366,7 +381,7 @@ protected virtual ValuesExpression PruneValues(ValuesExpression values) { if (referencedColumns[j]) { - newValues.Add(oldValues[j]); + newValues.Add((SqlExpression)Visit(oldValues[j])); } } @@ -374,6 +389,7 @@ protected virtual ValuesExpression PruneValues(ValuesExpression values) } return new ValuesExpression(values.Alias, newRowValues, valuesParameter: null, newColumnNames); + } default: throw new UnreachableException(); diff --git a/test/EFCore.Relational.Specification.Tests/Query/AdHocQueryFiltersQueryRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/AdHocQueryFiltersQueryRelationalTestBase.cs index 68072cb7bc2..b9fd81d9ca9 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/AdHocQueryFiltersQueryRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/AdHocQueryFiltersQueryRelationalTestBase.cs @@ -13,4 +13,68 @@ protected void ClearLog() protected void AssertSql(params string[] expected) => TestSqlLoggerFactory.AssertBaseline(expected); + + #region 38700 + + [Theory, MemberData(nameof(IsAsyncData))] + public virtual async Task Query_filter_with_inline_collection_of_navigation_column(bool async) + { + var contextFactory = await InitializeNonSharedTest(seed: c => c.SeedAsync()); + using var context = contextFactory.CreateDbContext(); + + Context38700.AuthorizedServiceIds = [10]; + + var query = context.Children.AsNoTracking().Select(c => c.Label); + + var results = async + ? await query.ToListAsync() + : query.ToList(); + + Assert.Equal(["ok"], results); + } + + protected class Context38700(DbContextOptions options) : DbContext(options) + { + public static List AuthorizedServiceIds { get; set; } = []; + + public DbSet Parents + => Set(); + + public DbSet Children + => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity().HasQueryFilter( + p => AuthorizedServiceIds.Contains(p.ServiceId)); + + // Inline array of a navigation column — triggers VALUES pruning of outer join columns (#38700). + modelBuilder.Entity().HasQueryFilter(c => + new int?[] { c.Parent.ServiceId } + .Any(id => id.HasValue && AuthorizedServiceIds.Contains(id.Value))); + } + + public Task SeedAsync() + { + var parent = new Parent38700 { ServiceId = 10 }; + Children.Add(new Child38700 { Parent = parent, Label = "ok" }); + return SaveChangesAsync(); + } + } + + protected class Parent38700 + { + public int Id { get; set; } + public int ServiceId { get; set; } + } + + protected class Child38700 + { + public int Id { get; set; } + public int ParentId { get; set; } + public Parent38700 Parent { get; set; } = null!; + public string Label { get; set; } = null!; + } + + #endregion } diff --git a/test/EFCore.Relational.Specification.Tests/Query/AdHocQuerySplittingQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/AdHocQuerySplittingQueryTestBase.cs index 228dfae6c3a..ec05322c80c 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/AdHocQuerySplittingQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/AdHocQuerySplittingQueryTestBase.cs @@ -620,4 +620,90 @@ public sealed class Tag } #endregion + + #region 38700 + + [Theory, MemberData(nameof(IsAsyncData))] + public virtual async Task Split_query_with_inline_collection_Max_over_related_columns(bool async) + { + var contextFactory = await InitializeNonSharedTest( + seed: c => c.SeedAsync(), + onConfiguring: Configure38700); + + using var context = contextFactory.CreateDbContext(); + + // Same shape as #38700 comment (jk-aau): Max over an inline array of related DateTimes, + // combined with AsSplitQuery + Skip/Take. On providers without GREATEST (e.g. SQL Server + // compat < 160), this becomes VALUES + MAX and hits the PruneValues column-registration bug. + var query = context.Parents + .OrderBy(p => p.Id) + .Select(p => new + { + p.Id, + Children = p.Children.Select(c => c.Id).ToList(), + LatestModified = new[] + { + p.ModifiedAt, + p.Address!.ModifiedAt, + p.Children.Max(c => c.ModifiedAt) + }.Max() + }) + .AsSplitQuery() + .Skip(0) + .Take(50); + + var results = async + ? await query.ToListAsync() + : query.ToList(); + + Assert.Single(results); + Assert.Single(results[0].Children); + Assert.Equal(new DateTime(2024, 3, 1), results[0].LatestModified); + } + + protected virtual void Configure38700(DbContextOptionsBuilder optionsBuilder) + => SetQuerySplittingBehavior(optionsBuilder, QuerySplittingBehavior.SplitQuery); + + protected class Context38700(DbContextOptions options) : DbContext(options) + { + public DbSet Parents + => Set(); + + public Task SeedAsync() + { + Parents.Add( + new Parent38700 + { + ModifiedAt = new DateTime(2024, 1, 1), + Address = new Address38700 { ModifiedAt = new DateTime(2024, 2, 1) }, + Children = [new Child38700 { ModifiedAt = new DateTime(2024, 3, 1) }] + }); + + return SaveChangesAsync(); + } + + public class Parent38700 + { + public int Id { get; set; } + public DateTime ModifiedAt { get; set; } + public Address38700? Address { get; set; } + public List Children { get; set; } = []; + } + + public class Address38700 + { + public int Id { get; set; } + public int ParentId { get; set; } + public DateTime ModifiedAt { get; set; } + } + + public class Child38700 + { + public int Id { get; set; } + public int ParentId { get; set; } + public DateTime ModifiedAt { get; set; } + } + } + + #endregion } diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs index 6e7aab96e46..dfe44089f37 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQueryFiltersQuerySqlServerTest.cs @@ -520,6 +520,26 @@ public override async Task Query_filter_with_EF_Parameter_throws() AssertSql(); } + public override async Task Query_filter_with_inline_collection_of_navigation_column(bool async) + { + await base.Query_filter_with_inline_collection_of_navigation_column(async); + + AssertSql( + """ +SELECT [c].[Label] +FROM [Children] AS [c] +INNER JOIN ( + SELECT [p].[Id], [p].[ServiceId] + FROM [Parents] AS [p] + WHERE [p].[ServiceId] = 10 +) AS [p0] ON [c].[ParentId] = [p0].[Id] +WHERE EXISTS ( + SELECT 1 + FROM (VALUES ([p0].[ServiceId])) AS [v]([Value]) + WHERE [v].[Value] = 10) +"""); + } + [Fact] public virtual void Check_all_tests_overridden() => TestHelpers.AssertAllMethodsOverridden(GetType()); diff --git a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQuerySplittingQuerySqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQuerySplittingQuerySqlServerTest.cs index fdb6da6d4a1..5631bb1e5f7 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQuerySplittingQuerySqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Query/AdHocQuerySplittingQuerySqlServerTest.cs @@ -456,6 +456,55 @@ public override Task Split_include_collection_throws_for_orphan_child_rows_after public override Task Split_include_collection_not_dropped_when_other_parent_made_childless_concurrently(bool async) => base.Split_include_collection_not_dropped_when_other_parent_made_childless_concurrently(async); + public override async Task Split_query_with_inline_collection_Max_over_related_columns(bool async) + { + await base.Split_query_with_inline_collection_Max_over_related_columns(async); + + AssertSql( + """ +@p='0' +@p1='50' + +SELECT [p0].[Id], ( + SELECT MAX([v].[Value]) + FROM (VALUES ([p0].[ModifiedAt]), ([a].[ModifiedAt]), (( + SELECT MAX([c].[ModifiedAt]) + FROM [Child38700] AS [c] + WHERE [p0].[Id] = [c].[Parent38700Id]))) AS [v]([Value])) +FROM ( + SELECT [p].[Id], [p].[AddressId], [p].[ModifiedAt] + FROM [Parents] AS [p] + ORDER BY [p].[Id] + OFFSET @p ROWS FETCH NEXT @p1 ROWS ONLY +) AS [p0] +LEFT JOIN [Address38700] AS [a] ON [p0].[AddressId] = [a].[Id] +ORDER BY [p0].[Id] +""", + // + """ +@p='0' +@p1='50' + +SELECT [c1].[Id], [p0].[Id] +FROM ( + SELECT [p].[Id] + FROM [Parents] AS [p] + ORDER BY [p].[Id] + OFFSET @p ROWS FETCH NEXT @p1 ROWS ONLY +) AS [p0] +INNER JOIN [Child38700] AS [c1] ON [p0].[Id] = [c1].[Parent38700Id] +ORDER BY [p0].[Id] +"""); + } + + // Force the VALUES + MAX translation (no GREATEST) used when SQL Server compat is below 160 — + // this is the path that manifested #38700 with split-query pagination. + protected override void Configure38700(DbContextOptionsBuilder optionsBuilder) + { + base.Configure38700(optionsBuilder); + optionsBuilder.UseSqlServerCompatibilityLevel(150); + } + [Fact] public virtual void Check_all_tests_overridden() => TestHelpers.AssertAllMethodsOverridden(GetType());