Skip to content

Commit

Permalink
Query: Update table references in pushdown after moving table referen…
Browse files Browse the repository at this point in the history
…ces to subquery (#26700)

Doing so causes all table references to maintain referential integrity again, which is required in processing projections in certain scenarios

Resolves #26587
  • Loading branch information
smitpatel authored Nov 15, 2021
1 parent e621b10 commit 1b1c77a
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 3 deletions.
21 changes: 18 additions & 3 deletions src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2817,6 +2817,18 @@ private SqlRemappingVisitor PushdownIntoSubqueryInternal()
_tables.Add(subquery);
_tableReferences.Add(subqueryTableReferenceExpression);

var useOldBehavior = AppContext.TryGetSwitch("Microsoft.EntityFrameworkCore.Issue26587", out var enabled)
&& enabled;

if (!useOldBehavior)
{
// Remap tableReferences in inner so that all components follow referential integrity.
foreach (var tableReference in subquery._tableReferences)
{
tableReference.UpdateTableReference(this, subquery);
}
}

var projectionMap = new Dictionary<SqlExpression, ColumnExpression>(ReferenceEqualityComparer.Instance);

if (_projection.Count > 0)
Expand Down Expand Up @@ -2953,10 +2965,13 @@ private SqlRemappingVisitor PushdownIntoSubqueryInternal()
subquery.ClearOrdering();
}

// Remap tableReferences in inner
foreach (var tableReference in subquery._tableReferences)
if (useOldBehavior)
{
tableReference.UpdateTableReference(this, subquery);
// Remap tableReferences in inner
foreach (var tableReference in subquery._tableReferences)
{
tableReference.UpdateTableReference(this, subquery);
}
}

var tableReferenceUpdatingExpressionVisitor = new TableReferenceUpdatingExpressionVisitor(this, subquery);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,20 @@ public virtual Task GroupBy_aggregate_Pushdown(bool async)
.Skip(4));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_aggregate_using_grouping_key_Pushdown(bool async)
{
return AssertQuery(
async,
ss => ss.Set<Order>().GroupBy(e => e.CustomerID)
.Where(g => g.Count() > 10)
.Select(g => new { g.Key, Max = g.Max(e => g.Key) })
.OrderBy(t => t.Key)
.Take(20)
.Skip(4));
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual Task GroupBy_aggregate_Pushdown_followed_by_projecting_Length(bool async)
Expand Down
47 changes: 47 additions & 0 deletions test/EFCore.Specification.Tests/Query/SimpleQueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -551,5 +551,52 @@ protected class Membership
public Group Group { get; set; }
public int GroupId { get; set; }
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task GroupBy_aggregate_on_right_side_of_join(bool async)
{
var contextFactory = await InitializeAsync<Context26587>();
using var context = contextFactory.CreateContext();

var orderId = 123456;

var orderItems = context.OrderItems.Where(o => o.OrderId == orderId);
var items = orderItems
.GroupBy(o => o.OrderId, (o, g) => new
{
Key = o,
IsPending = g.Max(y => y.ShippingDate == null && y.CancellationDate == null ? o : (o - 10000000))
})
.OrderBy(e => e.Key);


var query = orderItems
.Join(items, x => x.OrderId, x => x.Key, (x, y) => x)
.OrderBy(x => x.OrderId);


var users = async
? await query.ToListAsync()
: query.ToList();
}

protected class Context26587 : DbContext
{
public Context26587(DbContextOptions options)
: base(options)
{
}

public DbSet<OrderItem> OrderItems { get; set; }
}

protected class OrderItem
{
public int Id { get; set; }
public int OrderId { get; set; }
public DateTime? ShippingDate { get; set; }
public DateTime? CancellationDate { get; set; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,26 @@ ORDER BY [t].[CustomerID]
OFFSET @__p_1 ROWS");
}

public override async Task GroupBy_aggregate_using_grouping_key_Pushdown(bool async)
{
await base.GroupBy_aggregate_using_grouping_key_Pushdown(async);

AssertSql(
@"@__p_0='20'
@__p_1='4'
SELECT [t].[Key], [t].[Max]
FROM (
SELECT TOP(@__p_0) [o].[CustomerID] AS [Key], MAX([o].[CustomerID]) AS [Max]
FROM [Orders] AS [o]
GROUP BY [o].[CustomerID]
HAVING COUNT(*) > 10
ORDER BY [o].[CustomerID]
) AS [t]
ORDER BY [t].[Key]
OFFSET @__p_1 ROWS");
}

public override async Task GroupBy_aggregate_Pushdown_followed_by_projecting_Length(bool async)
{
await base.GroupBy_aggregate_Pushdown_followed_by_projecting_Length(async);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,24 @@ ELSE CAST(0 AS bit)
END AS [HasAccess]
FROM [Users] AS [u]");
}

public override async Task GroupBy_aggregate_on_right_side_of_join(bool async)
{
await base.GroupBy_aggregate_on_right_side_of_join(async);

AssertSql(
@"@__orderId_0='123456'
SELECT [o].[Id], [o].[CancellationDate], [o].[OrderId], [o].[ShippingDate]
FROM [OrderItems] AS [o]
INNER JOIN (
SELECT [o0].[OrderId] AS [Key]
FROM [OrderItems] AS [o0]
WHERE [o0].[OrderId] = @__orderId_0
GROUP BY [o0].[OrderId]
) AS [t] ON [o].[OrderId] = [t].[Key]
WHERE [o].[OrderId] = @__orderId_0
ORDER BY [o].[OrderId]");
}
}
}

0 comments on commit 1b1c77a

Please sign in to comment.