Skip to content

Commit

Permalink
Pack of minor Cosmos changes (#34143)
Browse files Browse the repository at this point in the history 
* Refactor common point for implementation subquery pushdown,
  and improve error reporting when that happens.
* Move pushdown detection logic out of SelectExpression
* Detect ToPageAsync in subqueries and throw detailed message
* Make SelectExpression sealed
* Some relational syntax cleanup
  • Loading branch information
@roji
roji authored Jul 4, 2024
1 parent f9878b9 commit ac7380e
Show file tree
Hide file tree
Showing 10 changed files with 280 additions and 217 deletions.
8 changes: 7 additions & 1 deletion src/EFCore.Cosmos/Properties/CosmosStrings.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/EFCore.Cosmos/Properties/CosmosStrings.resx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@
<value>Both properties '{property1}' and '{property2}' on entity type '{entityType}' are mapped to '{storeName}'. Map one of the properties to a different JSON property.</value>
</data>
<data name="LimitOffsetNotSupportedInSubqueries" xml:space="preserve">
<value>Skip, Take, First/FirstOrDefault and Single/SingleOrDefault aren't supported in subqueries since Cosmos doesn't support LIMIT/OFFSET in subqueries.</value>
<value>The query requires use of LIMIT and OFFSET in a subquery, which is unsupported by Cosmos.</value>
</data>
<data name="LogExecutedCreateItem" xml:space="preserve">
<value>Executed CreateItem ({elapsed} ms, {charge} RU) ActivityId='{activityId}', Container='{container}', Id='{id}', Partition='{partitionKey}'</value>
Expand Down Expand Up @@ -303,6 +303,9 @@
<data name="ThroughputTypeMismatch" xml:space="preserve">
<value>The provisioned throughput was configured as manual on '{manualEntityType}', but on '{autoscaleEntityType}' it was configured as autoscale. All entity types mapped to the same container '{container}' must be configured with the same provisioned throughput type.</value>
</data>
<data name="ToPageAsyncAtTopLevelOnly" xml:space="preserve">
<value>'ToPageAsync' can only be used as the terminating operator of the top-level query.</value>
</data>
<data name="TransactionsNotSupported" xml:space="preserve">
<value>The Cosmos database provider does not support transactions.</value>
</data>
Expand Down
186 changes: 148 additions & 38 deletions src/EFCore.Cosmos/Query/Internal/CosmosQueryableMethodTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ public override Expression Translate(Expression expression)
&& method.DeclaringType == typeof(CosmosQueryableExtensions)
&& method.Name is nameof(CosmosQueryableExtensions.ToPageAsync))
{
if (_subquery)
{
AddTranslationErrorDetails(CosmosStrings.ToPageAsyncAtTopLevelOnly);
return QueryCompilationContext.NotTranslatedExpression;
}

var source = base.Translate(arguments[0]);

if (source == QueryCompilationContext.NotTranslatedExpression)
Expand Down Expand Up @@ -430,10 +436,12 @@ protected override ShapedQueryExpression CreateShapedQueryExpression(IEntityType
var discriminatorColumn = ((EntityProjectionExpression)selectExpression.GetMappedProjection(new ProjectionMember()))
.BindProperty(discriminatorProperty, clientEval: false);

selectExpression.ApplyPredicate(
var success = TryApplyPredicate(
selectExpression,
_sqlExpressionFactory.Equal(
(SqlExpression)discriminatorColumn,
_sqlExpressionFactory.Constant(concreteEntityType.GetDiscriminatorValue())));
Check.DebugAssert(success, "Couldn't apply predicate when creating a new ShapedQueryExpression");
}
}
else
Expand All @@ -443,10 +451,12 @@ protected override ShapedQueryExpression CreateShapedQueryExpression(IEntityType
var discriminatorColumn = ((EntityProjectionExpression)selectExpression.GetMappedProjection(new ProjectionMember()))
.BindProperty(discriminatorProperty, clientEval: false);

selectExpression.ApplyPredicate(
var success = TryApplyPredicate(
selectExpression,
_sqlExpressionFactory.In(
(SqlExpression)discriminatorColumn,
concreteEntityTypes.Select(et => _sqlExpressionFactory.Constant(et.GetDiscriminatorValue())).ToArray()));
Check.DebugAssert(success, "Couldn't apply predicate when creating a new ShapedQueryExpression");
}

return CreateShapedQueryExpression(entityType, selectExpression);
Expand Down Expand Up @@ -653,9 +663,18 @@ protected override ShapedQueryExpression TranslateCast(ShapedQueryExpression sou
/// 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.
/// </summary>
protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression source)
protected override ShapedQueryExpression? TranslateDistinct(ShapedQueryExpression source)
{
((SelectExpression)source.QueryExpression).ApplyDistinct();
var select = (SelectExpression)source.QueryExpression;

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

select.ApplyDistinct();

return source;
}
Expand Down Expand Up @@ -755,9 +774,13 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression
_queryCompilationContext.Logger.RowLimitingOperationWithoutOrderByWarning();
}

select.ApplyOffset(translatedIndex);
select.ApplyLimit(TranslateExpression(Expression.Constant(1))!);
if (!TryApplyOffset(select, translatedIndex)
|| !TryApplyLimit(select, TranslateExpression(Expression.Constant(1))!))
{
return null;
}

// TODO: ElementAt on top level
return null;
}

