Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 31 additions & 15 deletions src/EFCore.Relational/Query/SqlTreePruner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,17 @@ protected virtual SelectExpression PruneSelect(SelectExpression select, bool pre
/// that ordering isn't actually necessary.
/// </summary>
/// <remarks>
/// 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.
/// <para>
/// This also visits the row/parameter contents of the <see cref="ValuesExpression" />, so that column
/// references embedded within it (e.g. a navigation column inlined into a row) get registered in
/// <see cref="ReferencedColumnMap" /> before the tables providing them are considered for pruning.
/// </para>
/// <para>
/// 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.
/// </para>
/// </remarks>
[EntityFrameworkInternal]
protected virtual ValuesExpression PruneValues(ValuesExpression values)
Expand All @@ -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]);
Comment thread
AndriySvyryd marked this conversation as resolved.
}
}

Expand Down Expand Up @@ -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++)
Expand All @@ -366,14 +381,15 @@ protected virtual ValuesExpression PruneValues(ValuesExpression values)
{
if (referencedColumns[j])
{
newValues.Add(oldValues[j]);
newValues.Add((SqlExpression)Visit(oldValues[j]));
}
}

newRowValues[i] = new RowValueExpression(newValues);
}

return new ValuesExpression(values.Alias, newRowValues, valuesParameter: null, newColumnNames);
}

default:
throw new UnreachableException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Context38700>(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<int> AuthorizedServiceIds { get; set; } = [];

public DbSet<Parent38700> Parents
=> Set<Parent38700>();

public DbSet<Child38700> Children
=> Set<Child38700>();

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
modelBuilder.Entity<Parent38700>().HasQueryFilter(
p => AuthorizedServiceIds.Contains(p.ServiceId));

// Inline array of a navigation column — triggers VALUES pruning of outer join columns (#38700).
modelBuilder.Entity<Child38700>().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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<Context38700>(
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<Parent38700> Parents
=> Set<Parent38700>();

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<Child38700> 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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading