search

dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.74k stars 3.18k forks source link

ShapedQueryExpression.UpdateQueryExpression does not work correctly with ProjectionBindingExpression #31511

Open roji opened 1 year ago

roji commented 1 year ago

ShapedQueryExpression.UpdateQueryExpression can be used to replace the SelectExpression; this method also goes into the ShaperExpression, replacing references to the old SelectExpression with the new one, so that referential integrity is maintained etc:

public virtual ShapedQueryExpression UpdateQueryExpression(Expression queryExpression)
    => !ReferenceEquals(queryExpression, QueryExpression)
        ? new ShapedQueryExpression(
            queryExpression,
            ReplacingExpressionVisitor.Replace(QueryExpression, queryExpression, ShaperExpression), ResultCardinality)
        : this;

However, when the ShaperExpression is a ProjectionBindingExpression, its QueryExpression isn't replaced since ProjectionBindingExpression.VisitChildren just returns this, i.e. does not visit its children. This makes ReplacingExpressionVisitor leave the old QueryExpression if it's embedded within ProjectionBindingExpression, and we lose referential integrity, causing issues.

As we're using UpdateQueryExpression in various contexts where this could happen, there may be some hidden bugs - investigate this.

Note: when this is done, we can simplify code in NpgsqlQueryableMethodTranslatingExpressionVisitor, where we manually reconstruct the shaper expressions instead of just using UpdateQueryExpression.

ajcvickers commented 1 year ago

/cc @maumar FYI