Expand Down Expand Up @@ -803,13 +826,17 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression
return TranslateElementAtOrDefault(source, Expression.Constant(0), returnDefault);
}

var selectExpression = (SelectExpression)source.QueryExpression;
if (selectExpression is { Predicate: null, Orderings: [] })
var select = (SelectExpression)source.QueryExpression;

if (!TryApplyLimit(select, TranslateExpression(Expression.Constant(1))!))
{
_queryCompilationContext.Logger.FirstWithoutOrderByAndFilterWarning();
return null;
}

selectExpression.ApplyLimit(TranslateExpression(Expression.Constant(1))!);
if (select is { Predicate: null, Orderings: [] })
{
_queryCompilationContext.Logger.FirstWithoutOrderByAndFilterWarning();
}

return source.ShaperExpression.Type != returnType
? source.UpdateShaperExpression(Expression.Convert(source.ShaperExpression, returnType))
Expand Down Expand Up @@ -891,9 +918,13 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression
source = translatedSource;
}

var selectExpression = (SelectExpression)source.QueryExpression;
selectExpression.ReverseOrderings();
selectExpression.ApplyLimit(TranslateExpression(Expression.Constant(1))!);
var select = (SelectExpression)source.QueryExpression;
select.ReverseOrderings();

if (!TryApplyLimit(select, TranslateExpression(Expression.Constant(1))!))
{
return null;
}

return source.ShaperExpression.Type != returnType
? source.UpdateShaperExpression(Expression.Convert(source.ShaperExpression, returnType))
Expand Down Expand Up @@ -1005,18 +1036,14 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression
return source;
}

var select = (SelectExpression)source.QueryExpression;

var parameterExpression = Expression.Parameter(entityShaperExpression.Type);
var predicate = Expression.Lambda(Expression.TypeIs(parameterExpression, resultType), parameterExpression);
if (TranslateLambdaExpression(source, predicate) is not SqlExpression translation)
{
// EntityType is not part of hierarchy
return null;
}

var selectExpression = (SelectExpression)source.QueryExpression;
if (translation is not SqlConstantExpression { Value: true })
if (!TryApplyPredicate(source, predicate))
{
selectExpression.ApplyPredicate(translation);
return null;
}

var baseType = entityType.GetAllBaseTypes().SingleOrDefault(et => et.ClrType == resultType);
Expand All @@ -1031,8 +1058,8 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression
var projectionMember = projectionBindingExpression.ProjectionMember;
Check.DebugAssert(new ProjectionMember().Equals(projectionMember), "Invalid ProjectionMember when processing OfType");

var entityProjectionExpression = (EntityProjectionExpression)selectExpression.GetMappedProjection(projectionMember);
selectExpression.ReplaceProjectionMapping(
var entityProjectionExpression = (EntityProjectionExpression)select.GetMappedProjection(projectionMember);
select.ReplaceProjectionMapping(
new Dictionary<ProjectionMember, Expression>
{
{ projectionMember, entityProjectionExpression.UpdateEntityType(derivedType) }
Expand All @@ -1052,6 +1079,14 @@ protected override ShapedQueryExpression TranslateDistinct(ShapedQueryExpression
LambdaExpression keySelector,
bool ascending)
{
var select = (SelectExpression)source.QueryExpression;

if ((select.IsDistinct || select.Limit is not null || select.Offset is not null)
&& !TryPushdownIntoSubquery(select))
{
return null;
}

if (TranslateLambdaExpression(source, keySelector) is SqlExpression translation)
{
((SelectExpression)source.QueryExpression).ApplyOrdering(new OrderingExpression(translation, ascending));
Expand Down Expand Up @@ -1195,8 +1230,11 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
return TranslateElementAtOrDefault(source, Expression.Constant(0), returnDefault);
}

var selectExpression = (SelectExpression)source.QueryExpression;
selectExpression.ApplyLimit(TranslateExpression(Expression.Constant(2))!);
var select = (SelectExpression)source.QueryExpression;
if (!TryApplyLimit(select, TranslateExpression(Expression.Constant(2))!))
{
return null;
}

return source.ShaperExpression.Type == returnType
? source
Expand Down Expand Up @@ -1274,6 +1312,11 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
return null;
}

if (!TryApplyOffset(select, translatedCount))
{
return null;
}

// Ordering of documents is not guaranteed in Cosmos, so we warn for Skip without OrderBy.
// However, when querying on JSON arrays within documents, the order of elements is guaranteed, and Skip without OrderBy is
// fine. Since subqueries must be correlated (i.e. reference an array in the outer query), we use that to decide whether to
Expand All @@ -1283,8 +1326,6 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
_queryCompilationContext.Logger.RowLimitingOperationWithoutOrderByWarning();
}

select.ApplyOffset(translatedCount);

return source;
}

Expand Down Expand Up @@ -1403,6 +1444,11 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
return null;
}

