Skip to content

Commit c0c0e08

Browse files
maumarmaumar
committed
Fix to #28648 - Json/Query: translate element access of a json array
Converting indexer into AsQueryable().ElementAt(x) so that nav expansion can understand it and inject MaterializeCollectionNavigationExpression in the right places. Then in translation we recognize the pattern and convert it back to element access on a JsonQueryExpression. In order to shape the results correctly, we need to add all the array indexes (that are not constants) to the projection, so that we can populate the ordinal keys correctly (and also to do de-duplication). Fixes #28648
1 parent 089a035 commit c0c0e08

12 files changed

+815
-29
lines changed

‎src/EFCore.Relational/Query/Internal/CollectionIndexerToElementAtConvertingExpressionVisitor.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace Microsoft.EntityFrameworkCore.Query.Internal;
5+
6+
/// <summary>
7+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
8+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
9+
/// any release. You should only use it directly in your code with extreme caution and knowing that
10+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
11+
/// </summary>
12+
internal class CollectionIndexerToElementAtConvertingExpressionVisitor : ExpressionVisitor
13+
{
14+
protected override Expression VisitMethodCall(MethodCallExpression methodCallExpression)
15+
{
16+
if (methodCallExpression.Method.Name == "get_Item"
17+
&& !methodCallExpression.Method.IsStatic
18+
&& methodCallExpression.Method.DeclaringType != null
19+
&& methodCallExpression.Method.DeclaringType != typeof(string)
20+
&& methodCallExpression.Method.DeclaringType != typeof(byte[])
21+
&& ((methodCallExpression.Method.DeclaringType.IsGenericType
22+
&& methodCallExpression.Method.DeclaringType.GetGenericTypeDefinition() == typeof(List<>))
23+
|| methodCallExpression.Method.DeclaringType.IsArray))
24+
{
25+
var source = Visit(methodCallExpression.Object!);
26+
var index = Visit(methodCallExpression.Arguments[0]);
27+
var sourceTypeArgument = source.Type.GetSequenceType();
28+
29+
return Expression.Call(
30+
QueryableMethods.ElementAt.MakeGenericMethod(sourceTypeArgument),
31+
Expression.Call(
32+
QueryableMethods.AsQueryable.MakeGenericMethod(sourceTypeArgument),
33+
source),
34+
index);
35+
}
36+
37+
return base.VisitMethodCall(methodCallExpression);
38+
}
39+
}

‎src/EFCore.Relational/Query/JsonQueryExpression.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,32 @@ public virtual JsonQueryExpression BindNavigation(INavigation navigation)
161161
IsNullable || !navigation.ForeignKey.IsRequiredDependent);
162162
}
163163

164+
/// <summary>
165+
/// Binds a collection element access with this JSON query expression to get the SQL representation.
166+
/// </summary>
167+
/// <param name="collectionIndexExpression">The collection index to bind.</param>
168+
public virtual JsonQueryExpression BindCollectionElement(SqlExpression collectionIndexExpression)
169+
{
170+
var newPath = Path.Take(Path.Count - 1).ToList();
171+
var lastPathSegment = Path.Last();
172+
if (lastPathSegment.CollectionIndexExpression != null)
173+
{
174+
throw new InvalidOperationException("Already accessing collection element.");
175+
}
176+
177+
newPath.Add(new PathSegment(lastPathSegment.Key, collectionIndexExpression));
178+
179+
return new JsonQueryExpression(
180+
EntityType,
181+
JsonColumn,
182+
_keyPropertyMap,
183+
newPath,
184+
EntityType.ClrType,
185+
collection: false,
186+
// TODO: is this the right nullable?
187+
nullable: true);
188+
}
189+
164190
/// <summary>
165191
/// Makes this JSON query expression nullable.
166192
/// </summary>

‎src/EFCore.Relational/Query/PathSegment.cs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,30 @@ public PathSegment(string key)
2525
Key = key;
2626
}
2727

