Skip to content

Commit

Permalink
Cosmos: error on operator ordering that requires pushdown
Browse files Browse the repository at this point in the history 
Fixes dotnet#34123
  • Loading branch information
@roji
roji committed Jul 19, 2024
1 parent 594688b commit 0b59eda
Show file tree
Hide file tree
Showing 7 changed files with 172 additions and 277 deletions.
31 changes: 15 additions & 16 deletions src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -550,12 +550,11 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou
{
var select = (SelectExpression)source.QueryExpression;

// TODO: #34123
// if ((select.IsDistinct || select.Limit is not null || select.Offset is not null)
// && !TryPushdownIntoSubquery(select))
// {
// return null;
// }
if ((select.Limit is not null || select.Offset is not null)
&& !TryPushdownIntoSubquery(select))
{
return null;
}

select.ApplyDistinct();

Expand Down Expand Up @@ -1522,11 +1521,11 @@ private bool TryApplyPredicate(ShapedQueryExpression source, LambdaExpression pr
{
var select = (SelectExpression)source.QueryExpression;

// TODO: #34123
// if ((select.Limit is not null || select.Offset is not null) && !TryPushdownIntoSubquery(select))
// {
// select.PushdownIntoSubquery();
// }
if ((select.Limit is not null || select.Offset is not null)
&& !TryPushdownIntoSubquery(select))
{
return false;
}

if (TranslateLambdaExpression(source, predicate) is SqlExpression translation)
{
Expand All @@ -1543,11 +1542,11 @@ private bool TryApplyPredicate(ShapedQueryExpression source, LambdaExpression pr

private bool TryApplyPredicate(SelectExpression select, SqlExpression predicate)
{
// TODO: #34123
// if ((select.Limit is not null || select.Offset is not null) && !TryPushdownIntoSubquery(select, predicate))
// {
// return false;
// }
if ((select.Limit is not null || select.Offset is not null)
&& !TryPushdownIntoSubquery(select))
{
return false;
}

select.ApplyPredicate(predicate);
return true;
Expand Down
21 changes: 5 additions & 16 deletions test/EFCore.Cosmos.FunctionalTests/Query/InheritanceQueryCosmosTest.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.EntityFrameworkCore.Cosmos.Internal;

namespace Microsoft.EntityFrameworkCore.Query;

#nullable disable
Expand Down Expand Up @@ -475,22 +477,9 @@ public override async Task OfType_Union_OfType(bool async)
}

public override Task Subquery_OfType(bool async)
=> Fixture.NoSyncTest(
async, async a =>
{
await base.Subquery_OfType(a);

AssertSql(
"""
@__p_0='5'
SELECT DISTINCT c
FROM root c
WHERE (c["Discriminator"] IN ("Eagle", "Kiwi") AND (c["Discriminator"] = "Kiwi"))
ORDER BY c["Species"]
OFFSET 0 LIMIT @__p_0
""");
});
=> AssertTranslationFailedWithDetails(
() => base.Subquery_OfType(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

public override async Task Union_entity_equality(bool async)
{
Expand Down
193 changes: 32 additions & 161 deletions test/EFCore.Cosmos.FunctionalTests/Query/NorthwindMiscellaneousQueryCosmosTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1269,34 +1269,15 @@ await AssertTranslationFailedWithDetails(
AssertSql();
}

public override async Task Skip_Distinct(bool async)
{
Assert.Equal(
CosmosStrings.OffsetRequiresLimit,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => base.Skip_Distinct(async))).Message);

AssertSql();
}
public override Task Skip_Distinct(bool async)
=> AssertTranslationFailedWithDetails(
() => base.Skip_Distinct(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

public override Task Skip_Take_Distinct(bool async)
=> Fixture.NoSyncTest(
async, async a =>
{
await base.Skip_Take_Distinct(a);

AssertSql(
"""
@__p_0='5'
@__p_1='10'
SELECT DISTINCT c
FROM root c
WHERE (c["Discriminator"] = "Customer")
ORDER BY c["ContactName"]
OFFSET @__p_0 LIMIT @__p_1
""");
});
=> AssertTranslationFailedWithDetails(
() => base.Skip_Take_Distinct(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

public override async Task Skip_Take_Any(bool async)
{
Expand Down Expand Up @@ -1339,54 +1320,15 @@ public override async Task Take_All(bool async)
AssertSql();
}

public override async Task Skip_Take_Any_with_predicate(bool async)
{
// Always throws for sync.
if (async)
{
// Top-level parameterless Any(), see #33854.
var exception = await Assert.ThrowsAsync<CosmosException>(() => base.Skip_Take_Any_with_predicate(async));

Assert.Equal(HttpStatusCode.BadRequest, exception.StatusCode);

AssertSql(
"""
@__p_0='5'
@__p_1='7'
SELECT EXISTS (
SELECT 1
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND STARTSWITH(c["CustomerID"], "C"))
ORDER BY c["CustomerID"]
OFFSET @__p_0 LIMIT @__p_1) AS c
""");
}
}

public override async Task Take_Any_with_predicate(bool async)
{
// Always throws for sync.
if (async)
{
// Top-level parameterless Any(), see #33854.
var exception = await Assert.ThrowsAsync<CosmosException>(() => base.Take_Any_with_predicate(async));

Assert.Equal(HttpStatusCode.BadRequest, exception.StatusCode);

AssertSql(
"""
@__p_0='5'
public override Task Skip_Take_Any_with_predicate(bool async)
=> AssertTranslationFailedWithDetails(
() => base.Skip_Take_Any_with_predicate(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

SELECT EXISTS (
SELECT 1
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND STARTSWITH(c["CustomerID"], "B"))
ORDER BY c["CustomerID"]
OFFSET 0 LIMIT @__p_0) AS c
""");
}
}
public override Task Take_Any_with_predicate(bool async)
=> AssertTranslationFailedWithDetails(
() => base.Take_Any_with_predicate(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

public override Task OrderBy(bool async)
=> Fixture.NoSyncTest(
Expand Down Expand Up @@ -1507,33 +1449,15 @@ public override async Task OrderBy_multiple_queries(bool async)
}

public override Task Take_Distinct(bool async)
=> Fixture.NoSyncTest(
async, async a =>
{
await base.Take_Distinct(a);

AssertSql(
"""
@__p_0='5'
SELECT DISTINCT c
FROM root c
WHERE (c["Discriminator"] = "Order")
ORDER BY c["OrderID"]
OFFSET 0 LIMIT @__p_0
""");
});
=> AssertTranslationFailedWithDetails(
() => base.Take_Distinct(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

public override async Task Distinct_Take(bool async)
{
// Subquery pushdown. Issue #16156.
await AssertTranslationFailedWithDetails(
public override Task Distinct_Take(bool async)
=> AssertTranslationFailedWithDetails(
() => base.Distinct_Take(async),
CosmosStrings.NoSubqueryPushdown);

AssertSql();
}

public override async Task Distinct_Take_Count(bool async)
{
// Cosmos client evaluation. Issue #17246.
Expand Down Expand Up @@ -2813,73 +2737,20 @@ await AssertTranslationFailedWithDetails(
AssertSql();
}

public override async Task OrderBy_skip_take_distinct(bool async)
{
// Always throws for sync.
if (async)
{
// Cosmos client evaluation. Issue #17246.
await Assert.ThrowsAsync<CosmosException>(
async () => await base.OrderBy_skip_take_distinct(async));

AssertSql(
"""
@__p_0='5'
@__p_1='15'
SELECT DISTINCT c
FROM root c
WHERE (c["Discriminator"] = "Customer")
ORDER BY c["ContactTitle"], c["ContactName"]
OFFSET @__p_0 LIMIT @__p_1
""");
}
}

public override async Task OrderBy_coalesce_take_distinct(bool async)
{
// Always throws for sync.
if (async)
{
// Cosmos client evaluation. Issue #17246.
await Assert.ThrowsAsync<CosmosException>(
async () => await base.OrderBy_coalesce_take_distinct(async));

AssertSql(
"""
@__p_0='15'
SELECT DISTINCT c
FROM root c
WHERE (c["Discriminator"] = "Product")
ORDER BY ((c["UnitPrice"] != null) ? c["UnitPrice"] : 0.0)
OFFSET 0 LIMIT @__p_0
""");
}
}

public override async Task OrderBy_coalesce_skip_take_distinct(bool async)
{
// Always throws for sync.
if (async)
{
// Cosmos client evaluation. Issue #17246.
await Assert.ThrowsAsync<CosmosException>(
async () => await base.OrderBy_coalesce_skip_take_distinct(async));
public override Task OrderBy_skip_take_distinct(bool async)
=> AssertTranslationFailedWithDetails(
() => base.OrderBy_skip_take_distinct(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

AssertSql(
"""
@__p_0='5'
@__p_1='15'
public override Task OrderBy_coalesce_take_distinct(bool async)
=> AssertTranslationFailedWithDetails(
() => base.OrderBy_coalesce_take_distinct(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

SELECT DISTINCT c
FROM root c
WHERE (c["Discriminator"] = "Product")
ORDER BY ((c["UnitPrice"] != null) ? c["UnitPrice"] : 0.0)
OFFSET @__p_0 LIMIT @__p_1
""");
}
}
public override Task OrderBy_coalesce_skip_take_distinct(bool async)
=> AssertTranslationFailedWithDetails(
() => base.OrderBy_coalesce_skip_take_distinct(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

public override async Task OrderBy_coalesce_skip_take_distinct_take(bool async)
{
Expand Down
19 changes: 3 additions & 16 deletions test/EFCore.Cosmos.FunctionalTests/Query/NorthwindSelectQueryCosmosTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1425,22 +1425,9 @@ await AssertTranslationFailed(
}

public override Task Projection_take_predicate_projection(bool async)
=> Fixture.NoSyncTest(
async, async a =>
{
await base.Projection_take_predicate_projection(a);

AssertSql(
"""
@__p_0='10'
SELECT ((c["CustomerID"] || " ") || c["City"]) AS Aggregate
FROM root c
WHERE ((c["Discriminator"] = "Customer") AND STARTSWITH(c["CustomerID"], "A"))
ORDER BY c["CustomerID"]
OFFSET 0 LIMIT @__p_0
""");
});
=> AssertTranslationFailedWithDetails(
() => base.Projection_take_predicate_projection(async),
CosmosStrings.LimitOffsetNotSupportedInSubqueries);

public override Task Projection_take_projection_doesnt_project_intermittent_column(bool async)
=> Fixture.NoSyncTest(
Expand Down
Loading

0 comments on commit 0b59eda

Please # to comment.