Skip to content

Commit

Permalink
API review items (#28709)
Browse files Browse the repository at this point in the history
* Append{Update,Delete}Command now accept a higher-level flag for
  adding a 1 constant to the RETURNING clause.
* Accept MaxBatchSize in the ReadModificationCommandBatch constructor

Part of #27588
  • Loading branch information
roji authored Aug 15, 2022
1 parent ec3d80a commit 3291ff5
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 37 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Data;

namespace Microsoft.EntityFrameworkCore.Update;

/// <summary>
Expand All @@ -23,8 +21,9 @@ public abstract class AffectedCountModificationCommandBatch : ReaderModification
/// Creates a new <see cref="AffectedCountModificationCommandBatch" /> instance.
/// </summary>
/// <param name="dependencies">Service dependencies.</param>
protected AffectedCountModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies)
: base(dependencies)
/// <param name="maxBatchSize">The maximum batch size. Defaults to 1000.</param>
protected AffectedCountModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies, int? maxBatchSize = null)
: base(dependencies, maxBatchSize)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ public abstract class ReaderModificationCommandBatch : ModificationCommandBatch
/// Creates a new <see cref="ReaderModificationCommandBatch" /> instance.
/// </summary>
/// <param name="dependencies">Service dependencies.</param>
protected ReaderModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies)
/// <param name="maxBatchSize">The maximum batch size. Defaults to 1000.</param>
protected ReaderModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies, int? maxBatchSize = null)
{
Dependencies = dependencies;

Expand All @@ -42,6 +43,8 @@ protected ReaderModificationCommandBatch(ModificationCommandBatchFactoryDependen
UpdateSqlGenerator = dependencies.UpdateSqlGenerator;
UpdateSqlGenerator.AppendBatchHeader(SqlBuilder);
_batchHeaderLength = SqlBuilder.Length;

MaxBatchSize = maxBatchSize ?? 1000;
}

/// <summary>
Expand All @@ -62,8 +65,7 @@ protected ReaderModificationCommandBatch(ModificationCommandBatchFactoryDependen
/// <summary>
/// The maximum number of <see cref="ModificationCommand"/> instances that can be added to a single batch.
/// </summary>
protected virtual int MaxBatchSize
=> 1000;
protected virtual int MaxBatchSize { get; }

/// <summary>
/// Gets the command text builder for the commands in the batch.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,7 @@ public class SingularModificationCommandBatch : AffectedCountModificationCommand
/// </summary>
/// <param name="dependencies">Service dependencies.</param>
public SingularModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies)
: base(dependencies)
: base(dependencies, maxBatchSize: 1)
{
}

/// <summary>
/// The maximum number of <see cref="ModificationCommand"/> instances that can be added to a single batch; always returns 1.
/// </summary>
protected override int MaxBatchSize
=> 1;
}
18 changes: 9 additions & 9 deletions src/EFCore.Relational/Update/UpdateSqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ protected virtual ResultSetMapping AppendUpdateReturningOperation(

AppendUpdateCommand(
commandStringBuilder, name, schema, writeOperations, readOperations, conditionOperations,
additionalReadValues: readOperations.Count == 0 ? "1" : null);
appendReturningOneClause: readOperations.Count == 0);

return ResultSetMapping.LastInResultSet;
}
Expand Down Expand Up @@ -175,7 +175,7 @@ protected virtual ResultSetMapping AppendDeleteReturningOperation(
requiresTransaction = false;

AppendDeleteCommand(
commandStringBuilder, name, schema, Array.Empty<IColumnModification>(), conditionOperations, additionalReadValues: "1");
commandStringBuilder, name, schema, Array.Empty<IColumnModification>(), conditionOperations, appendReturningOneClause: true);

return ResultSetMapping.LastInResultSet;
}
Expand Down Expand Up @@ -211,19 +211,19 @@ protected virtual void AppendInsertCommand(
/// <param name="writeOperations">The operations for each column.</param>
/// <param name="readOperations">The operations for column values to be read back.</param>
/// <param name="conditionOperations">The operations used to generate the <c>WHERE</c> clause for the update.</param>
/// <param name="additionalReadValues">Additional values to be read back.</param>
/// <param name="appendReturningOneClause">Whether to append an additional constant of 1 to be read back.</param>
protected virtual void AppendUpdateCommand(
StringBuilder commandStringBuilder,
string name,
string? schema,
IReadOnlyList<IColumnModification> writeOperations,
IReadOnlyList<IColumnModification> readOperations,
IReadOnlyList<IColumnModification> conditionOperations,
string? additionalReadValues = null)
bool appendReturningOneClause = false)
{
AppendUpdateCommandHeader(commandStringBuilder, name, schema, writeOperations);
AppendWhereClause(commandStringBuilder, conditionOperations);
AppendReturningClause(commandStringBuilder, readOperations, additionalReadValues);
AppendReturningClause(commandStringBuilder, readOperations, appendReturningOneClause ? "1" : null);
commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator);
}

