From 10d7ffac1d30d5c1a4a258d870254ffbd7f56948 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Thu, 14 Apr 2022 14:00:53 -0700 Subject: [PATCH] Query: Introduce UpdateQueryExpression on ShapedQueryExpression This makes sure that shaper expression has right query expression --- ...eConverterCompensatingExpressionVisitor.cs | 11 +------ .../Query/Internal/SqlExpressionVisitor.cs | 3 +- ...eConverterCompensatingExpressionVisitor.cs | 11 +------ ...sionProjectionApplyingExpressionVisitor.cs | 3 +- ...lExpressionSimplifyingExpressionVisitor.cs | 2 +- ...yableMethodTranslatingExpressionVisitor.cs | 31 ++++++++----------- .../Query/SqlExpressionVisitor.cs | 3 +- .../Query/SqlExpressions/SelectExpression.cs | 7 ++--- src/EFCore/Query/ShapedQueryExpression.cs | 12 +++++++ 9 files changed, 34 insertions(+), 49 deletions(-) diff --git a/src/EFCore.Cosmos/Query/Internal/CosmosValueConverterCompensatingExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/CosmosValueConverterCompensatingExpressionVisitor.cs index 49e246054ff..7ab2250452a 100644 --- a/src/EFCore.Cosmos/Query/Internal/CosmosValueConverterCompensatingExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/CosmosValueConverterCompensatingExpressionVisitor.cs @@ -42,16 +42,7 @@ protected override Expression VisitExtension(Expression extensionExpression) }; private Expression VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression) - { - var selectExpression = shapedQueryExpression.QueryExpression; - var updatedSelectExpression = Visit(selectExpression); - return updatedSelectExpression != selectExpression - ? shapedQueryExpression.Update( - updatedSelectExpression, - ReplacingExpressionVisitor.Replace( - selectExpression, updatedSelectExpression, shapedQueryExpression.ShaperExpression)) - : shapedQueryExpression; - } + => shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); private Expression VisitSelect(SelectExpression selectExpression) { diff --git a/src/EFCore.Cosmos/Query/Internal/SqlExpressionVisitor.cs b/src/EFCore.Cosmos/Query/Internal/SqlExpressionVisitor.cs index 60017c8b670..6d5748089ed 100644 --- a/src/EFCore.Cosmos/Query/Internal/SqlExpressionVisitor.cs +++ b/src/EFCore.Cosmos/Query/Internal/SqlExpressionVisitor.cs @@ -22,8 +22,7 @@ protected override Expression VisitExtension(Expression extensionExpression) switch (extensionExpression) { case ShapedQueryExpression shapedQueryExpression: - return shapedQueryExpression.Update( - Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression); + return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); case ReadItemExpression readItemExpression: return readItemExpression; diff --git a/src/EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs index 0fd2135393b..805297b7e10 100644 --- a/src/EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/RelationalValueConverterCompensatingExpressionVisitor.cs @@ -46,16 +46,7 @@ protected override Expression VisitExtension(Expression extensionExpression) }; private Expression VisitShapedQueryExpression(ShapedQueryExpression shapedQueryExpression) - { - var selectExpression = shapedQueryExpression.QueryExpression; - var updatedSelectExpression = Visit(selectExpression); - return updatedSelectExpression != selectExpression - ? shapedQueryExpression.Update( - updatedSelectExpression, - ReplacingExpressionVisitor.Replace( - selectExpression, updatedSelectExpression, shapedQueryExpression.ShaperExpression)) - : shapedQueryExpression; - } + => shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); private Expression VisitCase(CaseExpression caseExpression) { diff --git a/src/EFCore.Relational/Query/Internal/SelectExpressionProjectionApplyingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/SelectExpressionProjectionApplyingExpressionVisitor.cs index b5614f67b23..130250095b0 100644 --- a/src/EFCore.Relational/Query/Internal/SelectExpressionProjectionApplyingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/SelectExpressionProjectionApplyingExpressionVisitor.cs @@ -35,8 +35,7 @@ public SelectExpressionProjectionApplyingExpressionVisitor(QuerySplittingBehavio protected override Expression VisitExtension(Expression extensionExpression) => extensionExpression is ShapedQueryExpression shapedQueryExpression && shapedQueryExpression.QueryExpression is SelectExpression selectExpression - ? shapedQueryExpression.Update( - selectExpression, + ? shapedQueryExpression.UpdateShaperExpression( selectExpression.ApplyProjection( shapedQueryExpression.ShaperExpression, shapedQueryExpression.ResultCardinality, _querySplittingBehavior)) : base.VisitExtension(extensionExpression); diff --git a/src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs index d45017786db..c31df74cd72 100644 --- a/src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/SqlExpressionSimplifyingExpressionVisitor.cs @@ -40,7 +40,7 @@ protected override Expression VisitExtension(Expression extensionExpression) { if (extensionExpression is ShapedQueryExpression shapedQueryExpression) { - return shapedQueryExpression.Update(Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression); + return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); } // Only applies to 'CASE WHEN condition...' not 'CASE operand WHEN...' diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 00629d05c22..b8bad0935f7 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -193,12 +193,10 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent } translation = _sqlExpressionFactory.Exists(selectExpression, true); + selectExpression = _sqlExpressionFactory.Select(translation); - return source.Update( - _sqlExpressionFactory.Select(translation), - Expression.Convert( - new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), typeof(bool?)), - typeof(bool))); + return source.Update(selectExpression, + Expression.Convert(new ProjectionBindingExpression(selectExpression, new ProjectionMember(), typeof(bool?)), typeof(bool))); } /// @@ -225,12 +223,10 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent } var translation = _sqlExpressionFactory.Exists(selectExpression, false); + selectExpression = _sqlExpressionFactory.Select(translation); - return source.Update( - _sqlExpressionFactory.Select(translation), - Expression.Convert( - new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), typeof(bool?)), - typeof(bool))); + return source.Update(selectExpression, + Expression.Convert(new ProjectionBindingExpression(selectExpression, new ProjectionMember(), typeof(bool?)), typeof(bool))); } /// @@ -290,12 +286,11 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent selectExpression.ApplyProjection(); translation = _sqlExpressionFactory.In(translation, selectExpression, false); + selectExpression = _sqlExpressionFactory.Select(translation); - return source.Update( - _sqlExpressionFactory.Select(translation), + return source.Update(selectExpression, Expression.Convert( - new ProjectionBindingExpression(source.QueryExpression, new ProjectionMember(), typeof(bool?)), - typeof(bool))); + new ProjectionBindingExpression(selectExpression, new ProjectionMember(), typeof(bool?)), typeof(bool))); } } @@ -1089,9 +1084,9 @@ protected override Expression VisitExtension(Expression extensionExpression) var innerSelectExpression = BuildInnerSelectExpressionForOwnedTypeMappedToDifferentTable( entityProjectionExpression, navigation); - - var innerShapedQuery = CreateShapedQueryExpression( - targetEntityType, innerSelectExpression); + + var innerShapedQuery = CreateShapedQueryExpression( + targetEntityType, innerSelectExpression); var makeNullable = foreignKey.PrincipalKey.Properties .Concat(foreignKey.Properties) @@ -1181,7 +1176,7 @@ outerKey is NewArrayExpression newArrayExpression // Owned types don't support inheritance See https://github.com/dotnet/efcore/issues/9630 // So there is no handling for dependent having TPT table = targetEntityType.GetViewOrTableMappings().Single().Table; - var innerSelectExpression = BuildInnerSelectExpressionForOwnedTypeMappedToDifferentTable( + var innerSelectExpression = BuildInnerSelectExpressionForOwnedTypeMappedToDifferentTable( entityProjectionExpression, navigation); diff --git a/src/EFCore.Relational/Query/SqlExpressionVisitor.cs b/src/EFCore.Relational/Query/SqlExpressionVisitor.cs index 340f02ff56c..765d865c33b 100644 --- a/src/EFCore.Relational/Query/SqlExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/SqlExpressionVisitor.cs @@ -22,8 +22,7 @@ protected override Expression VisitExtension(Expression extensionExpression) switch (extensionExpression) { case ShapedQueryExpression shapedQueryExpression: - return shapedQueryExpression.Update( - Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression); + return shapedQueryExpression.UpdateQueryExpression(Visit(shapedQueryExpression.QueryExpression)); case CaseExpression caseExpression: return VisitCase(caseExpression); diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index 531b5ea81bd..80cc3ffa157 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -2893,8 +2893,8 @@ private SqlRemappingVisitor PushdownIntoSubqueryInternal() { if (_clientProjections[i] is ShapedQueryExpression shapedQueryExpression) { - _clientProjections[i] = shapedQueryExpression.Update( - sqlRemappingVisitor.Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression); + _clientProjections[i] = shapedQueryExpression.UpdateQueryExpression( + sqlRemappingVisitor.Visit(shapedQueryExpression.QueryExpression)); } } } @@ -3398,8 +3398,7 @@ List VisitList(List list, bool inPlace, out bool changed) { var item = list[i]; var newItem = item is ShapedQueryExpression shapedQueryExpression - ? shapedQueryExpression.Update( - visitor.Visit(shapedQueryExpression.QueryExpression), shapedQueryExpression.ShaperExpression) + ? shapedQueryExpression.UpdateQueryExpression(visitor.Visit(shapedQueryExpression.QueryExpression)) : visitor.Visit(item); if (newItem != item && newList == list) diff --git a/src/EFCore/Query/ShapedQueryExpression.cs b/src/EFCore/Query/ShapedQueryExpression.cs index 886162a25ef..5f60f9b36b3 100644 --- a/src/EFCore/Query/ShapedQueryExpression.cs +++ b/src/EFCore/Query/ShapedQueryExpression.cs @@ -83,6 +83,18 @@ public virtual ShapedQueryExpression Update(Expression queryExpression, Expressi ? new ShapedQueryExpression(queryExpression, shaperExpression, ResultCardinality) : this; + /// + /// Creates a new expression that is like this one, but using the supplied query expression. If query expression is the same, it will + /// return this expression. + /// + /// The property of the result. + /// This expression if shaper expression did not change, or an expression with the updated shaper expression. + public virtual ShapedQueryExpression UpdateQueryExpression(Expression queryExpression) + => !ReferenceEquals(queryExpression, QueryExpression) + ? new ShapedQueryExpression(queryExpression, + ReplacingExpressionVisitor.Replace(QueryExpression, queryExpression, ShaperExpression), ResultCardinality) + : this; + /// /// Creates a new expression that is like this one, but using the supplied shaper expression. If shaper expression is the same, it will /// return this expression.