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

Net 5 Core Scaffold for SQL Server Calculated Fields doesn't emit ValueGeneratedOnAddOrUpdate() #22545

Closed
scott452 opened this issue Sep 15, 2020 · 27 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@scott452
Copy link

Using Net Core RC 1 CLI the command:
dotnet ef dbcontext scaffold Name=ConnectV3Connection Microsoft.EntityFrameworkCore.SqlServer

It doesn't emit ValueGeneratedOnAddOrUpdate() for SQL Server calculated fields in the generated OnModelCreating method, nor does it generate an attribute if using the --data-annotations parameter.

For a quick fix I created a partial class for the Context and used the partial OnModelCreatingPartial(ModelBuilder builder) method and added the code:

entity.Property(e => e.ExtendedName).ValueGeneratedOnAddOrUpdate();

This situation was true for Preview 7 & 8 as well.

Thanks,

Scott

@ajcvickers
Copy link
Member

@roji Is it possible this was broken at the same time as the other reverse engineering breaks?

@smitpatel
Copy link
Member

HasComputedColumnSql round trips correctly and is scaffolded correctly.

@ajcvickers
Copy link
Member

@scott452 Are you seeing a functional impact of this? What is not working without this configuration?

@smitpatel
Copy link
Member

RowVersion also round-trips correctly.

@scott452
Copy link
Author

@scott452 Are you seeing a functional impact of this? What is not working without this configuration?

Yes, without my workaround, I get an error the EF Core code try's to set the value of the field, then SQL server throws an error, saying that the field is a calculated value.

@smitpatel
Copy link
Member

Share you database design script so we can investigate.

@scott452
Copy link
Author

Will do, here is the error as logged:

