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.79k stars 3.19k forks source link

Computed columns are ignored if they are also a foreign key #18401

Closed paillave closed 2 years ago

paillave commented 5 years ago

Explanation

It happens that we want to make some global computations in a view that we link to a table using the primary key. The problem is that the automatic characteristic of the primary key is ignored if it is a foreign key as well. This case seems to be crazy as if a primary key is also a foreign key means that the two tables should actually be one. But one a a table, another one is a view.

Steps to reproduce

Position.cs

using System.Collections.Generic;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace TestBlabla
{
    public class Position
    {
        public int Id { get; set; }
        public int PortfolioCompositionId { get; set; }
        public PositionComputation Computation { get; set; }
        public string ReferenceCode { get; set; }
    }
    public class PositionConfiguration : IEntityTypeConfiguration<Position>
    {
        public void Configure(EntityTypeBuilder<Position> builder)
        {
            builder.ToTable("Position");
            builder.HasOne(i => i.Computation).WithMany().HasForeignKey(i => i.Id);
            builder.HasKey(i => i.Id);
            builder.Property(i => i.Id).UseIdentityColumn();
            // Lot of irrelevant setup for the problem
        }
    }
}

PositionComputation.cs

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

namespace TestBlabla
{
    public class PositionComputation
    {
        public int PositionId { get; set; }
        public decimal? Weight { get; set; }
    }
    public class PositionComputationConfiguration : IEntityTypeConfiguration<PositionComputation>
    {
        public void Configure(EntityTypeBuilder<PositionComputation> builder)
        {
            // THIS IS NOT A TABLE. 
            // THIS IS A VIEW THAT CONTAINS COMPUTATION BASED ON OTHER TABLES
            // IF THIS WAS A TABLE, THAT WOULD NOT MAKE SENSE AS I WOULD PUT ALL ITS CONTENT IN THE `POSITION` TABLE
            builder.ToView("PositionComputation");
            builder.HasKey(i => i.PositionId);
        }
    }
}

DatabaseContext.cs

public class DatabaseContext : DbContext
{
        public DatabaseContext(string connectionString) : base(new DbContextOptionsBuilder<DatabaseContext>().UseSqlServer(connectionString).Options)
        {
        }
        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.ApplyConfigurationsFromAssembly(typeof(DatabaseContext).Assembly);
        }
}

PositionComputation.sql

create or alter view [dbo].[PositionComputation]
as
    with portfolioCompositionComputation as 
    (
        select 
            pc.Id as PortfolioCompositionId, 
            sum(p.MarketValueInPortfolioCcy) as TotalMarketValueInPortfolioCcy
        from 
            Pms.PortfolioComposition as pc
            inner join Pms.Position as p on p.PortfolioCompositionId = pc.Id
        group by
            pc.Id
    )
    select 
        p.Id as PositionId,
        p.MarketValueInPortfolioCcy / pcc.TotalMarketValueInPortfolioCcy as Weight
    from 
        portfolioCompositionComputation as pcc
        inner join Pms.Position as p on p.PortfolioCompositionId = pcc.PortfolioCompositionId;

what is above permits to accomplish this following code that actually works:

using(var dbContext=new DatabaseContext(myConnectionString))
{
    // get positions with computations based on the content of other tables with grouping and so on...
    var positionsAndComputation = dbContext
        .Set<Position>()
        .Include(i=>i.Computation)
        .Where(i=>listOfIds.Contains(i.Id))
        .ToList();
    foreach(var positionAndComputation in positionsAndComputation)
        Console.WriteLine($"Position {positionAndComputation.ReferenceCode} has the weight {positionAndComputation.Computation.Weight}");
}

but this code that I also need to work fails :boom:

using(var dbContext=new DatabaseContext(myConnectionString))
{
    // unfortunately, if I can retrieve position and their related computations, I can't insert any.
    var posi=new Position
        {
            PortfolioCompositionId = 1,
            ReferenceCode = "UserGivenReferenceCode"
        };
        dbContext.Add(posi);
        dbContext.SaveChanges(); // The exception is raised here
}

raised exception:

Exception thrown: 'Microsoft.EntityFrameworkCore.DbUpdateException' in Microsoft.EntityFrameworkCore.dll: 'An error occurred while updating the entries. See the inner exception for details.'
 Inner exceptions found, see $exception in variables window for more details.
 Innermost exception     Microsoft.Data.SqlClient.SqlException : Cannot insert explicit value for identity column in table 'Position' when IDENTITY_INSERT is set to OFF.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at Microsoft.Data.SqlClient.SqlDataReader.get_MetaData()
   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteDbDataReader(CommandBehavior behavior)
   at System.Data.Common.DbCommand.ExecuteReader()
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReader(RelationalCommandParameterObject parameterObject)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)

other useful information:

var entityType = dbContext.Model.FindEntityType(typeof(Position));
// the following gets "Never" instead of "OnAdd"
var valueGenerated = entityType.FindProperty(nameof(Position.Id)).ValueGenerated;

What is interesting is that the migration is correctly generated.

Further technical details

EF Core version: Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET Core 3.0 Operating system: ubuntu 19.04 IDE: VS Code

ajcvickers commented 5 years ago

