Skip to content

Commit

Permalink
Query: Disallow FromSql on owner when owned navigation is mapped to s…
Browse files Browse the repository at this point in the history
…ame table (#21778)

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
  • Loading branch information
smitpatel committed Jul 25, 2020
1 parent 2256528 commit bc9dc1b
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 38 deletions.

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

3 changes: 3 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -726,4 +726,7 @@
<data name="TableValuedFunctionNonTPH" xml:space="preserve">
<value>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.</value>
</data>
<data name="CustomQueryMappingOnOwner" xml:space="preserve">
<value>Using 'FromSqlRaw' or 'FromSqlInterpolated' on an entity type which has owned reference navigations sharing same table is not supported.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down Expand Up @@ -1411,48 +1409,51 @@ private Expression TryExpand(Expression source, MemberIdentity member)
return innerShaper;
}

private static IDictionary<IProperty, ColumnExpression> GetPropertyExpressionsFromSubquery(
IEntityType entityType, ITableBase table, ColumnExpression identifyingColumn, bool nullable)
private static IDictionary<IProperty, ColumnExpression> 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<IProperty, ColumnExpression>();
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<TableExpression>()
.First(t => string.Equals(t.Name, table.Name) && string.Equals(t.Schema, table.Schema));
}

return newPropertyExpressions;
}
var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();
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<IProperty, ColumnExpression> 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<TableExpression>()
.First(t => string.Equals(t.Name, table.Name) && string.Equals(t.Schema, table.Schema));
return propertyExpressions;
}

var propertyExpressions = new Dictionary<IProperty, ColumnExpression>();
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<IProperty, ColumnExpression>();
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<IProperty, ColumnExpression> GetPropertyExpressionsFromJoinedTable(
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -111,13 +113,34 @@ public virtual Task Can_query_on_indexer_properties_split(bool async)
ss => ss.Set<OwnedPerson>().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<OwnedPerson>().FromSqlRaw(NormalizeDelimitersInRawString("SELECT * FROM [OwnedPersons]"));

var message = async
? (await Assert.ThrowsAsync<InvalidOperationException>(() => query.ToListAsync())).Message
: Assert.Throws<InvalidOperationException>(() => 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)
=> new RelationalQueryAsserter(fixture, RewriteExpectedQueryExpression, RewriteServerQueryExpression, canExecuteQueryString: CanExecuteQueryString);

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)
Expand Down

0 comments on commit bc9dc1b

Please sign in to comment.