2020-09-15 14:30:46 [Error] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand (46ms) [Parameters=[@p0='a20a0232-205a-491a-a7bb-347b9ac70ec1', @p1='2020-09-15T00:00:00.0000000-04:00' (DbType = DateTime), @p2=NULL (DbType = DateTime), @p3='Scott' (Nullable = false) (Size = 10), @p4=NULL (Nullable = false) (Size = 62), @p5='shlamb2' (Nullable = false) (Size = 15), @p6='Test' (Nullable = false) (Size = 50), @p7='2f287db4-a4ef-41c7-91e2-e48723b65fc3' (Nullable = true)], CommandType='"Text"', CommandTimeout='30'] SET NOCOUNT ON; INSERT INTO [Department] ([DepartmentId], [ActiveFrom], [ActiveTo], [DepartmentCode], [ExtendedName], [ModifiedBy], [Name], [SupervisorId]) VALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7); SELECT [ClusterKey], [Timestamp] FROM [Department] WHERE @@ROWCOUNT = 1 AND [DepartmentId] = @p0; 2020-09-15 14:30:46 [Error] [Microsoft.EntityFrameworkCore.Update] An exception occurred in the database while saving changes for context type 'Connect.Repository.Models.V3.ConnectContext'. Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> Microsoft.Data.SqlClient.SqlException (0x80131904): The column "ExtendedName" cannot be modified because it is either a computed column or is the result of a UNION operator. at Microsoft.Data.SqlClient.SqlCommand.<>c.<ExecuteDbDataReaderAsync>b__169_0(Task1 result)
at System.Threading.Tasks.ContinuationResultTaskFromResultTask2.InnerInvoke() at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location --- at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) --- End of stack trace from previous location --- at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) ClientConnectionId:cb55960d-5208-426a-9d51-b409080ff282 Error Number:271,State:1,Class:16 --- End of inner exception stack trace --- at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList1 entriesToSave, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(DbContext _, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func4 operation, Func4 verifySucceeded, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> Microsoft.Data.SqlClient.SqlException (0x80131904): The column "ExtendedName" cannot be modified because it is either a computed column or is the result of a UNION operator. at Microsoft.Data.SqlClient.SqlCommand.<>c.<ExecuteDbDataReaderAsync>b__169_0(Task1 result)
at System.Threading.Tasks.ContinuationResultTaskFromResultTask2.InnerInvoke() at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location --- at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread) --- End of stack trace from previous location --- at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) ClientConnectionId:cb55960d-5208-426a-9d51-b409080ff282 Error Number:271,State:1,Class:16 --- End of inner exception stack trace --- at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList1 entriesToSave, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(DbContext _, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func4 operation, Func4 verifySucceeded, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

@scott452
Copy link
Author

I have attached 2 database scripts:
Database Scripts.zip

Azure-ConnectDb-Department-Table.sql
Is a scripts a single table that shows the issue

Azure-ConnectDB.sql
Is the entire database, it has at least three tables with the issue, Project, Department, and Project

Tables that have the issue all have calculated fields
Database Scripts.zip

Thanks

Scott

@smitpatel
Copy link
Member

@scott452 - I ran the script to create Department table and scaffolded it. I don't see any missing ValueGeneratedOnAddOrUpdate.
Which property is erroneous?

Unrelated

  • [ExtendedName] AS (([DepartmentCode]+': ')+[Name]) PERSISTED NOT NULL, generated
                entity.Property(e => e.ExtendedName)
                    .IsRequired()
                    .HasMaxLength(62)
                    .HasComputedColumnSql("(([DepartmentCode]+': ')+[Name])", true);

Not sure why max length.

  • We are still generating this
                entity.Property(e => e.ActiveTo)
                    .HasColumnType("datetime")
                    .HasAnnotation("Relational:ColumnType", "datetime");

Using nightly builds.

@smitpatel
Copy link
Member

@ajcvickers - 2nd issue above is one line fix. Should we do it in rc2?

@ajcvickers
Copy link
Member

@smitpatel Is this a regression from 3.1?

@ajcvickers
Copy link
Member

Also, it's interesting that the column here is "PERSISTED". I assume we don't reverse engineer that facet (yet)? @roji?

@scott452
Copy link
Author

In the Department Table the ExtendedName offending property, it's the one that fires the errors and is fixed by my adding the ValueGeneratedOnAddOrUpdate for the the partial OnModelCreatingPartial(ModelBuilder builder)

@smitpatel
Copy link
Member

We actually reverse engineer persisted hence 2nd argument of HasComputedColumnSql is true.

@smitpatel
Copy link
Member

@smitpatel Is this a regression from 3.1?

Yes.

@ajcvickers
Copy link
Member

Then yes, we should prepare the PR for rc2.

@smitpatel
Copy link
Member

@scott452 - The scaffolded code mirrors what database says for value generation and does not look faulty. Can you share how your app is breaking after that scaffolded code?

@scott452
Copy link
Author

@scott452 - The scaffolded code mirrors what database says for value generation and does not look faulty. Can you share how your app is breaking after that scaffolded code?

Sure,

To test the Department Table and Classes, I run my project with two cases.

First Case, with this code included in the partial class for the dbcontext and within the partial OnModelCreatingPartial(ModelBuilder builder) method

builder.Entity<Department>(entity => { entity.Property(e => e.ExtendedName).ValueGeneratedOnAddOrUpdate(); });

I can create a new Department, no errors

Second Case, with that code commented out I get the error that I reported earlier, namely
The column "ExtendedName" cannot be modified because it is either a computed column or is the result of a UNION operator

The code that adds the record is exactly the same in both cases:

                using (var context = ConnectContextFactory.CreateDbContext())
                {
                    using (var transaction = await context.Database.BeginTransactionAsync())
                    {
                        try
                        {
                            Department department;
                            if (args.Data.DepartmentId == Guid.Empty)
                            {
                                // this is the new record case
                                args.Data.DepartmentId = Guid.NewGuid();
                                department = new Department();

                                department.DepartmentId = args.Data.DepartmentId;
                                department.ActiveFrom = args.Data.ActiveFrom;
                                department.ActiveTo = args.Data.ActiveTo;
                                department.DepartmentCode = args.Data.DepartmentCode;
                                department.ModifiedBy = UserId;
                                department.Name = args.Data.Name;
                                department.SupervisorId = args.Data.SupervisorId;

                                context.Departments.Add(department);
                            }
                            else
                            {
                                // it exists fetch the db record
                                department = await context.Departments
                                        .FindAsync(args.Data.DepartmentId);
                                department.ActiveFrom = args.Data.ActiveFrom;
                                department.ActiveTo = args.Data.ActiveTo;
                                department.DepartmentCode = args.Data.DepartmentCode;
                                department.ModifiedBy = UserId;
                                department.Name = args.Data.Name;
                                department.SupervisorId = args.Data.SupervisorId;
                            }

                            await context.SaveChangesAsync();
                            await SyncService.SyncDepartmentAsync(department);
                            await transaction.CommitAsync();

                            var supervisor = await context.Employees
                                .AsNoTracking()
                                .FirstAsync(r => r.EmployeeId == args.Data.SupervisorId);

                            args.Data.Supervisor = supervisor.ExtendedName;
                            args.Data.SupervisorLinkblueId = supervisor.LinkblueId;
                        }
                        catch (SyncException ex)
                        {
                            Logger.LogError("Admin Sync Failed", ex);
                            await transaction.RollbackAsync();
                            await MessageDialog.ShowModal("Database Error", "Admin Sync Failed");
                            args.Cancel = true;
                        }
                        catch (Exception ex)
                        {
                            Logger.LogError("Grid data operation failed", ex);
                            await transaction.RollbackAsync();
                            await MessageDialog.ShowModal("Database Exception", "Database operation failed.");
                            args.Cancel = true;
                        }

                    }
                }

@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 16, 2020

I am not sure this a regression.

EF Core 3.1.6:

                entity.Property(e => e.ExtendedName)
                    .IsRequired()
                    .HasMaxLength(62)
                    .HasComputedColumnSql("(([DepartmentCode]+': ')+[Name])");

EF Core 5 RC1:

                entity.Property(e => e.ExtendedName)
                    .IsRequired()
                    .HasMaxLength(62)
                    .HasComputedColumnSql("(([DepartmentCode]+': ')+[Name])", true);

@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 16, 2020

I am actually not even able to repro, when using simpler code than above - this works fine with EF 5 RC1:

            using var db = new NorthwindContext();

            db.Departments.Add(new Department
            {
                ActiveFrom = DateTime.Now,
                ActiveTo = DateTime.Now,
                ClusterKey = 0,
                DepartmentCode = Guid.NewGuid().ToString().Substring(0, 4),
                DepartmentId = Guid.NewGuid(),
                ModifiedBy = "erik",
                Name = Guid.NewGuid().ToString(),
            });

            db.SaveChanges();

@scott452
Copy link
Author

I was creating a simpler project to try and reproduce my problem. I created a local database with a simple table. The table has a computed field. When I scaffolded the table it generated

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<Department>(entity =>
        {
            entity.Property(e => e.DepartmentId).ValueGeneratedNever();

            entity.Property(e => e.ExtendedName).HasComputedColumnSql("(([DepartmentCode]+': ')+[DepartmentName])", true);
        });

        OnModelCreatingPartial(modelBuilder);
    }

This has the HasComputedColumnSql call, the context in my problematic solution did not generate this call, I re-scaffoled the database in my problem child once again, and again it never emitted the call to HasComputedColumnSql for the property. I am stumped, I have checked the both solutions/projects all are using the latest version of the EF packages RC 1.

I don't know what is going on.

I haven't run the test project yet, but I will bet it will work since it has the call to HasComputedColumnSql

Scott

@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 16, 2020

I don't know what is going on.

Multiple databases with differing schema?

@scott452
Copy link
Author

The one that works is local the other in azure, I will check permissions and whatnot.

@scott452
Copy link
Author

Argh, it is a permissions issue, when I scaffold as DBOwner the HasComputedColumnSql call is emitted.

@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 16, 2020

So nothing is broken now?

@scott452
Copy link
Author

Nope, just my pride.

@smitpatel smitpatel added the closed-no-further-action The issue is closed and no further action is planned. label Sep 16, 2020
@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 27, 2020

@scott452 the user running reverse engineer must have VIEW DEFINITION rights in the database

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

4 participants