@AndriySvyryd If I am reading this correctly, Position.Id is both a PK and is a foreign key constrained to the PositionComputation.PositionId PK. So this is a PK to PK relationship, which means it must be 1:1. However, the relationship is configured using WithMany. Isn't this invalid?

AndriySvyryd commented 5 years ago

@ajcvickers It's not invalid, but it is misleading, since in effect it will be a 1:1 relationship

@paillave We don't allow FK properties to be store-generated as that means that the store would create relationships between entities and this is generally not a safe pattern. But I don't understand why would you even need the value to be store generated for your case. Could you provide a runnable project that shows the issue?

paillave commented 5 years ago

As I mentioned in the initial post, this case seems to be crazy as a primary that key that is also a foreign key means that the two entities should be merged. I'm mentally healthy, so I would do that :), but in my case one entity is a table, another one is a view that contains computed columns based on other tables.

As I said, the point of all this is to have computed columns based on computations that rely on complicated joins with sub queries and so on. Something EF can't do out of the box. So I put it in a view that is referenced by the matching entity.

Everything you need to understand the pb is in the code I gave you. I'll submit an actual "working" code later.

ajcvickers commented 5 years ago

@paillave I also don't understand what you're trying to do here. Why are you forcing the table to have a foreign key referencing the view? Why not just configure the view to have a foreign key referencing the table? Something like this:

builder.HasOne(i => i.Computation).WithOne().HasForeignKey<PositionComputation>(i => i.PositionId);
paillave commented 5 years ago

@ajcvickers this is what I did as a workaround. But as I mentionned: I want to have extra computed properties on my entity. These computations come from a view that makes reference to many tables under the hood.

If I take the usual example of the invoice (for a simple illustration): I want to get the list of invoices... plus a property that contains the computed total of the invoice based on other tables like invoiceline. To formulate differently: I want to get the list of invoices, with, per each, in a dedicated property or sub property, the total of the invoice that is provided by a view.

ajcvickers commented 5 years ago

@paillave We have discussed this again, but we still don't understand exactly what it is you are trying to do or why. Maybe with more complete code we might be able to understand, but right now there isn't anything actionable on our side since we're not understanding what it is you want.

paillave commented 5 years ago

I added more code and completed what is missing. Now, nearly nothing is missing anymore.

I'll try to explain again: I need computed columns based on columns that are in other tables... only a view can provide this to me. This view return strictly one row per row of the table Position. This row contains only computed columns (Weight), plus the column PositionId that permits to know to what occurrence of the table Position these computed values refer to.

I want to get an entity... plus related computed values based on what is other tables. For the entity, I get the entity position based on the table position... and for computed values... I get the entity positionComputation based on the view positionComputation. Now I want entity framework to make something like select p.*, pc.* from position as p left join positionComputation as pc on p.id = pc.id; pc.* represents computed columns based on what could be found in other table, and p.* represents non computed columns.

So far so good, everything works, but when I try to add a row in the table position, I can't because I get the exception due to the fact that the primary key is considered as not computed. What is strange is that the fact the primary key is computed is not ignored by the migration system. So let's imagine the provider works as expected, still, there is an inconsistency with the migration system that has no pb with a computed primary key that is also a foreign key.

ajcvickers commented 4 years ago

@smitpatel to take a look.

smitpatel commented 4 years ago

If you configure the model following way


    public class PositionConfiguration : IEntityTypeConfiguration<Position>
    {
        public void Configure(EntityTypeBuilder<Position> builder)
        {
            builder.ToTable("Position");
            //builder.HasOne(i => i.Computation).WithMany().HasForeignKey(i => i.Id);
            builder.HasKey(i => i.Id);
            builder.Property(i => i.Id).UseIdentityColumn();
            // Lot of irrelevant setup for the problem
        }
    }

    public class PositionComputationConfiguration : IEntityTypeConfiguration<PositionComputation>
    {
        public void Configure(EntityTypeBuilder<PositionComputation> builder)
        {
            // THIS IS NOT A TABLE.
            // THIS IS A VIEW THAT CONTAINS COMPUTATION BASED ON OTHER TABLES
            // IF THIS WAS A TABLE, THAT WOULD NOT MAKE SENSE AS I WOULD PUT ALL ITS CONTENT IN THE `POSITION` TABLE
            builder.ToView("PositionComputation");
            builder.HasOne<Position>().WithOne(e => e.Computation).HasForeignKey<PositionComputation>(e => e.PositionId);
            builder.HasKey(i => i.PositionId);
        }
    }

Then insert works fine and generated SQL is like this as you expect

      SELECT [p].[Id], [p].[PortfolioCompositionId], [p].[ReferenceCode], [p0].[PositionId], [p0].[Weight]
      FROM [Position] AS [p]
      LEFT JOIN [PositionComputation] AS [p0] ON [p].[Id] = [p0].[PositionId]
      WHERE [p].[Id] IN (1, 2, 3)

But as I mentionned: I want to have extra computed properties on my entity.

But in your code the additional computed property is on PositionComputation which is not the main entity. Your main entity Position. Any additional entities which computes extra properties should be dependents since you would insert into Position table (others are view). Hence you need to configure your view entity as dependent via one-to-one mapping. And you will have computed properties always available through navigation.