Expand All @@ -235,18 +235,18 @@ protected virtual void AppendUpdateCommand(
/// <param name="schema">The table schema, or <see langword="null" /> to use the default schema.</param>
/// <param name="readOperations">The operations for column values to be read back.</param>
/// <param name="conditionOperations">The operations used to generate the <c>WHERE</c> clause for the delete.</param>
/// <param name="additionalReadValues">Additional values to be read back.</param>
/// <param name="appendReturningOneClause">Whether to append an additional constant of 1 to be read back.</param>
protected virtual void AppendDeleteCommand(
StringBuilder commandStringBuilder,
string name,
string? schema,
IReadOnlyList<IColumnModification> readOperations,
IReadOnlyList<IColumnModification> conditionOperations,
string? additionalReadValues = null)
bool appendReturningOneClause = false)
{
AppendDeleteCommandHeader(commandStringBuilder, name, schema);
AppendWhereClause(commandStringBuilder, conditionOperations);
AppendReturningClause(commandStringBuilder, readOperations, additionalReadValues);
AppendReturningClause(commandStringBuilder, readOperations, appendReturningOneClause ? "1" : null);
commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator);
}

Expand Down Expand Up @@ -478,7 +478,7 @@ protected virtual void AppendReturningClause(
commandStringBuilder.Append(", ");
}

commandStringBuilder.Append(additionalValues);
commandStringBuilder.Append("1");
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ public class SqlServerModificationCommandBatch : AffectedCountModificationComman
public SqlServerModificationCommandBatch(
ModificationCommandBatchFactoryDependencies dependencies,
int maxBatchSize)
: base(dependencies)
=> MaxBatchSize = maxBatchSize;
: base(dependencies, maxBatchSize)
{
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand All @@ -41,14 +42,6 @@ public SqlServerModificationCommandBatch(
protected new virtual ISqlServerUpdateSqlGenerator UpdateSqlGenerator
=> (ISqlServerUpdateSqlGenerator)base.UpdateSqlGenerator;

/// <summary>
/// The maximum number of <see cref="ModificationCommand"/> instances that can be added to a single batch.
/// </summary>
/// <remarks>
/// For SQL Server, this is 42 by default, and cannot exceed 1000.
/// </remarks>
protected override int MaxBatchSize { get; }

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,11 @@ protected override void AppendUpdateCommand(
IReadOnlyList<IColumnModification> writeOperations,
IReadOnlyList<IColumnModification> readOperations,
IReadOnlyList<IColumnModification> conditionOperations,
string? additionalReadValues = null)
bool appendReturningOneClause = false)
{
// In SQL Server the OUTPUT clause is placed differently (before the WHERE instead of at the end)
AppendUpdateCommandHeader(commandStringBuilder, name, schema, writeOperations);
AppendOutputClause(commandStringBuilder, readOperations, additionalReadValues);
AppendOutputClause(commandStringBuilder, readOperations, appendReturningOneClause ? "1" : null);
AppendWhereClause(commandStringBuilder, conditionOperations);
commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator);
}
Expand Down Expand Up @@ -166,11 +166,11 @@ protected override void AppendDeleteCommand(
string? schema,
IReadOnlyList<IColumnModification> readOperations,
IReadOnlyList<IColumnModification> conditionOperations,
string? additionalReadValues = null)
bool appendReturningOneClause = false)
{
// In SQL Server the OUTPUT clause is placed differently (before the WHERE instead of at the end)
AppendDeleteCommandHeader(commandStringBuilder, name, schema);
AppendOutputClause(commandStringBuilder, readOperations, additionalReadValues);
AppendOutputClause(commandStringBuilder, readOperations, appendReturningOneClause ? "1" : null);
AppendWhereClause(commandStringBuilder, conditionOperations);
commandStringBuilder.AppendLine(SqlGenerationHelper.StatementTerminator);
}
Expand Down

0 comments on commit 3291ff5

Please sign in to comment.