From b3c4604d59842329654d0ffae69524fe26d537d7 Mon Sep 17 00:00:00 2001 From: Smit Patel Date: Fri, 24 Jul 2020 15:49:01 -0700 Subject: [PATCH] Query: Disallow FromSql on owner when owned navigation is mapped to same table Resolves #21769 It would still work if owned navigation data is not accessed. Though keep in mind that selecting entity will bring owned data too due to auto include --- .../Properties/RelationalStrings.Designer.cs | 6 ++ .../Properties/RelationalStrings.resx | 3 + ...yableMethodTranslatingExpressionVisitor.cs | 77 ++++++++++--------- .../Query/OwnedQueryRelationalTestBase.cs | 23 ++++++ 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index a1d3c47a9d8..2cae152bda5 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -1019,6 +1019,12 @@ public static string TableValuedFunctionNonTPH([CanBeNull] object dbFunction, [C GetString("TableValuedFunctionNonTPH", nameof(dbFunction), nameof(entityType)), dbFunction, entityType); + /// + /// Using 'FromSqlRaw' or 'FromSqlInterpolated' on an entity type which has owned reference navigations sharing same table is not supported. + /// + public static string CustomQueryMappingOnOwner + => GetString("CustomQueryMappingOnOwner"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index fbba7fa7967..61c0ba4df3f 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -726,4 +726,7 @@ The element type of result of '{dbFunction}' is mapped to '{entityType}'. This is not supported since '{entityType}' is part of hierarchy and does not contain a discriminator property. + + Using 'FromSqlRaw' or 'FromSqlInterpolated' on an entity type which has owned reference navigations sharing same table is not supported. + \ No newline at end of file diff --git a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs index 14cf95dbe34..8eb732f92d0 100644 --- a/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs +++ b/src/EFCore.Relational/Query/RelationalQueryableMethodTranslatingExpressionVisitor.cs @@ -1347,6 +1347,7 @@ private Expression TryExpand(Expression source, MemberIdentity member) // So there is no handling for dependent having TPT // If navigation is defined on derived type and entity type is part of TPT then we need to get ITableBase for derived type. + // TODO: The following code should also handle Function and SqlQuery mappings var table = navigation.DeclaringEntityType.BaseType == null || entityType.GetDiscriminatorProperty() != null ? navigation.DeclaringEntityType.GetViewOrTableMappings().Single().Table @@ -1366,11 +1367,8 @@ private Expression TryExpand(Expression source, MemberIdentity member) || (entityType.GetDiscriminatorProperty() == null && navigation.DeclaringEntityType.IsStrictlyDerivedFrom(entityShaperExpression.EntityType)); - var propertyExpressions = identifyingColumn.Table is TableExpression innerTable - ? GetPropertyExpressionsFromTable( - targetEntityType, table, _selectExpression, innerTable, principalNullable) - // If the principal table is SelectExpression then we may need to populate inner projection - : GetPropertyExpressionsFromSubquery(targetEntityType, table, identifyingColumn, principalNullable); + var propertyExpressions = GetPropertyExpressionFromSameTable( + targetEntityType, table, _selectExpression, identifyingColumn, principalNullable); innerShaper = new RelationalEntityShaperExpression( targetEntityType, new EntityProjectionExpression(targetEntityType, propertyExpressions), true); @@ -1411,48 +1409,51 @@ private Expression TryExpand(Expression source, MemberIdentity member) return innerShaper; } - private static IDictionary GetPropertyExpressionsFromSubquery( - IEntityType entityType, ITableBase table, ColumnExpression identifyingColumn, bool nullable) + private static IDictionary GetPropertyExpressionFromSameTable( + IEntityType entityType, ITableBase table, SelectExpression selectExpression, ColumnExpression identifyingColumn, bool nullable) { - var subquery = (SelectExpression)identifyingColumn.Table; - var subqueryIdentifyingColumn = (ColumnExpression)subquery.Projection - .SingleOrDefault(e => string.Equals(e.Alias, identifyingColumn.Name, StringComparison.OrdinalIgnoreCase)).Expression; - - var subqueryPropertyExpressions = subqueryIdentifyingColumn.Table is TableExpression innerTable - ? GetPropertyExpressionsFromTable(entityType, table, subquery, innerTable, nullable) - : GetPropertyExpressionsFromSubquery(entityType, table, subqueryIdentifyingColumn, nullable); - - var newPropertyExpressions = new Dictionary(); - foreach (var item in subqueryPropertyExpressions) + if (identifyingColumn.Table is TableExpression tableExpression) { - newPropertyExpressions[item.Key] = new ColumnExpression( - subquery.Projection[subquery.AddToProjection(item.Value)], subquery); - } + if (!string.Equals(tableExpression.Name, table.Name, StringComparison.OrdinalIgnoreCase)) + { + // Fetch the table for the type which is defining the navigation since dependent would be in that table + tableExpression = selectExpression.Tables + .Select(t => (t as InnerJoinExpression)?.Table ?? (t as LeftJoinExpression)?.Table ?? t) + .Cast() + .First(t => string.Equals(t.Name, table.Name) && string.Equals(t.Schema, table.Schema)); + } - return newPropertyExpressions; - } + var propertyExpressions = new Dictionary(); + foreach (var property in entityType + .GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive()) + .SelectMany(EntityTypeExtensions.GetDeclaredProperties)) + { + propertyExpressions[property] = new ColumnExpression( + property, table.FindColumn(property), tableExpression, nullable || !property.IsPrimaryKey()); + } - private static IDictionary GetPropertyExpressionsFromTable( - IEntityType entityType, ITableBase table, SelectExpression selectExpression, TableExpression tableExpression, bool nullable) - { - if (!string.Equals(tableExpression.Name, table.Name, StringComparison.OrdinalIgnoreCase)) - { - // Fetch the table for the type which is defining the navigation since dependent would be in that table - tableExpression = selectExpression.Tables - .Select(t => (t as InnerJoinExpression)?.Table ?? (t as LeftJoinExpression)?.Table ?? t) - .Cast() - .First(t => string.Equals(t.Name, table.Name) && string.Equals(t.Schema, table.Schema)); + return propertyExpressions; } - var propertyExpressions = new Dictionary(); - foreach (var property in entityType - .GetAllBaseTypes().Concat(entityType.GetDerivedTypesInclusive()).SelectMany(EntityTypeExtensions.GetDeclaredProperties)) + if (identifyingColumn.Table is SelectExpression subquery) { - propertyExpressions[property] = new ColumnExpression( - property, table.FindColumn(property), tableExpression, nullable || !property.IsPrimaryKey()); + var subqueryIdentifyingColumn = (ColumnExpression)subquery.Projection + .SingleOrDefault(e => string.Equals(e.Alias, identifyingColumn.Name, StringComparison.OrdinalIgnoreCase)).Expression; + + var subqueryPropertyExpressions = GetPropertyExpressionFromSameTable( + entityType, table, subquery, subqueryIdentifyingColumn, nullable); + + var newPropertyExpressions = new Dictionary(); + foreach (var item in subqueryPropertyExpressions) + { + newPropertyExpressions[item.Key] = new ColumnExpression( + subquery.Projection[subquery.AddToProjection(item.Value)], subquery); + } + + return newPropertyExpressions; } - return propertyExpressions; + throw new InvalidOperationException(RelationalStrings.CustomQueryMappingOnOwner); } private static IDictionary GetPropertyExpressionsFromJoinedTable( diff --git a/test/EFCore.Relational.Specification.Tests/Query/OwnedQueryRelationalTestBase.cs b/test/EFCore.Relational.Specification.Tests/Query/OwnedQueryRelationalTestBase.cs index 55c7044d079..03a9a60317a 100644 --- a/test/EFCore.Relational.Specification.Tests/Query/OwnedQueryRelationalTestBase.cs +++ b/test/EFCore.Relational.Specification.Tests/Query/OwnedQueryRelationalTestBase.cs @@ -1,8 +1,10 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Linq; using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.TestUtilities; using Xunit; @@ -111,6 +113,25 @@ public virtual Task Can_query_on_indexer_properties_split(bool async) ss => ss.Set().Where(c => (string)c["Name"] == "Mona Cy").AsSplitQuery()); } + [ConditionalTheory] + [MemberData(nameof(IsAsyncData))] + public virtual async Task Throws_when_using_from_sql_on_owner(bool async) + { + using var context = CreateContext(); + var query = context.Set().FromSqlRaw(NormalizeDelimitersInRawString("SELECT * FROM [OwnedPersons]")); + + var message = async + ? (await Assert.ThrowsAsync(() => query.ToListAsync())).Message + : Assert.Throws(() => query.ToList()).Message; + Assert.Equal(RelationalStrings.CustomQueryMappingOnOwner, message); + } + + protected string NormalizeDelimitersInRawString(string sql) + => Fixture.TestStore.NormalizeDelimitersInRawString(sql); + + protected FormattableString NormalizeDelimitersInInterpolatedString(FormattableString sql) + => Fixture.TestStore.NormalizeDelimitersInInterpolatedString(sql); + protected virtual bool CanExecuteQueryString => false; protected override QueryAsserter CreateQueryAsserter(TFixture fixture) @@ -118,6 +139,8 @@ protected override QueryAsserter CreateQueryAsserter(TFixture fixture) public abstract class RelationalOwnedQueryFixture : OwnedQueryFixtureBase { + public new RelationalTestStore TestStore => (RelationalTestStore)base.TestStore; + public TestSqlLoggerFactory TestSqlLoggerFactory => (TestSqlLoggerFactory)ListLoggerFactory; protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext context)