28+
/// <summary>
29+
/// Creates a new instance of the <see cref="PathSegment" /> class.
30+
/// </summary>
31+
/// <param name="key">A key which is being accessed in the JSON.</param>
32+
/// <param name="collectionIndexExpression">A collection index which is being accessed in the JSON.</param>
33+
public PathSegment(string key, SqlExpression collectionIndexExpression)
34+
: this(key)
35+
{
36+
CollectionIndexExpression = collectionIndexExpression;
37+
}
38+
2839
/// <summary>
2940
/// The key which is being accessed in the JSON.
3041
/// </summary>
3142
public virtual string Key { get; }
3243

44+
/// <summary>
45+
/// The index of the collection which is being accessed in the JSON.
46+
/// </summary>
47+
public virtual SqlExpression? CollectionIndexExpression { get; }
48+
3349
/// <inheritdoc />
3450
public override string ToString()
35-
=> (Key == "$" ? "" : ".") + Key;
51+
=> (Key == "$" ? "" : ".") + Key + (CollectionIndexExpression == null ? "" : $"[{CollectionIndexExpression}]");
3652

3753
/// <inheritdoc />
3854
public override bool Equals(object? obj)
@@ -42,9 +58,11 @@ public override bool Equals(object? obj)
4258
&& Equals(pathSegment));
4359

4460
private bool Equals(PathSegment pathSegment)
45-
=> Key == pathSegment.Key;
61+
=> Key == pathSegment.Key
62+
&& ((CollectionIndexExpression == null && pathSegment.CollectionIndexExpression == null)
63+
|| (CollectionIndexExpression != null && CollectionIndexExpression.Equals(pathSegment.CollectionIndexExpression)));
4664

4765
/// <inheritdoc />
4866
public override int GetHashCode()
49-
=> HashCode.Combine(Key);
67+
=> HashCode.Combine(Key, CollectionIndexExpression);
5068
}

‎src/EFCore.Relational/Query/RelationalQueryTranslationPreprocessor.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public override Expression NormalizeQueryableMethod(Expression expression)
3737
expression = new RelationalQueryMetadataExtractingExpressionVisitor(_relationalQueryCompilationContext).Visit(expression);
3838
expression = base.NormalizeQueryableMethod(expression);
3939
expression = new TableValuedFunctionToQueryRootConvertingExpressionVisitor(QueryCompilationContext.Model).Visit(expression);
40+
expression = new CollectionIndexerToElementAtConvertingExpressionVisitor().Visit(expression);
4041

4142
return expression;
4243
}

‎src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1640,6 +1640,65 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
16401640
return TryExpand(source, MemberIdentity.Create(navigationName))
16411641
?? methodCallExpression.Update(null!, new[] { source, methodCallExpression.Arguments[1] });
16421642
}
1643+
else if (methodCallExpression.Method.IsGenericMethod
1644+
&& (methodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.ElementAt
1645+
|| methodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.ElementAtOrDefault))
1646+
{
1647+
source = methodCallExpression.Arguments[0];
1648+
var selectMethodCallExpression = default(MethodCallExpression);
1649+
1650+
if (source is MethodCallExpression sourceMethod
1651+
&& sourceMethod.Method.IsGenericMethod
1652+
&& sourceMethod.Method.GetGenericMethodDefinition() == QueryableMethods.Select)
1653+
{
1654+
selectMethodCallExpression = sourceMethod;
1655+
source = sourceMethod.Arguments[0];
1656+
}
1657+
1658+
if (source is MethodCallExpression asQueryableMethodCallExpression
1659+
&& asQueryableMethodCallExpression.Method.IsGenericMethod
1660+
&& asQueryableMethodCallExpression.Method.GetGenericMethodDefinition() == QueryableMethods.AsQueryable)
1661+
{
1662+
source = asQueryableMethodCallExpression.Arguments[0];
1663+
}
1664+
1665+
source = Visit(source);
1666+
1667+
if (source is JsonQueryExpression jsonQueryExpression)
1668+
{
1669+
var collectionIndexExpression = _sqlTranslator.Translate(methodCallExpression.Arguments[1]!);
1670+
1671+
if (collectionIndexExpression == null)
1672+
{
1673+
return methodCallExpression.Update(null!, new[] { source, methodCallExpression.Arguments[1] });
1674+
}
1675+
1676+
collectionIndexExpression = _sqlExpressionFactory.ApplyDefaultTypeMapping(collectionIndexExpression);
1677+
var newJsonQuery = jsonQueryExpression.BindCollectionElement(collectionIndexExpression!);
1678+
1679+
var entityShaper = new RelationalEntityShaperExpression(
1680+
jsonQueryExpression.EntityType,
1681+
newJsonQuery,
1682+
nullable: true);
1683+
1684+
// look into select (if there was any)
1685+
// strip the includes
1686+
// and if there was anything (e.g. MaterializeCollectionNavigationExpression) wrap the entity shaper around it and return that
1687+
1688+
if (selectMethodCallExpression != null)
1689+
{
1690+
var selectorLambda = selectMethodCallExpression.Arguments[1].UnwrapLambdaFromQuote();
1691+
var replaced = new ReplacingExpressionVisitor(new[] { selectorLambda.Parameters[0] }, new[] { entityShaper })
1692+
.Visit(selectorLambda.Body);
1693+
1694+
var result = Visit(replaced);
1695+
1696+
return result;
1697+
}
1698+
1699+
return entityShaper;
1700+
}
1701+
}
16431702

