Skip to content

Stop referencing the query from ProjectionBindingExpression in the shaper #32980

Open

Description

Our ShapedQueryExpression contains a QueryExpression - representing the server side of the query (e.g. a SelectExpression) - and a ShaperExpression - representing the client side. Currently, the shaper contains references to the QueryExpression (via ProjectionBindingExpression), meaning that our query tree is not a tree, but a graph. When the QueryExpression is replaced via ShapedQueryExpression.UpdateQueryExpression(), this does a recursive visit into the ShaperExpression to update all references to point to the new, replacing query expression.

This situation isn't good: we can have subtle referential integrity issues (like in SqlExpressionSimplifyingExpressionVisitor, see #32979), and making things fully immutable is difficult as references constantly need to be updated. We should stop doing this, and have ProjectionBindingExpression implicitly refer to the QueryExpression of the ShapedQueryExpression in which its located

  • hopefully this really is the way things really work. This may be a bit tricky, e.g. it seems that for final GroupBy we diverge the actual QueryExpression and the SelectExpression referenced by its ShaperExpression for some reason.

Note: this situation is similar to how ColumnExpressions were referencing their TableExpressionBase directly via .NET references (changed in #32812). This also meant that the tree was in fact a graph, that we needed to recursively replace columns in many cases when their table changed, and that we had various bugs due to lost referential integrity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions