Log a warning when "rowid" is a column on the table

[zlepper]

Include your code

I'm currently working on migrating some old code to dotnet core. As part of that we have to work with an existing database until all the code can be changed (Which is probably still several years away). In an attempt to improve the code quality i'm trying to add some unit tests, however inserts into the table is failing. That code has this table:

CREATE TABLE "item_metafield_value" (
          "item_metafield_valueid" INTEGER NOT NULL CONSTRAINT "PK_item_metafield_value" PRIMARY KEY AUTOINCREMENT,
          "item_metafield_labelid" INTEGER NOT NULL,
          "itemid" INTEGER NOT NULL,
          "rowid" INTEGER NULL,
          "item_combo_valueid" INTEGER NULL,
          "item_tree_valueid" INTEGER NULL,
          "ref_itemid" INTEGER NULL,
          "value" TEXT NULL,
          "expires" datetime NULL,
          "DateModified" datetime NOT NULL,
          "smaleValue" TEXT NULL,
          "extravalue" TEXT NULL,
          "valueInt" INTEGER NULL
      );

Though in essense this table is enough to cause issues:

CREATE TABLE "item_metafield_value" (
          "item_metafield_valueid" INTEGER NOT NULL CONSTRAINT "PK_item_metafield_value" PRIMARY KEY AUTOINCREMENT,
          "rowid" INTEGER NULL,
      );

When we insert values we often insert rowid = null. like this (Generated by entity framework):

Executed DbCommand (1ms) [Parameters=[@p0='0001-01-01T00:00:00.0000000' (DbType = String), @p1=NULL, @p2=NULL, @p3=NULL, @p4='1' (DbType = String), @p5=NULL, @p6='961' (DbType = String), @p7=NULL, @p8=NULL, @p9=NULL, @p10='Hello World!' (Size = 12), @p11=NULL], CommandType='Text', CommandTimeout='30']
      INSERT INTO "item_metafield_value" ("DateModified", "expires", "extravalue", "item_combo_valueid", "item_metafield_labelid", "item_tree_valueid", "itemid", "ref_itemid", "rowid", "smaleValue", "value", "valueInt")
      VALUES (@p0, @p1, @p2, @p3, @p4, @p5, @p6, @p7, @p8, @p9, @p10, @p11);
      SELECT "item_metafield_valueid"
      FROM "item_metafield_value"
      WHERE changes() = 1 AND "rowid" = last_insert_rowid();

The problem then comes from the last select, which is now querrying on the wrong column. So EF doesn't believe it actually inserted anything. A "solution" would be to use _rowid_, (https://sqlite.org/lang_createtable.html#rowid), which is believe would have less chance of conflicting, and would result in the same.

Include stack traces

Microsoft.EntityFrameworkCore.DbUpdateConcurrencyException : Database operation expected to affect 1 row(s) but actually affected 0 row(s). Data may have been modified or deleted since entities were loaded. See http://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions.
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ThrowAggregateUpdateConcurrencyException(Int32 commandIndex, Int32 expectedRowsAffected, Int32 rowsAffected)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeResultSetWithPropagationAsync(Int32 commandIndex, RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeAsync(RelationalDataReader reader, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(DbContext _, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at LegacyService.Tests.Services.MetadataEditorLoaderServiceTests.LoadsStringValues() in C:\dev\digizuite.core\Apps\Tests\LegacyService.Tests\Services\MetadataEditorLoaderServiceTests.cs:line 99
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.GetResult()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0()
   at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action)

Include provider and version information

EF Core version: 5.0.0
Database provider: Microsoft.EntityFrameworkCore.Sqlite
Target framework: .net 5.0
Operating system: Windows 10
IDE: JetBrains Rider 2020.3.2

Workaround

For now i have added the following workaround so i can continue to write tests, even if the code doesn't mock the full database exactly anymore:

        partial void OnModelCreatingPartial(ModelBuilder modelBuilder)
        {
            if (!Database.IsSqlServer())
            {
                foreach (var entityType in modelBuilder.Model.GetEntityTypes())
                {
                    var rowIdProperty = entityType.GetProperties()
                        .FirstOrDefault(p => p.Name.Equals("rowid", StringComparison.InvariantCultureIgnoreCase));

                    rowIdProperty?.SetColumnName("dgz_rowid");
                }
            }
         }

[bricelam]

I'm not sure we can fix this. SQLite lets you completely hide access to the internal rowid value if you create a table that has a rowid, oid, and _rowid_ column. The best we could do is to use the least common name for it, but even that won't fully fix the issue.

sqlite> create table X(rowid, oid, _rowid_);
sqlite> insert into X values (2, 2, 2);
sqlite> select rowid, oid, _rowid_ from X;
2|2|2

[zlepper]

Personally I'm fine with considering this just a minor inconvenience, i have a workaround, but maybe log a warning when the name rowid is used with the SQLite provider, so people can know to look for a workaround? This took me quite a while to actually identify originally.

[bricelam]

Just realized that #24835 would mitigate this

[bricelam]

Not needed after PR #27573

[roji]

Oh cool, didn't even mean to fix this...