Skip to content

Commit a310f2a

Browse files
committed
Generate OPENJSON with WITH unless ordering is required
Part of dotnet#13617
1 parent fc5be30 commit a310f2a

File tree

43 files changed

+806
-671
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+806
-671
lines changed

Diff for: src/EFCore.Relational/Query/QuerySqlGenerator.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -697,8 +697,7 @@ string GetUniqueParameterName(string currentName)
697697
/// <inheritdoc />
698698
protected override Expression VisitOrdering(OrderingExpression orderingExpression)
699699
{
700-
if (orderingExpression.Expression is SqlConstantExpression
701-
|| orderingExpression.Expression is SqlParameterExpression)
700+
if (orderingExpression.Expression is SqlConstantExpression or SqlParameterExpression)
702701
{
703702
_relationalCommandBuilder.Append("(SELECT 1)");
704703
}

Diff for: src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs

+5-4
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent
575575

576576
/// <inheritdoc />
577577
protected override ShapedQueryExpression? TranslateCount(ShapedQueryExpression source, LambdaExpression? predicate)
578-
=> TranslateAggregateWithPredicate(source, predicate, QueryableMethods.CountWithoutPredicate);
578+
=> TranslateAggregateWithPredicate(source, predicate, QueryableMethods.CountWithoutPredicate, liftOrderings: false);
579579

580580
/// <inheritdoc />
581581
protected override ShapedQueryExpression? TranslateDefaultIfEmpty(ShapedQueryExpression source, Expression? defaultValue)
@@ -914,7 +914,7 @@ private SqlExpression CreateJoinPredicate(Expression outerKey, Expression innerK
914914

915915
/// <inheritdoc />
916916
protected override ShapedQueryExpression? TranslateLongCount(ShapedQueryExpression source, LambdaExpression? predicate)
917-
=> TranslateAggregateWithPredicate(source, predicate, QueryableMethods.LongCountWithoutPredicate);
917+
=> TranslateAggregateWithPredicate(source, predicate, QueryableMethods.LongCountWithoutPredicate, liftOrderings: false);
918918

919919
/// <inheritdoc />
920920
protected override ShapedQueryExpression? TranslateMax(ShapedQueryExpression source, LambdaExpression? selector, Type resultType)
@@ -2377,7 +2377,8 @@ private static Expression MatchShaperNullabilityForSetOperation(Expression shape
23772377
private ShapedQueryExpression? TranslateAggregateWithPredicate(
23782378
ShapedQueryExpression source,
23792379
LambdaExpression? predicate,
2380-
MethodInfo predicateLessMethodInfo)
2380+
MethodInfo predicateLessMethodInfo,
2381+
bool liftOrderings)
23812382
{
23822383
if (predicate != null)
23832384
{
@@ -2396,7 +2397,7 @@ private static Expression MatchShaperNullabilityForSetOperation(Expression shape
23962397
selectExpression.ReplaceProjection(new List<Expression>());
23972398
}
23982399

2399-
selectExpression.PrepareForAggregate();
2400+
selectExpression.PrepareForAggregate(liftOrderings);
24002401
var selector = _sqlExpressionFactory.Fragment("*");
24012402
var methodCall = Expression.Call(
24022403
predicateLessMethodInfo.MakeGenericMethod(selector.Type),

Diff for: src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,9 @@ private sealed class CloningExpressionVisitor : ExpressionVisitor
10141014
return newTpcTable;
10151015
}
10161016

1017+
case IClonableTableExpressionBase cloneable:
1018+
return cloneable.Clone();
1019+
10171020
case TableValuedFunctionExpression tableValuedFunctionExpression:
10181021
{
10191022
var newArguments = new SqlExpression[tableValuedFunctionExpression.Arguments.Count];
@@ -1036,9 +1039,6 @@ private sealed class CloningExpressionVisitor : ExpressionVisitor
10361039
return newTableValuedFunctionExpression;
10371040
}
10381041

1039-
case IClonableTableExpressionBase cloneable:
1040-
return cloneable.Clone();
1041-
10421042
// join and set operations are fine, because they contain other TableExpressionBases inside, that will get cloned
10431043
// and therefore set expression's Update function will generate a new instance.
10441044
case JoinExpressionBase or SetOperationBase:

Diff for: src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs

+2-6
Original file line numberDiff line numberDiff line change
@@ -3919,14 +3919,14 @@ public bool IsNonComposedFromSql()
39193919
/// <summary>
39203920
/// Prepares the <see cref="SelectExpression" /> to apply aggregate operation over it.
39213921
/// </summary>
3922-
public void PrepareForAggregate()
3922+
public void PrepareForAggregate(bool liftOrderings = true)
39233923
{
39243924
if (IsDistinct
39253925
|| Limit != null
39263926
|| Offset != null
39273927
|| _groupBy.Count > 0)
39283928
{
3929-
PushdownIntoSubquery();
3929+
PushdownIntoSubqueryInternal(liftOrderings);
39303930
}
39313931
}
39323932

@@ -4664,10 +4664,6 @@ protected override void Print(ExpressionPrinter expressionPrinter)
46644664
expressionPrinter.AppendLine().Append("ORDER BY ");
46654665
expressionPrinter.VisitCollection(Orderings);
46664666
}
4667-
else if (Offset != null)
4668-
{
4669-
expressionPrinter.AppendLine().Append("ORDER BY (SELECT 1)");
4670-
}
46714667

46724668
if (Offset != null)
46734669
{

Diff for: src/EFCore.Relational/Query/SqlExpressions/ValuesExpression.cs

+1
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ public virtual ValuesExpression Update(IReadOnlyList<RowValueExpression> rowValu
116116
protected override TableExpressionBase CreateWithAnnotations(IEnumerable<IAnnotation> annotations)
117117
=> new ValuesExpression(Alias, RowValues, ColumnNames, annotations);
118118

119+
// TODO: Deep clone, see #30982
119120
/// <inheritdoc />
120121
public virtual TableExpressionBase Clone()
121122
=> CreateWithAnnotations(GetAnnotations());

Diff for: src/EFCore.SqlServer/Query/Internal/SqlServerOpenJsonExpression.cs

+22-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;
2020
/// doing so can result in application failures when updating to a new Entity Framework Core release.
2121
/// </para>
2222
/// </remarks>
23-
public class SqlServerOpenJsonExpression : TableValuedFunctionExpression
23+
public class SqlServerOpenJsonExpression : TableValuedFunctionExpression, IClonableTableExpressionBase
2424
{
2525
/// <summary>
2626
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -80,6 +80,26 @@ public virtual SqlServerOpenJsonExpression Update(
8080
? this
8181
: new SqlServerOpenJsonExpression(Alias, jsonExpression, path, columnInfos);
8282

83+
84+
/// <summary>
85+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
86+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
87+
/// any release. You should only use it directly in your code with extreme caution and knowing that
88+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
89+
/// </summary>
90+
// TODO: Deep clone, see #30982
91+
public virtual TableExpressionBase Clone()
92+
{
93+
var clone = new SqlServerOpenJsonExpression(Alias, JsonExpression, Path, ColumnInfos);
94+
95+
foreach (var annotation in GetAnnotations())
96+
{
97+
clone.AddAnnotation(annotation.Name, annotation.Value);
98+
}
99+
100+
return clone;
101+
}
102+
83103
/// <inheritdoc />
84104
protected override void Print(ExpressionPrinter expressionPrinter)
85105
{
@@ -145,5 +165,5 @@ public override int GetHashCode()
145165
/// any release. You should only use it directly in your code with extreme caution and knowing that
146166
/// doing so can result in application failures when updating to a new Entity Framework Core release.
147167
/// </summary>
148-
public readonly record struct ColumnInfo(string Name, string? StoreType, string? Path = null, bool AsJson = false);
168+
public readonly record struct ColumnInfo(string Name, string StoreType, string? Path = null, bool AsJson = false);
149169
}

Diff for: src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessor.cs

+110-10
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;
1515
/// </summary>
1616
public class SqlServerQueryTranslationPostprocessor : RelationalQueryTranslationPostprocessor
1717
{
18-
private readonly SkipWithoutOrderByInSplitQueryVerifyingExpressionVisitor
19-
_skipWithoutOrderByInSplitQueryVerifyingExpressionVisitor = new();
18+
private readonly OpenJsonPostprocessor _openJsonPostprocessor;
19+
private readonly SkipWithoutOrderByInSplitQueryVerifier _skipWithoutOrderByInSplitQueryVerifier = new();
2020

2121
/// <summary>
2222
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
@@ -27,9 +27,11 @@ private readonly SkipWithoutOrderByInSplitQueryVerifyingExpressionVisitor
2727
public SqlServerQueryTranslationPostprocessor(
2828
QueryTranslationPostprocessorDependencies dependencies,
2929
RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
30-
QueryCompilationContext queryCompilationContext)
30+
QueryCompilationContext queryCompilationContext,
31+
IRelationalTypeMappingSource typeMappingSource)
3132
: base(dependencies, relationalDependencies, queryCompilationContext)
3233
{
34+
_openJsonPostprocessor = new(typeMappingSource, relationalDependencies.SqlExpressionFactory);
3335
}
3436

3537
/// <summary>
@@ -40,14 +42,15 @@ public SqlServerQueryTranslationPostprocessor(
4042
/// </summary>
4143
public override Expression Process(Expression query)
4244
{
43-
var result = base.Process(query);
45+
query = base.Process(query);
4446

45-
_skipWithoutOrderByInSplitQueryVerifyingExpressionVisitor.Visit(result);
47+
query = _openJsonPostprocessor.Process(query);
48+
_skipWithoutOrderByInSplitQueryVerifier.Visit(query);
4649

47-
return result;
50+
return query;
4851
}
4952

50-
private sealed class SkipWithoutOrderByInSplitQueryVerifyingExpressionVisitor : ExpressionVisitor
53+
private sealed class SkipWithoutOrderByInSplitQueryVerifier : ExpressionVisitor
5154
{
5255
[return: NotNullIfNotNull("expression")]
5356
public override Expression? Visit(Expression? expression)
@@ -68,9 +71,7 @@ private sealed class SkipWithoutOrderByInSplitQueryVerifyingExpressionVisitor :
6871

6972
return relationalSplitCollectionShaperExpression;
7073

71-
case SelectExpression selectExpression
72-
when selectExpression.Offset != null
73-
&& selectExpression.Orderings.Count == 0:
74+
case SelectExpression { Offset: not null, Orderings.Count: 0 }:
7475
throw new InvalidOperationException(SqlServerStrings.SplitQueryOffsetWithoutOrderBy);
7576

7677
case NonQueryExpression nonQueryExpression:
@@ -81,4 +82,103 @@ private sealed class SkipWithoutOrderByInSplitQueryVerifyingExpressionVisitor :
8182
}
8283
}
8384
}
85+
86+
/// <summary>
87+
/// Converts <see cref="SqlServerOpenJsonExpression" /> expressions with WITH (the default) to OPENJSON without WITH when an
88+
/// ordering still exists on the [key] column, i.e. when the ordering of the original JSON array needs to be preserved
89+
/// (e.g. limit/offset).
90+
/// </summary>
91+
private sealed class OpenJsonPostprocessor : ExpressionVisitor
92+
{
93+
private readonly IRelationalTypeMappingSource _typeMappingSource;
94+
private readonly ISqlExpressionFactory _sqlExpressionFactory;
95+
private readonly Dictionary<(SqlServerOpenJsonExpression, string), RelationalTypeMapping> _castsToApply = new();
96+
97+
public OpenJsonPostprocessor(IRelationalTypeMappingSource typeMappingSource, ISqlExpressionFactory sqlExpressionFactory)
98+
=> (_typeMappingSource, _sqlExpressionFactory) = (typeMappingSource, sqlExpressionFactory);
99+
100+
public Expression Process(Expression expression)
101+
{
102+
_castsToApply.Clear();
103+
return Visit(expression);
104+
}
105+
106+
[return: NotNullIfNotNull("expression")]
107+
public override Expression? Visit(Expression? expression)
108+
{
109+
switch (expression)
110+
{
111+
case ShapedQueryExpression shapedQueryExpression:
112+
return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression));
113+
114+
case SelectExpression
115+
{
116+
Tables: [SqlServerOpenJsonExpression { ColumnInfos: not null } openJsonExpression, ..],
117+
Orderings:
118+
[
119+
{
120+
Expression: SqlUnaryExpression
121+
{
122+
OperatorType: ExpressionType.Convert,
123+
Operand: ColumnExpression { Name: "key", Table: var keyColumnTable }
124+
}
125+
}
126+
]
127+
} selectExpression
128+
when keyColumnTable == openJsonExpression:
129+
{
130+
// Remove the WITH clause from the OPENJSON expression
131+
var newOpenJsonExpression = openJsonExpression.Update(
132+
openJsonExpression.JsonExpression,
133+
openJsonExpression.Path,
134+
columnInfos: null);
135+
136+
var newTables = selectExpression.Tables.ToArray();
137+
newTables[0] = newOpenJsonExpression;
138+
139+
var newSelectExpression = selectExpression.Update(
140+
selectExpression.Projection,
141+
newTables,
142+
selectExpression.Predicate,
143+
selectExpression.GroupBy,
144+
selectExpression.Having,
145+
selectExpression.Orderings,
146+
selectExpression.Limit,
147+
selectExpression.Offset);
148+
149+
// Record the OPENJSON expression and its projected column(s), along with the store type we just removed from the WITH
150+
// clause. Then visit the select expression, adding a cast around the matching ColumnExpressions.
151+
// TODO: Need to pass through the type mapping API for converting the JSON value (nvarchar) to the relational store type
152+
// (e.g. datetime2), see #30677
153+
foreach (var column in openJsonExpression.ColumnInfos)
154+
{
155+
var typeMapping = _typeMappingSource.FindMapping(column.StoreType);
156+
Check.DebugAssert(
157+
typeMapping is not null,
158+
$"Could not find mapping for store type {column.StoreType} when converting OPENJSON/WITH");
159+
160+
_castsToApply.Add((newOpenJsonExpression, column.Name), typeMapping);
161+
}
162+
163+
var result = base.Visit(newSelectExpression);
164+
165+
foreach (var column in openJsonExpression.ColumnInfos)
166+
{
167+
_castsToApply.Remove((newOpenJsonExpression, column.Name));
168+
}
169+
170+
return result;
171+
}
172+
173+
case ColumnExpression { Table: SqlServerOpenJsonExpression openJsonTable, Name: var name } columnExpression
174+
when _castsToApply.TryGetValue((openJsonTable, name), out var typeMapping):
175+
{
176+
return _sqlExpressionFactory.Convert(columnExpression, columnExpression.Type, typeMapping);
177+
}
178+
179+
default:
180+
return base.Visit(expression);
181+
}
182+
}
183+
}
84184
}

Diff for: src/EFCore.SqlServer/Query/Internal/SqlServerQueryTranslationPostprocessorFactory.cs

+6-5
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Query.Internal;
1111
/// </summary>
1212
public class SqlServerQueryTranslationPostprocessorFactory : IQueryTranslationPostprocessorFactory
1313
{
14+
private readonly IRelationalTypeMappingSource _typeMappingSource;
15+
1416
/// <summary>
1517
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
1618
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
@@ -19,10 +21,12 @@ public class SqlServerQueryTranslationPostprocessorFactory : IQueryTranslationPo
1921
/// </summary>
2022
public SqlServerQueryTranslationPostprocessorFactory(
2123
QueryTranslationPostprocessorDependencies dependencies,
22-
RelationalQueryTranslationPostprocessorDependencies relationalDependencies)
24+
RelationalQueryTranslationPostprocessorDependencies relationalDependencies,
25+
IRelationalTypeMappingSource typeMappingSource)
2326
{
2427
Dependencies = dependencies;
2528
RelationalDependencies = relationalDependencies;
29+
_typeMappingSource = typeMappingSource;
2630
}
2731

2832
/// <summary>
@@ -42,8 +46,5 @@ public SqlServerQueryTranslationPostprocessorFactory(
4246
/// doing so can result in application failures when updating to a new Entity Framework Core release.
4347
/// </summary>
4448
public virtual QueryTranslationPostprocessor Create(QueryCompilationContext queryCompilationContext)
45-
=> new SqlServerQueryTranslationPostprocessor(
46-
Dependencies,
47-
RelationalDependencies,
48-
queryCompilationContext);
49+
=> new SqlServerQueryTranslationPostprocessor(Dependencies, RelationalDependencies, queryCompilationContext, _typeMappingSource);
4950
}

0 commit comments

Comments
 (0)