Query: Mark SelectExpression as immutable after projection has been a…

…pplied

- 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

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
- <!--
- Microsoft ResX Schema
-
+ <!-- 
+ Microsoft ResX Schema 
+ 
  Version 2.0
-
- The primary goals of this format is to allow a simple XML format
- that is mostly human readable. The generation and parsing of the
- various data types are done through the TypeConverter classes
+ 
+ The primary goals of this format is to allow a simple XML format 
+ that is mostly human readable. The generation and parsing of the 
+ various data types are done through the TypeConverter classes 
  associated with the data types.
-
+ 
  Example:
-
+ 
  ... ado.net/XML headers & schema ...
  <resheader name="resmimetype">text/microsoft-resx</resheader>
  <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
  <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
  <comment>This is a comment</comment>
  </data>
-
- There are any number of "resheader" rows that contain simple
+ 
+ There are any number of "resheader" rows that contain simple 
  name/value pairs.
-
- Each data row contains a name, and value. The row also contains a
- type or mimetype. Type corresponds to a .NET class that support
- text/value conversion through the TypeConverter architecture.
- Classes that don't support this are serialized and stored with the
+ 
+ Each data row contains a name, and value. The row also contains a 
+ type or mimetype. Type corresponds to a .NET class that support 
+ text/value conversion through the TypeConverter architecture. 
+ Classes that don't support this are serialized and stored with the 
  mimetype set.
-
- The mimetype is used for serialized objects, and tells the
- ResXResourceReader how to depersist the object. This is currently not
+ 
+ The mimetype is used for serialized objects, and tells the 
+ ResXResourceReader how to depersist the object. This is currently not 
  extensible. For a given mimetype the value must be set accordingly:
-
- Note - application/x-microsoft.net.object.binary.base64 is the format
- that the ResXResourceWriter will generate, however the reader can
+ 
+ Note - application/x-microsoft.net.object.binary.base64 is the format 
+ that the ResXResourceWriter will generate, however the reader can 
  read any of the formats listed below.
-
+ 
  mimetype: application/x-microsoft.net.object.binary.base64
- value : The object must be serialized with
+ value : The object must be serialized with 
  : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
  : and then encoded with base64 encoding.
-
+ 
  mimetype: application/x-microsoft.net.object.soap.base64
- value : The object must be serialized with
+ value : The object must be serialized with 
  : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
  : and then encoded with base64 encoding.
 
  mimetype: application/x-microsoft.net.object.bytearray.base64
- value : The object must be serialized into a byte array
+ value : The object must be serialized into a byte array 
  : using a System.ComponentModel.TypeConverter
  : and then encoded with base64 encoding.
  -->
@@ -726,9 +726,6 @@
  <data name="PropertyNotMappedToTable" xml:space="preserve">
  <value>The property '{property}' on entity type '{entityType}' is not mapped to '{table}'.</value>
  </data>
- <data name="QueryFromSqlInsideExists" xml:space="preserve">
- <value>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.</value>
- </data>
  <data name="ReadonlyEntitySaved" xml:space="preserve">
  <value>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.</value>
  </data>
@@ -840,4 +837,4 @@
  <data name="VisitChildrenMustBeOverridden" xml:space="preserve">
  <value>'VisitChildren' must be overridden in the class deriving from 'SqlExpression'.</value>
  </data>
-</root>
+</root>

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;

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<object?[]>();
  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);
- }
- }
 }

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
 }

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<ProjectionMember, Expression>());
+ selectExpression.ReplaceProjection(new List<Expression>());
+ 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<ProjectionMember, Expression>());
+ selectExpression.ReplaceProjection(new List<Expression>());
+ 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<Expression>());
- selectExpression.AddToProjection(sqlExpression);
+ selectExpression.ReplaceProjection(new List<Expression> { sqlExpression });
+ selectExpression.ApplyProjection();
 
  translation = _sqlExpressionFactory.In(translation, selectExpression, false);
 

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;
 
  /// <summary>
  /// Creates a new instance of the <see cref="RelationalSqlTranslatingExpressionVisitor" /> class.
@@ -59,6 +60,7 @@ public RelationalSqlTranslatingExpressionVisitor(
  _model = queryCompilationContext.Model;
  _queryableMethodTranslatingExpressionVisitor = queryableMethodTranslatingExpressionVisitor;
  _sqlTypeMappingVerifyingExpressionVisitor = new SqlTypeMappingVerifyingExpressionVisitor();
+ _cloningExpressionVisitor = new CloningExpressionVisitor();
  }
 
  /// <summary>
@@ -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<Expression>());
- subquery.AddToProjection(sqlExpression);
+ subquery.ReplaceProjection(new List<Expression> { 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<ProjectionMember, Expression>());
+ subSelectExpression.ReplaceProjection(new List<Expression>());
+ 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<Expression>());
- subSelectExpression.AddToProjection(innerProjection);
+ subSelectExpression.ReplaceProjection(new List<Expression> { 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<TEntity> baseListParameter))
+ if (context.ParameterValues[baseParameterName] is not IEnumerable<TEntity> 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);
+ }
 }