Issue with Computed Fields using User Defined Functions

[skimedic]

This is the same issue that was described in #5793, but was never resolved because of a lack of code samples. I've attached code that illustrates the issues. If it's too much code, I can make a smaller sample, but it should be pretty clear. There are actually two issues that I am running into, don't know if they are related or if the other one should be a new issue.

1. When an entity/table a computed column based on a scalar function, EF can't update the record. I have an Order table and an OrderDetail table. I have a scalar function that computes the total cost of an order (summing all of the order detail records) to display on the Order table. Here are the abbreviated details:

Property definition:

public class Order : EntityBase 
{ 
    public int CustomerId { get; set; } 
    public string CategoryName {get;set;} 

    [DataType(DataType.Currency)]
    [DatabaseGenerated(DatabaseGeneratedOption.Computed)] 
    public decimal? OrderTotalComputed { get; set; } //shortened for brevity }
}

In my context:

OnModelCreating:

modelBuilder.Entity(entity =>
{
    entity.Property(e => e.OrderTotalComputed)
        .HasColumnType("money")
        .HasComputedColumnSql("Store.GetOrderTotal([Id])");
        //.ValueGeneratedOnAddOrUpdate();
});

When I first create an order record, it works fine. If I update an existing record I get the following error:
Result StackTrace:

 at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable1 commandBatches, IRelationalConnection connection) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList1 entriesToSave)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
at SpyStore.DAL.Tests.Context.OrderTests.ShouldUpdateAnOrder() in C:\GitHub\Responsive\CodeSamples\Chapter05-EF\RTM\SpyStore.DAL\test\SpyStore.DAL.Tests\Context\OrderTests.cs:line 42
at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action1 wrapCloseInAction) at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose) at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady) at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData() at System.Data.SqlClient.SqlDataReader.get_MetaData() at System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString) at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, SqlDataReader ds) at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior) at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, String executeMethod, IReadOnlyDictionary2 parameterValues, Boolean openConnection, Boolean closeConnection)
at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteReader(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues, Boolean manageConnection)
at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)

Result Message:
An error occurred while updating the entries. See the inner exception for details.
Column 'inserted.OrderTotalComputed' cannot be referenced in the OUTPUT clause because the column definition contains a subquery or references a function that performs user or system data access. A function is assumed by default to perform data access if it is not schemabound. Consider removing the subquery or function from the column definition or removing the column from the OUTPUT clause.

In the attached code, execute the SpyStore.DAL.Tests.Context.OrderTests.ShouldUpdateAnOrder test to see the error in action.

