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.

Sign up for GitHub

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

Jump to bottom

Make DbUpdateException.Entries contain all entries when we can't populate it #29571

Merged
1 commit merged into from
Nov 17, 2022
Merged

Make DbUpdateException.Entries contain all entries when we can't populate it #29571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 82 additions & 53 deletions src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand Down
35 changes: 35 additions & 0 deletions test/EFCore.Relational.Specification.Tests/Update/NonSharedModelUpdatesTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,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,
Expand Down
7 changes: 1 addition & 6 deletions test/EFCore.SqlServer.FunctionalTests/SqlServerValueGenerationScenariosTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
61 changes: 61 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/Update/NonSharedModelUpdatesSqlServerTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,67 @@ public class DailyDigest
public User User { get; set; }
}

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);

Expand Down
30 changes: 30 additions & 0 deletions test/EFCore.Sqlite.FunctionalTests/Update/NonSharedModelUpdatesSqliteTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down