Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query: Add TableReferenceExpression as a bridge to ColumnExpression for referential integrity #24458

Merged
merged 1 commit into from
Mar 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,8 @@ namespace Microsoft.EntityFrameworkCore.Cosmos.Query.Internal
/// </summary>
public class EntityProjectionExpression : Expression, IPrintableExpression, IAccessExpression
{
private readonly IDictionary<IProperty, IAccessExpression> _propertyExpressionsMap
= new Dictionary<IProperty, IAccessExpression>();

private readonly IDictionary<INavigation, IAccessExpression> _navigationExpressionsMap
= new Dictionary<INavigation, IAccessExpression>();
private readonly Dictionary<IProperty, IAccessExpression> _propertyExpressionsMap = new();
private readonly Dictionary<INavigation, IAccessExpression> _navigationExpressionsMap = new();

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ namespace Microsoft.EntityFrameworkCore.InMemory.Query.Internal
/// </summary>
public class EntityProjectionExpression : Expression, IPrintableExpression
{
private readonly IDictionary<IProperty, MethodCallExpression> _readExpressionMap;

private readonly IDictionary<INavigation, EntityShaperExpression> _navigationExpressionsCache
= new Dictionary<INavigation, EntityShaperExpression>();
private readonly IReadOnlyDictionary<IProperty, MethodCallExpression> _readExpressionMap;
private readonly Dictionary<INavigation, EntityShaperExpression> _navigationExpressionsCache = new();

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -33,7 +31,7 @@ private readonly IDictionary<INavigation, EntityShaperExpression> _navigationExp
/// </summary>
public EntityProjectionExpression(
IEntityType entityType,
IDictionary<IProperty, MethodCallExpression> readExpressionMap)
IReadOnlyDictionary<IProperty, MethodCallExpression> readExpressionMap)
{
EntityType = entityType;
_readExpressionMap = readExpressionMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ public class InMemoryProjectionBindingExpressionVisitor : ExpressionVisitor
private InMemoryQueryExpression _queryExpression;
private bool _clientEval;

private readonly IDictionary<ProjectionMember, Expression> _projectionMapping
= new Dictionary<ProjectionMember, Expression>();
private Dictionary<EntityProjectionExpression, ProjectionBindingExpression>? _entityProjectionCache;

private readonly Dictionary<ProjectionMember, Expression> _projectionMapping = new();
private readonly Stack<ProjectionMember> _projectionMembers = new();

/// <summary>
Expand Down Expand Up @@ -68,6 +68,7 @@ public virtual Expression Translate(InMemoryQueryExpression queryExpression, Exp
if (result == QueryCompilationContext.NotTranslatedExpression)
{
_clientEval = true;
_entityProjectionCache = new();

expandedExpression = _queryableMethodTranslatingExpressionVisitor.ExpandWeakEntities(_queryExpression, expression);
result = Visit(expandedExpression);
Expand Down Expand Up @@ -255,8 +256,14 @@ protected override Expression VisitExtension(Expression extensionExpression)

if (_clientEval)
{
return entityShaperExpression.Update(
new ProjectionBindingExpression(_queryExpression, _queryExpression.AddToProjection(entityProjectionExpression)));
if (!_entityProjectionCache!.TryGetValue(entityProjectionExpression, out var entityProjectionBinding))
{
entityProjectionBinding = new ProjectionBindingExpression(
_queryExpression, _queryExpression.AddToProjection(entityProjectionExpression));
_entityProjectionCache[entityProjectionExpression] = entityProjectionBinding;
}

return entityShaperExpression.Update(entityProjectionBinding);
}

_projectionMapping[_projectionMembers.Peek()] = entityProjectionExpression;
Expand Down
18 changes: 5 additions & 13 deletions src/EFCore.InMemory/Query/Internal/InMemoryQueryExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ private static readonly PropertyInfo _valueBufferCountMemberInfo
private readonly List<Expression> _clientProjectionExpressions = new();
private readonly List<MethodCallExpression> _projectionMappingExpressions = new();

private readonly IDictionary<EntityProjectionExpression, IDictionary<IProperty, int>> _entityProjectionCache
= new Dictionary<EntityProjectionExpression, IDictionary<IProperty, int>>();

private readonly ParameterExpression _valueBufferParameter;

private IDictionary<ProjectionMember, Expression> _projectionMapping = new Dictionary<ProjectionMember, Expression>();
Expand Down Expand Up @@ -319,17 +316,12 @@ EntityProjectionExpression UpdateEntityProjection(EntityProjectionExpression ent
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual IDictionary<IProperty, int> AddToProjection(EntityProjectionExpression entityProjectionExpression)
public virtual IReadOnlyDictionary<IProperty, int> AddToProjection(EntityProjectionExpression entityProjectionExpression)
{
if (!_entityProjectionCache.TryGetValue(entityProjectionExpression, out var indexMap))
var indexMap = new Dictionary<IProperty, int>();
foreach (var property in GetAllPropertiesInHierarchy(entityProjectionExpression.EntityType))
{
indexMap = new Dictionary<IProperty, int>();
foreach (var property in GetAllPropertiesInHierarchy(entityProjectionExpression.EntityType))
{
indexMap[property] = AddToProjection(entityProjectionExpression.BindProperty(property));
}

_entityProjectionCache[entityProjectionExpression] = indexMap;
indexMap[property] = AddToProjection(entityProjectionExpression.BindProperty(property));
}

return indexMap;
Expand Down Expand Up @@ -1032,7 +1024,7 @@ public ShaperRemappingExpressionVisitor(IDictionary<ProjectionMember, Expression
&& projectionBindingExpression.ProjectionMember != null)
{
var mappingValue = ((ConstantExpression)_projectionMapping[projectionBindingExpression.ProjectionMember]).Value;
return mappingValue is IDictionary<IProperty, int> indexMap
return mappingValue is IReadOnlyDictionary<IProperty, int> indexMap
? new ProjectionBindingExpression(projectionBindingExpression.QueryExpression, indexMap)
: mappingValue is int index
? new ProjectionBindingExpression(
Expand Down
28 changes: 6 additions & 22 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 3 additions & 9 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,9 @@
<value>The entity type '{entityType}' cannot be mapped to a table because it is derived from '{baseType}'. Only base entity types can be mapped to a table.</value>
<comment>Obsolete</comment>
</data>
<data name="DistinctOnCollectionNotSupported" xml:space="preserve">
<value>Using 'Distinct' operation on a projection containing a collection is not supported.</value>
</data>
<data name="DuplicateCheckConstraint" xml:space="preserve">
<value>The check constraint '{checkConstraint}' cannot be added to the entity type '{entityType}' because another check constraint with the same name already exists.</value>
</data>
Expand Down Expand Up @@ -621,15 +624,6 @@
<data name="MissingConcurrencyColumn" xml:space="preserve">
<value>Entity type '{entityType}' doesn't contain a property mapped to the store-generated concurrency token column '{missingColumn}' which is used by another entity type sharing the table '{table}'. Add a store-generated property to '{entityType}' which is mapped to the same column; it may be in shadow state.</value>
</data>
<data name="UnableToTranslateSubqueryWithDistinct" xml:space="preserve">
<value>Subquery with 'Distinct' can only be translated if projection consists only of entities and their properties, or it contains keys of all entities required to generate results on the client side. Either add '{column}' to the projection, remove complex elements of the projection, or rewrite the query to not use the 'Distinct' operation.</value>
</data>
<data name="UnableToTranslateSubqueryWithGroupBy" xml:space="preserve">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since these are gone, we should probably also update message for InsufficientInformationToIdentifyOuterElementOfCollectionJoin

<value>Subquery with 'GroupBy' can only be translated if grouping key consists only of entities and their properties, or it contains keys of all entities required to generate results on the client side. Either add '{column}' to the grouping key, remove complex elements of the grouping key, or rewrite the query to not use the 'GroupBy' operation.</value>
</data>
<data name="DistinctOnCollectionNotSupported" xml:space="preserve">
<value>Using 'Distinct' operation on a projection containing a collection is not supported.</value>
</data>
<data name="MissingOrderingInSelectExpression" xml:space="preserve">
<value>'Reverse' could not be translated to the server because there is no ordering on the server side.</value>
</data>
Expand Down
8 changes: 3 additions & 5 deletions src/EFCore.Relational/Query/EntityProjectionExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,8 @@ namespace Microsoft.EntityFrameworkCore.Query
/// </summary>
public class EntityProjectionExpression : Expression
{
private readonly IDictionary<IProperty, ColumnExpression> _propertyExpressionMap = new Dictionary<IProperty, ColumnExpression>();

private readonly IDictionary<INavigation, EntityShaperExpression> _ownedNavigationMap
= new Dictionary<INavigation, EntityShaperExpression>();
private readonly IReadOnlyDictionary<IProperty, ColumnExpression> _propertyExpressionMap;
private readonly Dictionary<INavigation, EntityShaperExpression> _ownedNavigationMap = new();

/// <summary>
/// Creates a new instance of the <see cref="EntityProjectionExpression" /> class.
Expand All @@ -49,7 +47,7 @@ public EntityProjectionExpression(IEntityType entityType, TableExpressionBase in
/// <param name="discriminatorExpression"> A <see cref="SqlExpression" /> to generate discriminator for each concrete entity type in hierarchy. </param>
public EntityProjectionExpression(
IEntityType entityType,
IDictionary<IProperty, ColumnExpression> propertyExpressionMap,
IReadOnlyDictionary<IProperty, ColumnExpression> propertyExpressionMap,
SqlExpression? discriminatorExpression = null)
{
Check.NotNull(entityType, nameof(entityType));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ 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#24460
: (SqlExpression)_sqlExpressionFactory.Equal(_sqlExpressionFactory.And(instance, argument), argument);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,9 @@ private static readonly MethodInfo _getParameterValueMethodInfo
private SelectExpression _selectExpression;
private SqlExpression[] _existingProjections;
private bool _clientEval;
private Dictionary<EntityProjectionExpression, ProjectionBindingExpression>? _entityProjectionCache;

private readonly IDictionary<ProjectionMember, Expression> _projectionMapping
= new Dictionary<ProjectionMember, Expression>();

private readonly Dictionary<ProjectionMember, Expression> _projectionMapping = new();
private readonly Stack<ProjectionMember> _projectionMembers = new();

/// <summary>
Expand Down Expand Up @@ -77,6 +76,7 @@ public virtual Expression Translate(SelectExpression selectExpression, Expressio
if (result == QueryCompilationContext.NotTranslatedExpression)
{
_clientEval = true;
_entityProjectionCache = new();

expandedExpression = _queryableMethodTranslatingExpressionVisitor.ExpandWeakEntities(_selectExpression, expression);
_existingProjections = _selectExpression.Projection.Select(e => e.Expression).ToArray();
Expand Down Expand Up @@ -334,9 +334,14 @@ protected override Expression VisitExtension(Expression extensionExpression)

if (_clientEval)
{
return entityShaperExpression.Update(
new ProjectionBindingExpression(
_selectExpression, _selectExpression.AddToProjection(entityProjectionExpression)));
if (!_entityProjectionCache!.TryGetValue(entityProjectionExpression, out var entityProjectionBinding))
{
entityProjectionBinding = new ProjectionBindingExpression(
_selectExpression, _selectExpression.AddToProjection(entityProjectionExpression));
_entityProjectionCache[entityProjectionExpression] = entityProjectionBinding;
}

return entityShaperExpression.Update(entityProjectionBinding);
}

_projectionMapping[_projectionMembers.Peek()] = entityProjectionExpression;
Expand Down
Loading