if (!TryApplyLimit(select, translatedCount))
{
return null;
}

// Ordering of documents is not guaranteed in Cosmos, so we warn for Take without OrderBy.
// However, when querying on JSON arrays within documents, the order of elements is guaranteed, and Take without OrderBy is
// fine. Since subqueries must be correlated (i.e. reference an array in the outer query), we use that to decide whether to
Expand All @@ -1412,8 +1458,6 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
_queryCompilationContext.Logger.RowLimitingOperationWithoutOrderByWarning();
}

select.ApplyLimit(translatedCount);

return source;
}

Expand Down Expand Up @@ -1461,7 +1505,9 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
/// </summary>
protected override ShapedQueryExpression? TranslateWhere(ShapedQueryExpression source, LambdaExpression predicate)
{
if (source.ShaperExpression is StructuralTypeShaperExpression { StructuralType: IEntityType entityType } entityShaperExpression
var select = (SelectExpression)source.QueryExpression;

if (source.ShaperExpression is StructuralTypeShaperExpression { StructuralType: IEntityType entityType }
&& entityType.GetPartitionKeyPropertyNames().FirstOrDefault() != null)
{
List<(Expression Expression, IProperty Property)?> partitionKeyValues = new();
Expand Down Expand Up @@ -1489,14 +1535,7 @@ protected override ShapedQueryExpression TranslateSelect(ShapedQueryExpression s
}
}

if (TranslateLambdaExpression(source, predicate) is SqlExpression translation)
{
((SelectExpression)source.QueryExpression).ApplyPredicate(translation);

return source;
}

return null;
return TryApplyPredicate(source, predicate) ? source : null;

bool TryExtractPartitionKey(
Expression expression,
Expand Down Expand Up @@ -1739,6 +1778,77 @@ when methodCallExpression.TryGetIndexerArguments(_queryCompilationContext.Model,

#endregion Queryable collection support

private bool TryApplyPredicate(ShapedQueryExpression source, LambdaExpression predicate)
{
var select = (SelectExpression)source.QueryExpression;

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

if (TranslateLambdaExpression(source, predicate) is SqlExpression translation)
{
if (translation is not SqlConstantExpression { Value: true })
{
select.ApplyPredicate(translation);
}

return true;
}

return false;
}

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;
// }

select.ApplyPredicate(predicate);
return true;
}

private bool TryApplyOffset(SelectExpression select, SqlExpression offset)
{
if ((select.Limit is not null || select.Offset is not null) && !TryPushdownIntoSubquery(select))
{
return false;
}

select.ApplyOffset(offset);
return true;
}

private bool TryApplyLimit(SelectExpression select, SqlExpression limit)
{
if (select.Limit is not null && !TryPushdownIntoSubquery(select))
{
return false;
}

select.ApplyLimit(limit);
return true;
}

private bool TryPushdownIntoSubquery(SelectExpression select)
{
if (select.Offset is not null || select.Limit is not null)
{
AddTranslationErrorDetails(CosmosStrings.LimitOffsetNotSupportedInSubqueries);
return false;
}

// TODO: Implement subquery pushdown (#33968); though since Cosmos doesn't support OFFSET/LIMIT in subqueries, this isn't
// going to unlock many scenarios.
AddTranslationErrorDetails(CosmosStrings.NoSubqueryPushdown);
return false;
}

private ShapedQueryExpression? TranslateCountLongCount(ShapedQueryExpression source, LambdaExpression? predicate, Type returnType)
{
// Simplify x.Array.Count() => ARRAY_LENGTH(x.Array) instead of (SELECT COUNT(1) FROM i IN x.Array))
Expand Down
5 changes: 4 additions & 1 deletion src/EFCore.Cosmos/Query/Internal/CosmosSqlTranslatingExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,10 @@ when method.GetGenericMethodDefinition().Equals(EnumerableMethods.Contains):
return TranslateContains(argument, methodCallExpression.Object!);

// For queryable methods, either we translate the whole aggregate or we go to subquery mode
case { Method.IsStatic: true, Arguments.Count: > 0 } when method.DeclaringType == typeof(Queryable):
case { Method.IsStatic: true, Arguments.Count: > 0 }
when method.DeclaringType == typeof(Queryable)
|| method.DeclaringType == typeof(EntityFrameworkQueryableExtensions)
|| method.DeclaringType == typeof(CosmosQueryableExtensions):
return TranslateAsSubquery(methodCallExpression);

default:
Expand Down
Loading

0 comments on commit ac7380e

Please sign in to comment.