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

ADO.NET batching API #54467

Merged
merged 6 commits into from
Jul 4, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
58 changes: 58 additions & 0 deletions src/libraries/System.Data.Common/ref/System.Data.Common.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,56 @@ public void RemoveAt(string sourceTable) { }
System.Data.ITableMapping System.Data.ITableMappingCollection.Add(string sourceTableName, string dataSetTableName) { throw null; }
System.Data.ITableMapping System.Data.ITableMappingCollection.GetByDataSetTable(string dataSetTableName) { throw null; }
}
public abstract class DbBatch : System.IDisposable, System.IAsyncDisposable
{
public System.Data.Common.DbBatchCommandCollection BatchCommands { get { throw null; } }
protected abstract System.Data.Common.DbBatchCommandCollection DbBatchCommands { get; }
public abstract int Timeout { get; set; }
public System.Data.Common.DbConnection? Connection { get; set; }
protected abstract System.Data.Common.DbConnection? DbConnection { get; set; }
public System.Data.Common.DbTransaction? Transaction { get; set; }
protected abstract System.Data.Common.DbTransaction? DbTransaction { get; set; }
public System.Data.Common.DbDataReader ExecuteReader() { throw null; }
protected abstract System.Data.Common.DbDataReader ExecuteDbDataReader();
public System.Threading.Tasks.Task<System.Data.Common.DbDataReader> ExecuteReaderAsync(System.Threading.CancellationToken cancellationToken = default) { throw null; }
protected abstract System.Threading.Tasks.Task<System.Data.Common.DbDataReader> ExecuteDbDataReaderAsync(System.Threading.CancellationToken cancellationToken);
public abstract int ExecuteNonQuery();
public abstract System.Threading.Tasks.Task<int> ExecuteNonQueryAsync(System.Threading.CancellationToken cancellationToken = default);
public abstract object? ExecuteScalar();
public abstract System.Threading.Tasks.Task<object?> ExecuteScalarAsync(System.Threading.CancellationToken cancellationToken = default);
public abstract void Prepare();
public abstract System.Threading.Tasks.Task PrepareAsync(System.Threading.CancellationToken cancellationToken = default);
public abstract void Cancel();
public System.Data.Common.DbBatchCommand CreateBatchCommand() { throw null; }
protected virtual System.Data.Common.DbBatchCommand CreateDbBatchCommand() { throw null; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected virtual System.Data.Common.DbBatchCommand CreateDbBatchCommand() { throw null; }
protected abstract System.Data.Common.DbBatchCommand CreateDbBatchCommand() { throw null; }

Shouldn't it be required for derived classes to implement this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch here too. CreateDbBatch on DbConnection and DbProviderFactory are virtual (and throw NotSupportedException by default) since we can't add abstract methods to existing types without breaking. But DbBatch is a new type, and it makes no sense to define an implementation of it which doesn't support creating a DbBatchCommand.

public virtual void Dispose() { throw null; }
public virtual System.Threading.Tasks.ValueTask DisposeAsync() { throw null; }
}
public abstract class DbBatchCommand
{
public abstract string CommandText { get; set; }
public abstract CommandType CommandType { get; set; }
public abstract CommandBehavior CommandBehavior { get; set; }
public abstract int RecordsAffected { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Public setter seems odd; should this be protected set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, there definitely shouldn't be a public setter. Though rather than making the setter protected I've removed it entirely, to not impose things on implementations (this is also how DbDataReader.RecordsAffected is defined.

public DbParameterCollection Parameters { get { throw null; } }
protected abstract DbParameterCollection DbParameterCollection { get; }
}
public abstract class DbBatchCommandCollection : System.Collections.Generic.IList<DbBatchCommand>
{
public abstract System.Collections.Generic.IEnumerator<System.Data.Common.DbBatchCommand> GetEnumerator();
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
public abstract void Add(System.Data.Common.DbBatchCommand item);
public abstract void Clear();
public abstract bool Contains(System.Data.Common.DbBatchCommand item);
public abstract void CopyTo(System.Data.Common.DbBatchCommand[] array, int arrayIndex);
public abstract bool Remove(System.Data.Common.DbBatchCommand item);
public abstract int Count { get; }
public abstract bool IsReadOnly { get; }
public abstract int IndexOf(DbBatchCommand item);
public abstract void Insert(int index, DbBatchCommand item);
public abstract void RemoveAt(int index);
public abstract DbBatchCommand this[int index] { get; set; }
}
public abstract partial class DbColumn
{
protected DbColumn() { }
Expand Down Expand Up @@ -2077,6 +2127,9 @@ public virtual event System.Data.StateChangeEventHandler? StateChange { add { }
public virtual System.Threading.Tasks.Task ChangeDatabaseAsync(string databaseName, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
public abstract void Close();
public virtual System.Threading.Tasks.Task CloseAsync() { throw null; }
public virtual bool CanCreateBatch { get { throw null; } }
public System.Data.Common.DbBatch CreateBatch() { throw null; }
protected virtual System.Data.Common.DbBatch CreateDbBatch() { throw null; }
public System.Data.Common.DbCommand CreateCommand() { throw null; }
protected abstract System.Data.Common.DbCommand CreateDbCommand();
public virtual System.Threading.Tasks.ValueTask DisposeAsync() { throw null; }
Expand Down Expand Up @@ -2367,6 +2420,8 @@ protected DbException(string? message, System.Exception? innerException) { }
protected DbException(string? message, int errorCode) { }
public virtual bool IsTransient { get { throw null; } }
public virtual string? SqlState { get { throw null; } }
public System.Data.Common.DbBatchCommand? BatchCommand { get { throw null; } }
public virtual System.Data.Common.DbBatchCommand? DbBatchCommand { get { throw null; } }
}
public static partial class DbMetaDataCollectionNames
{
Expand Down Expand Up @@ -2528,9 +2583,12 @@ public static void RegisterFactory(string providerInvariantName, [System.Diagnos
public abstract partial class DbProviderFactory
{
protected DbProviderFactory() { }
public virtual bool CanCreateBatch { get { throw null; } }
public virtual bool CanCreateCommandBuilder { get { throw null; } }
public virtual bool CanCreateDataAdapter { get { throw null; } }
public virtual bool CanCreateDataSourceEnumerator { get { throw null; } }
public virtual System.Data.Common.DbBatch CreateBatch() { throw null; }
public virtual System.Data.Common.DbBatchCommand CreateBatchCommand() { throw null; }
public virtual System.Data.Common.DbCommand? CreateCommand() { throw null; }
public virtual System.Data.Common.DbCommandBuilder? CreateCommandBuilder() { throw null; }
public virtual System.Data.Common.DbConnection? CreateConnection() { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@
<Compile Include="System\Data\Common\DataTableMappingCollection.cs" />
<Compile Include="System\Data\Common\DateTimeOffsetStorage.cs" />
<Compile Include="System\Data\Common\DateTimeStorage.cs" />
<Compile Include="System\Data\Common\DbBatch.cs" />
<Compile Include="System\Data\Common\DbBatchCommand.cs" />
<Compile Include="System\Data\Common\DbBatchCommandCollection.cs" />
<Compile Include="System\Data\Common\DbColumn.cs" />
<Compile Include="System\Data\Common\DbCommand.cs">
<SubType>Component</SubType>
Expand Down
61 changes: 61 additions & 0 deletions src/libraries/System.Data.Common/src/System/Data/Common/DbBatch.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Threading;
using System.Threading.Tasks;

namespace System.Data.Common
{
public abstract class DbBatch : IDisposable, IAsyncDisposable
{
public DbBatchCommandCollection BatchCommands => DbBatchCommands;

protected abstract DbBatchCommandCollection DbBatchCommands { get; }

public abstract int Timeout { get; set; }

public DbConnection? Connection { get; set; }
roji marked this conversation as resolved.
Show resolved Hide resolved

protected abstract DbConnection? DbConnection { get; set; }

public DbTransaction? Transaction { get; set; }
roji marked this conversation as resolved.
Show resolved Hide resolved

protected abstract DbTransaction? DbTransaction { get; set; }

public DbDataReader ExecuteReader()
=> ExecuteDbDataReader();

protected abstract DbDataReader ExecuteDbDataReader();

public Task<DbDataReader> ExecuteReaderAsync(CancellationToken cancellationToken = default)
=> ExecuteDbDataReaderAsync(cancellationToken);

protected abstract Task<DbDataReader> ExecuteDbDataReaderAsync(CancellationToken cancellationToken);

public abstract int ExecuteNonQuery();

public abstract Task<int> ExecuteNonQueryAsync(CancellationToken cancellationToken = default);

public abstract object? ExecuteScalar();

public abstract Task<object?> ExecuteScalarAsync(CancellationToken cancellationToken = default);

public abstract void Prepare();

public abstract Task PrepareAsync(CancellationToken cancellationToken = default);

public abstract void Cancel();

public DbBatchCommand CreateBatchCommand() => CreateDbBatchCommand();

protected virtual DbBatchCommand CreateDbBatchCommand() => throw new NotSupportedException();

public virtual void Dispose() {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stephentoub can you please confirm that this (and the below DisposeAsync) is acceptable in an abstract base class, given there are no resources to be disposed on it, and that the dispose pattern could simply be implemented by implementations if needed?

Also, I've made this non-abstract since most implementations aren't expected to override.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little surprised to not see the standard dispose pattern implemented here, although I do expect the inheritance hierarchy to be very shallow (a single derived class per provider, and it should be sealed), thus I can't imagine it being a problem in practice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bgrainger missed this originally.

I think the standard dispose pattern makes sense when the base class already have some disposable resource; DbCommand/DbConnection extend Component (DbBatch doesn't), which itself is disposable (mainly for event management, AFAIK), which is why the pattern is followed there. For an empty base class with no disposable resources, I'm not sure what the pattern would bring - it can always be added in implementing classes, if those classes have a disposable resource and expect to be themselves inherited...

Let me know if you see any issue with this logic.


public virtual ValueTask DisposeAsync()
{
Dispose();
return default;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.ComponentModel;
using System.Diagnostics.CodeAnalysis;

namespace System.Data.Common
{
public abstract class DbBatchCommand
{
public abstract string CommandText { get; set; }

public abstract CommandType CommandType { get; set; }

public abstract CommandBehavior CommandBehavior { get; set; }

public abstract int RecordsAffected { get; set; }

public DbParameterCollection Parameters => DbParameterCollection;

protected abstract DbParameterCollection DbParameterCollection { get; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Collections.Generic;

namespace System.Data.Common
{
public abstract class DbBatchCommandCollection : IList<DbBatchCommand>
{
public abstract IEnumerator<DbBatchCommand> GetEnumerator();

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

public abstract void Add(DbBatchCommand item);

public abstract void Clear();

public abstract bool Contains(DbBatchCommand item);

public abstract void CopyTo(DbBatchCommand[] array, int arrayIndex);

public abstract bool Remove(DbBatchCommand item);

public abstract int Count { get; }

public abstract bool IsReadOnly { get; }

public abstract int IndexOf(DbBatchCommand item);

public abstract void Insert(int index, DbBatchCommand item);

public abstract void RemoveAt(int index);

public abstract DbBatchCommand this[int index] { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ public virtual Task ChangeDatabaseAsync(string databaseName, CancellationToken c
}
}

public virtual bool CanCreateBatch => false;

public DbBatch CreateBatch() => CreateDbBatch();

protected virtual DbBatch CreateDbBatch() => throw new NotSupportedException();

public DbCommand CreateCommand() => CreateDbCommand();

IDbCommand IDbConnection.CreateCommand() => CreateDbCommand();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,9 @@ protected DbException(System.Runtime.Serialization.SerializationInfo info, Syste
/// A standard SQL 5-character return code, or <see langword="null" />.
/// </returns>
public virtual string? SqlState => null;

public DbBatchCommand? BatchCommand => DbBatchCommand;

public virtual DbBatchCommand? DbBatchCommand { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ public abstract partial class DbProviderFactory

protected DbProviderFactory() { }

public virtual bool CanCreateBatch => false;

public virtual bool CanCreateDataSourceEnumerator => false;

public virtual bool CanCreateDataAdapter
Expand Down Expand Up @@ -47,6 +49,10 @@ public virtual bool CanCreateCommandBuilder
}
}

public virtual DbBatch CreateBatch() => throw new NotSupportedException();

public virtual DbBatchCommand CreateBatchCommand() => throw new NotSupportedException();

public virtual DbCommand? CreateCommand() => null;

public virtual DbCommandBuilder? CreateCommandBuilder() => null;
Expand Down