16441703
return base.VisitMethodCall(methodCallExpression);
16451704
}

‎src/EFCore.Relational/Query/RelationalShapedQueryCompilingExpressionVisitor.ShaperProcessingExpressionVisitor.cs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
416416
{
417417
if (!_variableShaperMapping.TryGetValue(entityShaperExpression.ValueBufferExpression, out var accessor))
418418
{
419-
if (GetProjectionIndex(projectionBindingExpression) is ValueTuple<int, List<(IProperty, int)>, string[]>
419+
if (GetProjectionIndex(projectionBindingExpression) is ValueTuple<int, List<(IProperty, int)>, string[], int>
420420
jsonProjectionIndex)
421421
{
422422
// json entity at the root
@@ -510,7 +510,7 @@ protected override Expression VisitExtension(Expression extensionExpression)
510510
case CollectionResultExpression collectionResultExpression
511511
when collectionResultExpression.Navigation is INavigation navigation
512512
&& GetProjectionIndex(collectionResultExpression.ProjectionBindingExpression)
513-
is ValueTuple<int, List<(IProperty, int)>, string[]> jsonProjectionIndex:
513+
is ValueTuple<int, List<(IProperty, int)>, string[], int> jsonProjectionIndex:
514514
{
515515
// json entity collection at the root
516516
var (jsonElementParameter, keyValuesParameter) = JsonShapingPreProcess(
@@ -781,7 +781,7 @@ when collectionResultExpression.Navigation is INavigation navigation
781781

782782
// json include case
783783
if (projectionBindingExpression != null
784-
&& GetProjectionIndex(projectionBindingExpression) is ValueTuple<int, List<(IProperty, int)>, string[]>
784+
&& GetProjectionIndex(projectionBindingExpression) is ValueTuple<int, List<(IProperty, int)>, string[], int>
785785
jsonProjectionIndex)
786786
{
787787
var (jsonElementParameter, keyValuesParameter) = JsonShapingPreProcess(
@@ -1236,16 +1236,17 @@ private Expression CreateJsonShapers(
12361236
}
12371237

12381238
private (ParameterExpression, ParameterExpression) JsonShapingPreProcess(
1239-
ValueTuple<int, List<(IProperty, int)>, string[]> projectionIndex,
1239+
ValueTuple<int, List<(IProperty, int)>, string[], int> projectionIndex,
12401240
IEntityType entityType,
12411241
bool isCollection)
12421242
{
12431243
var jsonColumnProjectionIndex = projectionIndex.Item1;
12441244
var keyInfo = projectionIndex.Item2;
12451245
var additionalPath = projectionIndex.Item3;
1246+
var specifiedCollectionIndexesCount = projectionIndex.Item4;
12461247

12471248
var keyValuesParameter = Expression.Parameter(typeof(object[]));
1248-
var keyValues = new Expression[keyInfo.Count];
1249+
var keyValues = new Expression[keyInfo.Count + specifiedCollectionIndexesCount];
12491250

12501251
for (var i = 0; i < keyInfo.Count; i++)
12511252
{
@@ -1262,6 +1263,12 @@ private Expression CreateJsonShapers(
12621263
typeof(object));
12631264
}
12641265

1266+
for (var i = 0; i < specifiedCollectionIndexesCount; i++)
1267+
{
1268+
keyValues[keyInfo.Count + i] = Expression.Convert(
1269+
Expression.Constant(0), typeof(object));
1270+
}
1271+
12651272
var keyValuesInitialize = Expression.NewArrayInit(typeof(object), keyValues);
12661273
var keyValuesAssignment = Expression.Assign(keyValuesParameter, keyValuesInitialize);
12671274

‎src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1529,15 +1529,15 @@ Expression CopyProjectionToOuter(SelectExpression innerSelectExpression, Express
15291529

15301530
remappedConstant = Constant(newDictionary);
15311531
}
1532-
else if (constantValue is ValueTuple<int, List<ValueTuple<IProperty, int>>, string[]> tuple)
1532+
else if (constantValue is ValueTuple<int, List<(IProperty, int)>, string[], int> tuple)
15331533
{
1534-
var newList = new List<ValueTuple<IProperty, int>>();
1534+
var newList = new List<(IProperty, int)>();
15351535
foreach (var item in tuple.Item2)
15361536
{
15371537
newList.Add((item.Item1, projectionIndexMap[item.Item2]));
15381538
}
15391539

1540-
remappedConstant = Constant((projectionIndexMap[tuple.Item1], newList, tuple.Item3));
1540+
remappedConstant = Constant((projectionIndexMap[tuple.Item1], newList, tuple.Item3, tuple.Item4));
15411541
}
15421542
else
15431543
{
@@ -1591,6 +1591,7 @@ static Dictionary<JsonQueryExpression, JsonScalarExpression> BuildJsonProjection
15911591
var ordered = projections
15921592
.OrderBy(x => $"{x.JsonColumn.TableAlias}.{x.JsonColumn.Name}")
15931593
.ThenBy(x => x.Path.Count);
1594+
//.ThenBy(x => x.Path.Last().CollectionIndexExpression == null);
15941595

15951596
var needed = new List<JsonScalarExpression>();
15961597
foreach (var orderedElement in ordered)
@@ -1656,7 +1657,9 @@ ConstantExpression AddJsonProjection(JsonQueryExpression jsonQueryExpression, Js
16561657
keyInfo.Add((keyProperty, AddToProjection(keyColumn)));
16571658
}
16581659

1659-
return Constant((jsonColumnIndex, keyInfo, additionalPath));
1660+
var specifiedCollectionIndexesCount = jsonScalarToAdd.Path.Count(x => x.CollectionIndexExpression != null);
1661+
1662+
return Constant((jsonColumnIndex, keyInfo, additionalPath, specifiedCollectionIndexesCount));
16601663
}
16611664

16621665
static IReadOnlyList<IProperty> GetMappedKeyProperties(IKey key)
@@ -1697,7 +1700,15 @@ static bool JsonEntityContainedIn(JsonScalarExpression sourceExpression, JsonQue
16971700
return false;
16981701
}
16991702

1700-
return sourcePath.SequenceEqual(targetPath.Take(sourcePath.Count));
1703+
if (!sourcePath.SequenceEqual(targetPath.Take(sourcePath.Count)))
1704+
{
1705+
return false;
1706+
}
1707+
1708+
// we can only perform deduplication if there additional path doesn't contain any collection indexes
1709+
// collection indexes can only be part of the source path
1710+
// see issue #29513
1711+
return targetPath.Skip(sourcePath.Count).All(x => x.CollectionIndexExpression == null);
17011712
}
17021713
}
17031714

‎src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,32 @@ protected override Expression VisitJsonScalar(JsonScalarExpression jsonScalarExp
318318

319319
Visit(jsonScalarExpression.JsonColumn);
320320

321-
Sql.Append($",'{string.Join("", jsonScalarExpression.Path.Select(e => e.ToString()))}')");
321+
Sql.Append(",'");
322+
foreach (var pathSegment in jsonScalarExpression.Path)
323+
{
324+
Sql.Append((pathSegment.Key == "$" ? "" : ".") + pathSegment.Key);
325+
if (pathSegment.CollectionIndexExpression != null)
326+
{
327+
Sql.Append("[");
328+
329+
if (pathSegment.CollectionIndexExpression is SqlConstantExpression)
330+
{
331+
Visit(pathSegment.CollectionIndexExpression);
332+
}
333+
else
334+
{
335+
Sql.Append("' + CAST(");
336+
Visit(pathSegment.CollectionIndexExpression);
337+
Sql.Append(" AS ");
338+
Sql.Append(_typeMappingSource.GetMapping(typeof(string)).StoreType);
339+
Sql.Append(") + '");
340+
}
341+
342+
Sql.Append("]");
343+
}
344+
}
345+
346+
Sql.Append("')");
322347

323348
if (jsonScalarExpression.Type != typeof(JsonElement))
324349
{

‎src/EFCore/Query/Internal/NavigationExpandingExpressionVisitor.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,8 +619,6 @@ when QueryableMethods.IsSumWithSelector(method):
619619
// GroupJoin overloads
620620
// Zip
621621
// SequenceEqual overloads
622-
// ElementAt
623-
// ElementAtOrDefault
624622
// SkipWhile
625623
// TakeWhile
626624
// DefaultIfEmpty with argument

‎src/EFCore/Query/Internal/QueryableMethodNormalizingExpressionVisitor.cs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,13 +125,36 @@ protected override Expression VisitMethodCall(MethodCallExpression methodCallExp
125125
visitedExpression = base.VisitMethodCall(methodCallExpression);
126126
}
127127

128-
if (visitedExpression is MethodCallExpression visitedMethodCall
129-
&& visitedMethodCall.Method.DeclaringType == typeof(Queryable)
130-
&& visitedMethodCall.Method.IsGenericMethod)
128+
if (visitedExpression is MethodCallExpression visitedMethodCall)
131129
{
132-
return TryFlattenGroupJoinSelectMany(visitedMethodCall);
130+
if (visitedMethodCall.Method.DeclaringType == typeof(Queryable)
131+
&& visitedMethodCall.Method.IsGenericMethod)
132+
{
133+
return TryFlattenGroupJoinSelectMany(visitedMethodCall);
134+
}
135+
136+
if (visitedMethodCall.Method is MethodInfo methodInfo
137+
&& methodInfo.Name == "get_Item"
138+
&& !methodInfo.IsStatic
139+
&& ((methodInfo.DeclaringType!.IsGenericType && methodInfo.DeclaringType.GetGenericTypeDefinition() == typeof(List<>))
140+
|| methodInfo.DeclaringType.IsArray))
141+
{
142+
return Expression.Call(
143+
QueryableMethods.ElementAt.MakeGenericMethod(visitedMethodCall.Type),
144+
Expression.Call(
145+
QueryableMethods.AsQueryable.MakeGenericMethod(visitedMethodCall.Type),
146+
visitedMethodCall.Object!),
147+
visitedMethodCall.Arguments[0]);
148+
}
133149
}
134150

151+
//if (visitedExpression is MethodCallExpression visitedMethodCall
152+
// && visitedMethodCall.Method.DeclaringType == typeof(Queryable)
153+
// && visitedMethodCall.Method.IsGenericMethod)
154+
//{
155+
// return TryFlattenGroupJoinSelectMany(visitedMethodCall);
156+
//}
157+
135158
return visitedExpression;
136159
}
137160

0 commit comments

Comments
 (0)