Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Make DbUpdateException.Entries empty when we can't populate it
Browse files Browse the repository at this point in the history
roji committed Nov 16, 2022
1 parent 3dc1fdf commit c56d6be
Showing 6 changed files with 238 additions and 62 deletions.
135 changes: 82 additions & 53 deletions src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs
Original file line number Diff line number Diff line change
@@ -270,43 +270,58 @@ await ThrowAggregateUpdateConcurrencyExceptionAsync(
/// <returns>The ordinal of the next result set that must be consumed.</returns>
protected virtual int ConsumeResultSet(int startCommandIndex, RelationalDataReader reader)
{
var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
IReadOnlyModificationCommand? command = null;

try
{
if (!reader.Read())

var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
{
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
if (!reader.Read())
{
expectedRowsAffected++;
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
{
expectedRowsAffected++;
}

ThrowAggregateUpdateConcurrencyException(reader, commandIndex, expectedRowsAffected, rowsAffected);
}
else
{
var resultSetMapping = ResultSetMappings[commandIndex];

ThrowAggregateUpdateConcurrencyException(reader, commandIndex, expectedRowsAffected, rowsAffected);
}
else
{
var resultSetMapping = ResultSetMappings[commandIndex];
command = ModificationCommands[
resultSetMapping.HasFlag(ResultSetMapping.IsPositionalResultMappingEnabled)
? startCommandIndex + reader.DbDataReader.GetInt32(reader.DbDataReader.FieldCount - 1)
: commandIndex];

var command = ModificationCommands[
resultSetMapping.HasFlag(ResultSetMapping.IsPositionalResultMappingEnabled)
? startCommandIndex + reader.DbDataReader.GetInt32(reader.DbDataReader.FieldCount - 1)
: commandIndex];
Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");

command.PropagateResults(reader);

Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");
command = null;
}

command.PropagateResults(reader);
rowsAffected++;
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

rowsAffected++;
return commandIndex - 1;
}
catch (Exception ex) when (ex is not DbUpdateException and not OperationCanceledException)
{
throw new DbUpdateException(
RelationalStrings.UpdateStoreException,
ex,
command?.Entries ?? ModificationCommands.SelectMany(c => c.Entries).ToList());
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

return commandIndex - 1;
}

/// <summary>
@@ -326,44 +341,58 @@ protected virtual async Task<int> ConsumeResultSetAsync(
RelationalDataReader reader,
CancellationToken cancellationToken)
{
var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
IReadOnlyModificationCommand? command = null;

try
{
if (!await reader.ReadAsync(cancellationToken).ConfigureAwait(false))
var commandIndex = startCommandIndex;
var rowsAffected = 0;
do
{
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
if (!await reader.ReadAsync(cancellationToken).ConfigureAwait(false))
{
expectedRowsAffected++;
var expectedRowsAffected = rowsAffected + 1;
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet))
{
expectedRowsAffected++;
}

await ThrowAggregateUpdateConcurrencyExceptionAsync(
reader, commandIndex, expectedRowsAffected, rowsAffected, cancellationToken).ConfigureAwait(false);
}
else
{
var resultSetMapping = ResultSetMappings[commandIndex];

await ThrowAggregateUpdateConcurrencyExceptionAsync(
reader, commandIndex, expectedRowsAffected, rowsAffected, cancellationToken).ConfigureAwait(false);
}
else
{
var resultSetMapping = ResultSetMappings[commandIndex];
command = ModificationCommands[
resultSetMapping.HasFlag(ResultSetMapping.IsPositionalResultMappingEnabled)
? startCommandIndex + reader.DbDataReader.GetInt32(reader.DbDataReader.FieldCount - 1)
: commandIndex];

var command = ModificationCommands[
resultSetMapping.HasFlag(ResultSetMapping.IsPositionalResultMappingEnabled)
? startCommandIndex + reader.DbDataReader.GetInt32(reader.DbDataReader.FieldCount - 1)
: commandIndex];
Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");

command.PropagateResults(reader);

Check.DebugAssert(
!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly),
"!resultSetMapping.HasFlag(ResultSetMapping.ResultSetWithRowsAffectedOnly)");
command = null;
}

command.PropagateResults(reader);
rowsAffected++;
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

rowsAffected++;
return commandIndex - 1;
}
catch (Exception ex) when (ex is not DbUpdateException and not OperationCanceledException)
{
throw new DbUpdateException(
RelationalStrings.UpdateStoreException,
ex,
command?.Entries ?? ModificationCommands.SelectMany(c => c.Entries).ToList());
}
while (++commandIndex < ResultSetMappings.Count
&& ResultSetMappings[commandIndex - 1].HasFlag(ResultSetMapping.NotLastInResultSet));

return commandIndex - 1;
}

/// <summary>
Original file line number Diff line number Diff line change
@@ -12,8 +12,7 @@ protected override string StoreName
=> "NonSharedModelUpdatesTestBase";

[ConditionalTheory] // Issue #29356
[InlineData(false)]
[InlineData(true)]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async)
{
var contextFactory = await InitializeAsync<DbContext>(
@@ -111,6 +110,41 @@ private class Book
public Author? Author { get; set; }
}

[ConditionalTheory] // Issue #29379
[MemberData(nameof(IsAsyncData))]
public virtual async Task DbUpdateException_Entries_is_correct_with_multiple_inserts(bool async)
{
var contextFactory = await InitializeAsync<DbContext>(onModelCreating: mb => mb.Entity<Blog>().HasIndex(b => b.Name).IsUnique());

await ExecuteWithStrategyInTransactionAsync(
contextFactory,
async context =>
{
context.Add(new Blog { Name = "Blog2" });
await context.SaveChangesAsync();
},
async context =>
{
context.Add(new Blog { Name = "Blog1" });
context.Add(new Blog { Name = "Blog2" });
context.Add(new Blog { Name = "Blog3" });

var exception = async
? await Assert.ThrowsAsync<DbUpdateException>(() => context.SaveChangesAsync())
: Assert.Throws<DbUpdateException>(() => context.SaveChanges());

var entry = Assert.Single(exception.Entries);

Assert.Equal("Blog2", ((Blog)entry.Entity).Name);
});
}

public class Blog
{
public int Id { get; set; }
public string? Name { get; set; }
}

protected virtual void ExecuteWithStrategyInTransaction(
ContextFactory<DbContext> contextFactory,
Action<DbContext> testOperation,
Original file line number Diff line number Diff line change
@@ -457,6 +457,34 @@ public async Task OperationCanceledException_is_not_wrapped_with_DbUpdateExcepti
Assert.Same(originalException, actualException);
}

[ConditionalTheory]
[InlineData(false)]
[InlineData(true)]
public async Task DbUpdateException(bool async)
{
var entry = CreateEntry(EntityState.Added, generateKeyValues: true);

var command = CreateModificationCommand(entry, new ParameterNameGenerator().GenerateNext, true, null);
command.AddEntry(entry, true);

var originalException = new FakeDbException();

var connection = CreateConnection(
new FakeCommandExecutor(
executeReaderAsync: (c, b, ct) => throw originalException,
executeReader: (c, b) => throw originalException));

var batch = new ModificationCommandBatchFake();
batch.TryAddCommand(command);
batch.Complete(moreBatchesExpected: false);

var actualException = async
? await Assert.ThrowsAsync<DbUpdateException>(() => batch.ExecuteAsync(connection))
: Assert.Throws<DbUpdateException>(() => batch.Execute(connection));

Assert.Same(originalException, actualException.InnerException);
}

[ConditionalFact]
public void CreateStoreCommand_creates_parameters_for_each_ModificationCommand()
{
Original file line number Diff line number Diff line change
@@ -1234,12 +1234,7 @@ public void Insert_with_explicit_non_default_keys_by_default()
// inner exception for details.
// SqlException : Cannot insert explicit value for identity column in table
// 'Blog' when IDENTITY_INSERT is set to OFF.
context.Database.CreateExecutionStrategy().Execute(
context, c =>
{
var updateException = Assert.Throws<DbUpdateException>(() => c.SaveChanges());
Assert.Single(updateException.Entries);
});
context.Database.CreateExecutionStrategy().Execute(context, c => Assert.Throws<DbUpdateException>(() => c.SaveChanges()));
}

[ConditionalFact]
Original file line number Diff line number Diff line change
@@ -84,6 +84,66 @@ OUTPUT 1
""");
}

public override async Task DbUpdateException_Entries_is_correct_with_multiple_inserts(bool async)
{
// SQL Server's bulk insert support makes it impossible to populate the entry which caused the exception, since the position
// used to find the entry is returned as an output column, but the row is never received in case of an exception.
// Instead we make sure Entries contains all entries.
var contextFactory = await InitializeAsync<DbContext>(onModelCreating: mb => mb.Entity<Blog>().HasIndex(b => b.Name).IsUnique());

await ExecuteWithStrategyInTransactionAsync(
contextFactory,
async context =>
{
context.Add(new Blog { Name = "Blog2" });
await context.SaveChangesAsync();
},
async context =>
{
context.Add(new Blog { Name = "Blog1" });
context.Add(new Blog { Name = "Blog2" });
context.Add(new Blog { Name = "Blog3" });

var exception = async
? await Assert.ThrowsAsync<DbUpdateException>(() => context.SaveChangesAsync())
: Assert.Throws<DbUpdateException>(() => context.SaveChanges());

Assert.Collection(
exception.Entries.Select(e => (Blog)e.Entity).OrderBy(b => b.Name),
b => Assert.Equal("Blog1", b.Name),
b => Assert.Equal("Blog2", b.Name),
b => Assert.Equal("Blog3", b.Name));
});

AssertSql(
"""
@p0='Blog2' (Size = 450)
SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
INSERT INTO [Blog] ([Name])
OUTPUT INSERTED.[Id]
VALUES (@p0);
""",
//
"""
@p0='Blog1' (Size = 450)
@p1='Blog2' (Size = 450)
@p2='Blog3' (Size = 450)
SET IMPLICIT_TRANSACTIONS OFF;
SET NOCOUNT ON;
MERGE [Blog] USING (
VALUES (@p0, 0),
(@p1, 1),
(@p2, 2)) AS i ([Name], _Position) ON 1=0
WHEN NOT MATCHED THEN
INSERT ([Name])
VALUES (i.[Name])
OUTPUT INSERTED.[Id], i._Position;
""");
}

private void AssertSql(params string[] expected)
=> TestSqlLoggerFactory.AssertBaseline(expected);

Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@

namespace Microsoft.EntityFrameworkCore.Update;

public class NonSharedModelUpdatesSqlServerTest : NonSharedModelUpdatesTestBase
public class NonSharedModelUpdatesSqliteTest : NonSharedModelUpdatesTestBase
{
public override async Task Principal_and_dependent_roundtrips_with_cycle_breaking(bool async)
{
@@ -78,6 +78,36 @@ DELETE FROM "Author"
""");
}

public override async Task DbUpdateException_Entries_is_correct_with_multiple_inserts(bool async)
{
await base.DbUpdateException_Entries_is_correct_with_multiple_inserts(async);

AssertSql(
"""
@p0='Blog2' (Size = 5)
INSERT INTO "Blog" ("Name")
VALUES (@p0)
RETURNING "Id";
""",
//
"""
@p0='Blog1' (Size = 5)
INSERT INTO "Blog" ("Name")
VALUES (@p0)
RETURNING "Id";
""",
//
"""
@p0='Blog2' (Size = 5)
INSERT INTO "Blog" ("Name")
VALUES (@p0)
RETURNING "Id";
""");
}

private void AssertSql(params string[] expected)
=> TestSqlLoggerFactory.AssertBaseline(expected);

0 comments on commit c56d6be

Please sign in to comment.