2. The second issue is that when I have such a computed column, if I add/update order detail records in a DbContext, the computed column doesn't update in my entity. However, if I create a new Context, I get the correct value when retrieving the record. The new DbContext test is illustrated with this test: SpyStore.DAL.Tests.Context.OrderTests.ShouldGetOrderTotal (this test uses a new DbContext instead of the one that seeded the data)
   The failing test (where the value doesn't get updated in the same DbContext) is here: SpyStore.DAL.Tests.Context.OrderTests.ShouldGetOrderTotalAfterAddingAnOrderDetail

EF Core version: RTM (1.0.0.0)
Operating system: Win10
Visual Studio version: 2015

SpyStore.DAL.zip

[ajcvickers]

Note for triage: this seems to be an issue with the update pipeline.

@AndriySvyryd I'm going to send out a PR containing a failing test.

[ajcvickers]

@skimedic The second issue you list is actually by design. EF does identity resolution when running queries such that if an entity with the same ID is already being tracked by the context, then it is not re-created or updated by the query. There is an item on our backlog (#1203) to implement a Reload method that would allow the values in the entity to be refreshed based on the current values in the database.

[skimedic]

@ajcvickers ok, thanks for the info. I have a simple workaround for the second issue, so I'm not that concerned about it. Just wanted bring it up for completeness. I will eagerly wait for the reload to bubble to the top of the backlog :-)

[rowanmiller]

@AndriySvyryd when we understand this more we can make the 1.1 vs 1.0.1 call

[ajcvickers]

@AndriySvyryd One thing that came up in triage is whether or not this works if batching is switched off?

[AndriySvyryd]

@skimedic You would need to add WITH SCHEMABINDING to the function definition. This also can improve the performance of the function. However if the referenced objects are altered the function would need to be recreated.

This would still be the case without batching and in EF6 as far as I can tell.

[rowanmiller]

@AndriySvyryd this means no action on the EF side right?

[AndriySvyryd]

@rowanmiller Yes, unless we want to mention this in the docs

[skimedic]

ok, I will try that and report back - sorry, just saw this.

[skimedic]

@AndriySvyryd I don't see how this fixes anything. I updated my UDF to this:

ALTER FUNCTION [Store].[GetOrderTotal] ( @OrderId INT ) RETURNS MONEY WITH SCHEMABINDING BEGIN DECLARE @Result MONEY; SELECT @Result = SUM([Quantity]*[UnitCost]) FROM Store.OrderDetails WHERE OrderId = @OrderId; RETURN @Result END;
I'm still getting this exception on SaveChanges:

Test Name: SpyStore.DAL.Tests.Context.OrderTests.ShouldUpdateAnOrder
Test FullName: SpyStore.DAL.Tests.Context.OrderTests.ShouldUpdateAnOrder
Test Source: C:\GitHub\Responsive\CodeSamples\Chapter05-EF\RTM\SpyStore.DAL\test\SpyStore.DAL.Tests\Context\OrderTests.cs : line 39
Test Outcome: Failed
Test Duration: 0:00:02.143

Result StackTrace:

at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at SpyStore.DAL.Tests.Context.OrderTests.ShouldUpdateAnOrder() in C:\GitHub\Responsive\CodeSamples\Chapter05-EF\RTM\SpyStore.DAL\test\SpyStore.DAL.Tests\Context\OrderTests.cs:line 42
   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at System.Data.SqlClient.SqlDataReader.get_MetaData()
   at System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString)
   at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, SqlDataReader ds)
   at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, String executeMethod, IReadOnlyDictionary`2 parameterValues, Boolean openConnection, Boolean closeConnection)
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteReader(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues, Boolean manageConnection)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
Result Message: 
An error occurred while updating the entries. See the inner exception for details.
Column 'inserted.OrderTotalComputed' cannot be referenced in the OUTPUT clause because the column definition contains a subquery or references a function that performs user or system data access. A function is assumed by default to perform data access if it is not schemabound. Consider removing the subquery or function from the column definition or removing the column from the OUTPUT clause.

Here is the SQL captured through SQL Server Profiler:

exec sp_executesql N'SET NOCOUNT ON;
DECLARE @inserted0 TABLE ([OrderTotalComputed] money, [TimeStamp] varbinary(8));
UPDATE [Store].[Orders] SET [ShipDate] = @p0
OUTPUT INSERTED.[OrderTotalComputed], INSERTED.[TimeStamp]
INTO @inserted0
WHERE [Id] = @p1 AND [TimeStamp] = @p2;
SELECT [OrderTotalComputed], [TimeStamp] FROM @inserted0;
',N'@p1 int,@p0 datetime,@p2 varbinary(8)',@p1=2,@p0='2016-07-24 16:44:14.777',@p2=0x00000000000117AB

Please let me know what I am missing is with schemabinding fixes this problem.

[skimedic]

I don't believe this should have been closed. @rowanmiller did you get this working somewhere?

[rowanmiller]

Reopening to discuss in triage based on comments from @skimedic

[skimedic]

SpyStore.DAL.zip
Here is my updated code, the udf is using with schemabinding (I also change the udf to not reference the computed column in the order details table in case that was adding to the issue). Same test is failing as before.

[skimedic]

please let me know if I can help in any additional way to diagnose this. ty.

[AndriySvyryd]

My bad, WITH SCHEMABINDING only helps if the UDF doesn't contain a query. To support this we could add a way to mark the column as not compatible with OUTPUT and query it separately from the update statement.

As a workaround you can configure the property as ValueGeneratedNever() and reload the entity when you need to access the computed column.

_db.Orders.Where(o => o.Id == order.Id).Select(o => o.OrderTotalComputed).First()