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 c9d03fc104f..1411b781958 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 d153e1a1af9..b881ae64ea5 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..f67131f5f06 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,31 @@ public override Expression Process(Expression query) return query; } +#if DEBUG + 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(); @@ -130,4 +160,5 @@ public Expression EntryPoint(Expression expression) } } } +#endif } diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 66cb071461a..00629d05c22 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 90cac3419c2..3d55648a66c 100644 --- a/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalSqlTranslatingExpressionVisitor.cs @@ -429,8 +429,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 +706,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 +843,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 +941,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 +1010,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 +1300,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 +1338,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 +1400,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) { 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 e97a7516fc7..fb70e727647 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs @@ -775,6 +775,7 @@ private sealed class CloningExpressionVisitor : ExpressionVisitor _groupingCorrelationPredicate = groupingCorrelationPredicate, _groupingParentSelectExpressionId = selectExpression._groupingParentSelectExpressionId }; + 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 1793baac15a..53e98657cf9 100644 --- a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs +++ b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs @@ -42,12 +42,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(); @@ -387,7 +388,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. @@ -398,11 +463,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; @@ -452,6 +518,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) { @@ -489,6 +557,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(); } @@ -1601,6 +1670,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) @@ -1743,6 +1817,7 @@ public void ApplyDefaultIfEmpty(ISqlExpressionFactory sqlExpressionFactory) new List(), new List(), Enumerable.Empty()); + dummySelectExpression._mutable = false; if (Orderings.Any() || Limit != null @@ -2604,6 +2679,7 @@ private SqlRemappingVisitor PushdownIntoSubqueryInternal() _groupingParentSelectExpressionId = _groupingParentSelectExpressionId }; subquery._usedAliases = _usedAliases; + subquery._mutable = false; _tables.Clear(); _tableReferences.Clear(); _groupBy.Clear(); @@ -3079,7 +3155,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) @@ -3252,7 +3328,7 @@ protected override Expression VisitChildren(ExpressionVisitor visitor) _groupingCorrelationPredicate = groupingCorrelationPredicate, _groupingParentSelectExpressionId = _groupingParentSelectExpressionId }; - + newSelectExpression._mutable = false; newSelectExpression._tptLeftJoinTables.AddRange(_tptLeftJoinTables); newSelectExpression._identifier.AddRange(identifier.Zip(_identifier).Select(e => (e.First, e.Second.Comparer))); @@ -3341,6 +3417,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) { @@ -3361,6 +3439,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. @@ -3528,4 +3608,8 @@ private bool Equals(SelectExpression selectExpression) public override int GetHashCode() // Since equality above is reference equality, hash code can also be based on reference. => RuntimeHelpers.GetHashCode(this); + +#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]