From c7598b17b90a116af65a0cd170700dd3cc8d4e4e Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Tue, 14 Dec 2021 14:49:40 -0800 Subject: [PATCH] Query: Mark SelectExpression as immutable after projection has been applied - Add an internal state to remember the mutability which is set to false after ApplyProjection is called. - Add overload of ApplyProjection which ignores shaper to create an immutable select expression to be used as subquery - Add debug check to verify that all SelectExpression are marked as immutable - Clone mapped projection of SqlExpression kind before translating it further to avoid reference sharing with projection which may be applied at later stage Resolves #26532 - all those expressions have debug check in ctor Resolves #26167 - by having more accurate flag for immutability Resolves #26104 - by cloning the projection before translating further. This issue may get resolved in other way after pending selector work if we reuse projections from subquery but regardless the change seems better to avoid reference sharing of SelectExpression objects --- .../Properties/RelationalStrings.Designer.cs | 8 +- .../Properties/RelationalStrings.resx | 59 ++++++------ .../Query/Internal/EnumHasFlagTranslator.cs | 4 +- ...mSqlParameterExpandingExpressionVisitor.cs | 45 --------- ...RelationalQueryTranslationPostprocessor.cs | 31 +++++- ...yableMethodTranslatingExpressionVisitor.cs | 10 +- ...lationalSqlTranslatingExpressionVisitor.cs | 40 +++++--- .../Query/SqlExpressions/ExistsExpression.cs | 7 ++ .../Query/SqlExpressions/InExpression.cs | 6 ++ .../ScalarSubqueryExpression.cs | 7 ++ .../SqlExpressions/SelectExpression.Helper.cs | 1 + .../Query/SqlExpressions/SelectExpression.cs | 96 +++++++++++++++++-- .../Query/FromSqlQueryTestBase.cs | 20 ++-- .../Query/Ef6GroupByTestBase.cs | 6 +- 14 files changed, 219 insertions(+), 121 deletions(-) diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index 7e7db5126aa..d3f77963caf 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -977,12 +977,6 @@ public static string PropertyNotMappedToTable(object? property, object? entityTy GetString("PropertyNotMappedToTable", nameof(property), nameof(entityType), nameof(table)), property, entityType, table); - /// - /// The query contains usage of 'Any' or 'AnyAsync' operation after 'FromSqlRaw' or 'FromSqlInterpolated' method. Using this raw SQL query more than once isn't currently supported. Replace the use of 'Any' or 'AnyAsync' with 'Count' or 'CountAsync'. See https://go.microsoft.com/fwlink/?linkid=2174053 for more information. - /// - public static string QueryFromSqlInsideExists - => GetString("QueryFromSqlInsideExists"); - /// /// The entity type '{entityType}' is not mapped to a table, therefore the entities cannot be persisted to the database. Call 'ToTable' in 'OnModelCreating' to map it to a table. /// @@ -1162,7 +1156,7 @@ public static string UnsupportedOperatorForSqlExpression(object? nodeType, objec nodeType, expressionType); /// - /// No relational type mapping can be found for property '{entity}.{property}' and the current provider doesn't specify a default store type for the properties of type '{clrType}'. + /// No relational type mapping can be found for property '{entity}.{property}' and the current provider doesn't specify a default store type for the properties of type '{clrType}'. /// public static string UnsupportedPropertyType(object? entity, object? property, object? clrType) => string.Format( diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index 366f1912ce3..5aacd82eb7c 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1,17 +1,17 @@  - @@ -726,9 +726,6 @@ The property '{property}' on entity type '{entityType}' is not mapped to '{table}'. - - The query contains usage of 'Any' or 'AnyAsync' operation after 'FromSqlRaw' or 'FromSqlInterpolated' method. Using this raw SQL query more than once isn't currently supported. Replace the use of 'Any' or 'AnyAsync' with 'Count' or 'CountAsync'. See https://go.microsoft.com/fwlink/?linkid=2174053 for more information. - The entity type '{entityType}' is not mapped to a table, therefore the entities cannot be persisted to the database. Call 'ToTable' in 'OnModelCreating' to map it to a table. @@ -840,4 +837,4 @@ 'VisitChildren' must be overridden in the class deriving from 'SqlExpression'. - + \ No newline at end of file diff --git a/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs b/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs index 645947ca365..6d6f2645958 100644 --- a/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs +++ b/src/EFCore.Relational/Query/Internal/EnumHasFlagTranslator.cs @@ -47,9 +47,7 @@ public EnumHasFlagTranslator(ISqlExpressionFactory sqlExpressionFactory) var argument = arguments[0]; return instance.Type != argument.Type ? null - // TODO: If argument is SelectExpression, we need to clone it. - // See issue#26532 - : (SqlExpression)_sqlExpressionFactory.Equal(_sqlExpressionFactory.And(instance, argument), argument); + : _sqlExpressionFactory.Equal(_sqlExpressionFactory.And(instance, argument), argument); } return null; diff --git a/src/EFCore.Relational/Query/Internal/FromSqlParameterExpandingExpressionVisitor.cs b/src/EFCore.Relational/Query/Internal/FromSqlParameterExpandingExpressionVisitor.cs index d543c40c3b7..2606f06a452 100644 --- a/src/EFCore.Relational/Query/Internal/FromSqlParameterExpandingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/Internal/FromSqlParameterExpandingExpressionVisitor.cs @@ -134,12 +134,6 @@ public virtual SelectExpression Expand( Expression.Constant(new CompositeRelationalParameter(parameterExpression.Name!, subParameters))); case ConstantExpression constantExpression: - if (constantExpression.Value is not object?[] - && new FromSqlInExistVerifyingExpressionVisitor(fromSql).Verify(_selectExpression)) - { - throw new InvalidOperationException(RelationalStrings.QueryFromSqlInsideExists); - } - var existingValues = constantExpression.GetConstantValue(); var constantValues = new object?[existingValues.Length]; for (var i = 0; i < existingValues.Length; i++) @@ -173,43 +167,4 @@ public virtual SelectExpression Expand( return null; } } - - private sealed class FromSqlInExistVerifyingExpressionVisitor : ExpressionVisitor - { - private readonly FromSqlExpression _mutatedExpression; - private bool _faulty; - - public FromSqlInExistVerifyingExpressionVisitor(FromSqlExpression mutatedExpression) - { - _mutatedExpression = mutatedExpression; - } - - public bool Verify(SelectExpression selectExpression) - { - _faulty = false; - - Visit(selectExpression); - - return _faulty; - } - - [return: NotNullIfNotNull("expression")] - public override Expression? Visit(Expression? expression) - { - if (_faulty) - { - return expression; - } - - if (expression is ExistsExpression existsExpression - && existsExpression.Subquery.Tables.Contains(_mutatedExpression, ReferenceEqualityComparer.Instance)) - { - _faulty = true; - - return expression; - } - - return base.Visit(expression); - } - } } diff --git a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs index c235168248c..5e9a99029c1 100644 --- a/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryTranslationPostprocessor.cs @@ -39,8 +39,13 @@ public override Expression Process(Expression query) query = base.Process(query); query = new SelectExpressionProjectionApplyingExpressionVisitor( ((RelationalQueryCompilationContext)QueryCompilationContext).QuerySplittingBehavior).Visit(query); + #if DEBUG - query = new TableAliasVerifyingExpressionVisitor().Visit(query); + // Verifies that all SelectExpression are marked as immutable after this point. + new SelectExpressionMutableVerifyingExpressionVisitor().Visit(query); + // Verifies that all table aliases are uniquely assigned without skipping over + // Which points to possible mutation of a SelectExpression being used in multiple places. + new TableAliasVerifyingExpressionVisitor().Visit(query); #endif query = new SelectExpressionPruningExpressionVisitor().Visit(query); query = new SqlExpressionSimplifyingExpressionVisitor(RelationalDependencies.SqlExpressionFactory, _useRelationalNulls) @@ -50,6 +55,30 @@ public override Expression Process(Expression query) return query; } + private sealed class SelectExpressionMutableVerifyingExpressionVisitor : ExpressionVisitor + { + [return: NotNullIfNotNull("expression")] + public override Expression? Visit(Expression? expression) + { + if (expression is SelectExpression selectExpression) + { + if (selectExpression.IsMutable()) + { + throw new InvalidDataException(selectExpression.Print()); + } + } + + if (expression is ShapedQueryExpression shapedQueryExpression) + { + Visit(shapedQueryExpression.QueryExpression); + + return shapedQueryExpression; + } + + return base.Visit(expression); + } + } + private sealed class TableAliasVerifyingExpressionVisitor : ExpressionVisitor { private readonly ScopedVisitor _scopedVisitor = new(); diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 27f76828361..bdbfee354c9 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -184,7 +184,8 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent var selectExpression = (SelectExpression)source.QueryExpression; selectExpression.ApplyPredicate(_sqlExpressionFactory.Not(translation)); - selectExpression.ReplaceProjection(new Dictionary()); + selectExpression.ReplaceProjection(new List()); + selectExpression.ApplyProjection(); if (selectExpression.Limit == null && selectExpression.Offset == null) { @@ -215,7 +216,8 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent } var selectExpression = (SelectExpression)source.QueryExpression; - selectExpression.ReplaceProjection(new Dictionary()); + selectExpression.ReplaceProjection(new List()); + selectExpression.ApplyProjection(); if (selectExpression.Limit == null && selectExpression.Offset == null) { @@ -284,8 +286,8 @@ private static ShapedQueryExpression CreateShapedQueryExpression(IEntityType ent var projection = selectExpression.GetProjection(projectionBindingExpression); if (projection is SqlExpression sqlExpression) { - selectExpression.ReplaceProjection(new List()); - selectExpression.AddToProjection(sqlExpression); + selectExpression.ReplaceProjection(new List { sqlExpression }); + selectExpression.ApplyProjection(); translation = _sqlExpressionFactory.In(translation, selectExpression, false); diff --git a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs index 4d5307a9663..86b332df14f 100644 --- a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs @@ -41,6 +41,7 @@ private static readonly MethodInfo ObjectEqualsMethodInfo private readonly ISqlExpressionFactory _sqlExpressionFactory; private readonly QueryableMethodTranslatingExpressionVisitor _queryableMethodTranslatingExpressionVisitor; private readonly SqlTypeMappingVerifyingExpressionVisitor _sqlTypeMappingVerifyingExpressionVisitor; + private readonly CloningExpressionVisitor _cloningExpressionVisitor; /// /// Creates a new instance of the class. @@ -59,6 +60,7 @@ public RelationalSqlTranslatingExpressionVisitor( _model = queryCompilationContext.Model; _queryableMethodTranslatingExpressionVisitor = queryableMethodTranslatingExpressionVisitor; _sqlTypeMappingVerifyingExpressionVisitor = new SqlTypeMappingVerifyingExpressionVisitor(); + _cloningExpressionVisitor = new CloningExpressionVisitor(); } /// @@ -367,9 +369,15 @@ protected override Expression VisitExtension(Expression extensionExpression) return new EntityReferenceExpression(entityShaperExpression); case ProjectionBindingExpression projectionBindingExpression: - return ((SelectExpression)projectionBindingExpression.QueryExpression) + var mappedProjection = ((SelectExpression)projectionBindingExpression.QueryExpression) .GetProjection(projectionBindingExpression); + // We clone the SqlExpression here so that subquery inside it doesn't share reference with projection anymore. + return mappedProjection is ShapedQueryExpression + || mappedProjection is EntityProjectionExpression + ? mappedProjection + : _cloningExpressionVisitor.Visit(mappedProjection); + case ShapedQueryExpression shapedQueryExpression: if (shapedQueryExpression.ResultCardinality == ResultCardinality.Enumerable) { @@ -429,8 +437,8 @@ protected override Expression VisitExtension(Expression extensionExpression) return sqlExpression; } - subquery.ReplaceProjection(new List()); - subquery.AddToProjection(sqlExpression); + subquery.ReplaceProjection(new List { sqlExpression }); + subquery.ApplyProjection(); SqlExpression scalarSubqueryExpression = new ScalarSubqueryExpression(subquery); @@ -706,7 +714,8 @@ protected override Expression VisitTypeBinary(TypeBinaryExpression typeBinaryExp var predicate = GeneratePredicateTpt(entityProjection); subSelectExpression.ApplyPredicate(predicate); - subSelectExpression.ReplaceProjection(new Dictionary()); + subSelectExpression.ReplaceProjection(new List()); + subSelectExpression.ApplyProjection(); if (subSelectExpression.Limit == null && subSelectExpression.Offset == null) { @@ -842,7 +851,7 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression) private Expression? TryBindMember(Expression? source, MemberIdentity member) { - if (!(source is EntityReferenceExpression entityReferenceExpression)) + if (source is not EntityReferenceExpression entityReferenceExpression) { return null; } @@ -940,8 +949,8 @@ protected override Expression VisitUnary(UnaryExpression unaryExpression) var projectionBindingExpression = (ProjectionBindingExpression)entityShaper.ValueBufferExpression; var entityProjectionExpression = (EntityProjectionExpression)subSelectExpression.GetProjection(projectionBindingExpression); var innerProjection = entityProjectionExpression.BindProperty(property); - subSelectExpression.ReplaceProjection(new List()); - subSelectExpression.AddToProjection(innerProjection); + subSelectExpression.ReplaceProjection(new List { innerProjection }); + subSelectExpression.ApplyProjection(); return new ScalarSubqueryExpression(subSelectExpression); } @@ -1009,7 +1018,7 @@ private bool TryRewriteContainsEntity(Expression source, Expression item, [NotNu { result = null; - if (!(item is EntityReferenceExpression itemEntityReference)) + if (item is not EntityReferenceExpression itemEntityReference) { return false; } @@ -1299,7 +1308,7 @@ when memberInitExpression.Bindings.SingleOrDefault( string baseParameterName, IProperty property) { - if (!(context.ParameterValues[baseParameterName] is IEnumerable baseListParameter)) + if (context.ParameterValues[baseParameterName] is not IEnumerable baseListParameter) { return null; } @@ -1337,7 +1346,7 @@ private static bool IsNullSqlConstantExpression(Expression expression) private static bool TranslationFailed(Expression? original, Expression? translation, out SqlExpression? castTranslation) { if (original != null - && !(translation is SqlExpression)) + && translation is not SqlExpression) { castTranslation = null; return true; @@ -1399,7 +1408,7 @@ private sealed class SqlTypeMappingVerifyingExpressionVisitor : ExpressionVisito protected override Expression VisitExtension(Expression extensionExpression) { if (extensionExpression is SqlExpression sqlExpression - && !(extensionExpression is SqlFragmentExpression)) + && extensionExpression is not SqlFragmentExpression) { if (sqlExpression.TypeMapping == null) { @@ -1410,4 +1419,13 @@ protected override Expression VisitExtension(Expression extensionExpression) return base.VisitExtension(extensionExpression); } } + + private sealed class CloningExpressionVisitor : ExpressionVisitor + { + [return: NotNullIfNotNull("expression")] + public override Expression? Visit(Expression? expression) + => expression is SelectExpression selectExpression + ? selectExpression.Clone() + : base.Visit(expression); + } } diff --git a/src/EFCore.Relational/Query/SqlExpressions/ExistsExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/ExistsExpression.cs index 41bd55bb6b8..07722ce62df 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/ExistsExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/ExistsExpression.cs @@ -26,6 +26,13 @@ public ExistsExpression( RelationalTypeMapping? typeMapping) : base(typeof(bool), typeMapping) { + +#if DEBUG + if (subquery.IsMutable() == true) + { + throw new InvalidOperationException(); + } +#endif Subquery = subquery; IsNegated = negated; } diff --git a/src/EFCore.Relational/Query/SqlExpressions/InExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/InExpression.cs index 813babe3c44..af0777b47bb 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/InExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/InExpression.cs @@ -56,6 +56,12 @@ private InExpression( RelationalTypeMapping? typeMapping) : base(typeof(bool), typeMapping) { +#if DEBUG + if (subquery?.IsMutable() == true) + { + throw new InvalidOperationException(); + } +#endif Item = item; Subquery = subquery; Values = values; diff --git a/src/EFCore.Relational/Query/SqlExpressions/ScalarSubqueryExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/ScalarSubqueryExpression.cs index 889226d2709..32fd0c4d6bd 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/ScalarSubqueryExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/ScalarSubqueryExpression.cs @@ -31,6 +31,13 @@ private static SelectExpression Verify(SelectExpression selectExpression) throw new InvalidOperationException(CoreStrings.TranslationFailed(selectExpression.Print())); } +#if DEBUG + if (selectExpression.IsMutable() == true) + { + throw new InvalidOperationException(); + } +#endif + return selectExpression; } diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs index dde4e8ad491..17890f091cb 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs @@ -772,6 +772,7 @@ private sealed class CloningExpressionVisitor : ExpressionVisitor _usedAliases = selectExpression._usedAliases.ToHashSet(), _projectionMapping = newProjectionMappings }; + newSelectExpression._mutable = selectExpression._mutable; newSelectExpression._tptLeftJoinTables.AddRange(selectExpression._tptLeftJoinTables); // Since identifiers are ColumnExpression, they are not visited since they don't contain SelectExpression inside it. diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs index c6f59587e63..1f4bb2cc2e4 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -41,12 +41,13 @@ public sealed partial class SelectExpression : TableExpressionBase private readonly List _tableReferences = new(); private readonly List _groupBy = new(); private readonly List _orderings = new(); - private HashSet _usedAliases = new(); private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _identifier = new(); private readonly List<(ColumnExpression Column, ValueComparer Comparer)> _childIdentifiers = new(); - private readonly List _tptLeftJoinTables = new(); + + private bool _mutable = true; + private HashSet _usedAliases = new(); private Dictionary _projectionMapping = new(); private List _clientProjections = new(); private readonly List _aliasForClientProjections = new(); @@ -383,7 +384,71 @@ public void ApplyDistinct() } /// - /// Adds expressions from projection mapping to projection if not done already. + /// Adds expressions from projection mapping to projection ignoring the shaper expression. This method should only be used + /// when populating projection in subquery. + /// + public void ApplyProjection() + { + if (!_mutable) + { + throw new InvalidOperationException("Applying projection on already finalized select expression"); + } + + _mutable = false; + if (_clientProjections.Count > 0) + { + for (var i = 0; i < _clientProjections.Count; i++) + { + switch(_clientProjections[i]) + { + case EntityProjectionExpression entityProjectionExpression: + AddEntityProjection(entityProjectionExpression); + break; + + case SqlExpression sqlExpression: + AddToProjection(sqlExpression, _aliasForClientProjections[i]); + break; + + default: + throw new InvalidOperationException("Invalid type of projection to add when not associated with shaper expression."); + } + + } + + _clientProjections.Clear(); + } + else + { + foreach (var (_, expression) in _projectionMapping) + { + if (expression is EntityProjectionExpression entityProjectionExpression) + { + AddEntityProjection(entityProjectionExpression); + } + else + { + AddToProjection((SqlExpression)expression); + } + } + _projectionMapping.Clear(); + } + + void AddEntityProjection(EntityProjectionExpression entityProjectionExpression) + { + foreach (var property in GetAllPropertiesInHierarchy(entityProjectionExpression.EntityType)) + { + AddToProjection(entityProjectionExpression.BindProperty(property), null); + } + + if (entityProjectionExpression.DiscriminatorExpression != null) + { + AddToProjection(entityProjectionExpression.DiscriminatorExpression, DiscriminatorColumnAlias); + } + } + } + + /// + /// Adds expressions from projection mapping to projection and generate updated shaper expression for materialization. /// /// Current shaper expression which will shape results of this select expression. /// The result cardinality of this query expression. @@ -394,11 +459,12 @@ public Expression ApplyProjection( ResultCardinality resultCardinality, QuerySplittingBehavior querySplittingBehavior) { - if (Projection.Any()) + if (!_mutable) { throw new InvalidOperationException("Applying projection on already finalized select expression"); } + _mutable = false; if (_clientProjections.Count > 0) { EntityShaperNullableMarkingExpressionVisitor? entityShaperNullableMarkingExpressionVisitor = null; @@ -448,6 +514,8 @@ public Expression ApplyProjection( if (querySplittingBehavior == QuerySplittingBehavior.SplitQuery && containsCollection) { baseSelectExpression = (SelectExpression)cloningExpressionVisitor!.Visit(this); + // We mark this as mutable because the split query will combine into this and take it over. + baseSelectExpression._mutable = true; if (resultCardinality == ResultCardinality.Single || resultCardinality == ResultCardinality.SingleOrDefault) { @@ -485,6 +553,7 @@ static void UpdateLimit(SelectExpression selectExpression) // again so it does contain the single result subquery too. We erase projections for it since it would be non-empty. earlierClientProjectionCount = _clientProjections.Count; baseSelectExpression = (SelectExpression)cloningExpressionVisitor!.Visit(this); + baseSelectExpression._mutable = true; baseSelectExpression._projection.Clear(); } @@ -1584,6 +1653,11 @@ private void ApplySetOperation(SetOperationType setOperationType, SelectExpressi _tables.Add(setExpression); _tableReferences.Add(tableReferenceExpression); + // Mark both inner subqueries as immutable + select1._mutable = false; + select2._mutable = false; + + // We should apply _identifiers only when it is distinct and actual select expression had identifiers. if (distinct && outerIdentifiers.Length > 0) @@ -1725,6 +1799,7 @@ public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory) new List(), new List(), new List()); + dummySelectExpression._mutable = false; if (Orderings.Any() || Limit != null @@ -2585,6 +2660,7 @@ private SqlRemappingVisitor PushdownIntoSubqueryInternal() Limit = Limit }; subquery._usedAliases = _usedAliases; + subquery._mutable = false; _tables.Clear(); _tableReferences.Clear(); _groupBy.Clear(); @@ -3059,7 +3135,7 @@ private static string GenerateUniqueAlias(HashSet usedAliases, string cu /// protected override Expression VisitChildren(ExpressionVisitor visitor) { - if (_projection.Count == 0) + if (_mutable) { // If projection is not populated then we need to treat this as mutable object since it is not final yet. if (_clientProjections.Count > 0) @@ -3226,7 +3302,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) Tags = Tags, _usedAliases = _usedAliases }; - + newSelectExpression._mutable = false; newSelectExpression._tptLeftJoinTables.AddRange(_tptLeftJoinTables); newSelectExpression._identifier.AddRange(identifier.Zip(_identifier).Select(e => (e.First, e.Second.Comparer))); @@ -3315,6 +3391,8 @@ public SelectExpression Update( SqlExpression? limit, SqlExpression? offset) { + Check.DebugAssert(!_mutable, "SelectExpression shouldn't be mutable when calling this method."); + var projectionMapping = new Dictionary(); foreach (var (projectionMember, expression) in _projectionMapping) { @@ -3335,6 +3413,8 @@ public SelectExpression Update( Tags = Tags }; + newSelectExpression._mutable = false; + // We don't copy identifiers because when we are doing reconstruction so projection is already applied. // Update method should not be used pre-projection application. There are other methods to change SelectExpression. @@ -3548,4 +3628,8 @@ public override int GetHashCode() return hash.ToHashCode(); } + +#if DEBUG + internal bool IsMutable() => _mutable; +#endif } diff --git a/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs index f0f52b4b023..ea92dd45a7b 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/FromSqlQueryTestBase.cs @@ -1260,11 +1260,11 @@ public virtual async Task FromSql_used_twice_without_parameters(bool async) ? await query.AnyAsync() : query.Any(); - Assert.Equal( - RelationalStrings.QueryFromSqlInsideExists, - async - ? (await Assert.ThrowsAsync(() => query.AnyAsync())).Message - : Assert.Throws(() => query.Any()).Message); + var result2 = async + ? await query.AnyAsync() + : query.Any(); + + Assert.Equal(result1, result2); } [ConditionalTheory] @@ -1281,11 +1281,11 @@ public virtual async Task FromSql_used_twice_with_parameters(bool async) ? await query.AnyAsync() : query.Any(); - Assert.Equal( - RelationalStrings.QueryFromSqlInsideExists, - async - ? (await Assert.ThrowsAsync(() => query.AnyAsync())).Message - : Assert.Throws(() => query.Any()).Message); + var result2 = async + ? await query.AnyAsync() + : query.Any(); + + Assert.Equal(result1, result2); } [ConditionalTheory] diff --git a/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs b/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs index 24b9128fc4f..3a2c173d2d3 100644 --- a/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs +++ b/test/EFCore.Specification.Tests/Query/Ef6GroupByTestBase.cs @@ -458,7 +458,7 @@ public virtual Task Whats_new_2021_sample_2(bool async) .OrderBy(e => e.Key) .Select(g => g.First())); - [ConditionalTheory(Skip = "Issue #26104")] // From #12640 + [ConditionalTheory] // From #12640 [MemberData(nameof(IsAsyncData))] public virtual Task Whats_new_2021_sample_3(bool async) => AssertQuery( @@ -486,7 +486,7 @@ into people Count = people.Count() }); - [ConditionalTheory(Skip = "Issue #26104")] // From #12601 + [ConditionalTheory] // From #12601 [MemberData(nameof(IsAsyncData))] public virtual Task Whats_new_2021_sample_5(bool async) => AssertQuery( @@ -497,7 +497,7 @@ public virtual Task Whats_new_2021_sample_5(bool async) .OrderBy(e => e), assertOrder: true); - [ConditionalTheory(Skip = "Issue #26104")] // From #12600 + [ConditionalTheory] // From #12600 [MemberData(nameof(IsAsyncData))] public virtual Task Whats_new_2021_sample_6(bool async) => AssertQuery(