Support for DbBatch #3461
Support for DbBatch #3461
Conversation
|
Just a thought for the discussion (do not need to implement straight away). The .NET Core since 3 supports default interface members. With it we can add methods to interfaces without introducing breaking changes (it is similar to adding a virtual method for the class in prior versions). Since the DbBatch is .NET 6 and above we could use default methods to extend interfaces instead of doing our usual shimming. This should simplify code a little bit. |
Yes! I wanted to use that but didn't realize the compiler directives actually meant that I could. Fixed. |
|
We could make it select the DbBatchBatcher automatically when possible, but perhaps that is something we should/could wait with. |
|
@hazzik Thanks for helping out! A couple of ideas.
|
Co-authored-by: Frédéric Delaporte <[email protected]>
Co-authored-by: Frédéric Delaporte <[email protected]>
| @@ -6,5 +7,9 @@ public interface IDriveConnectionCommandProvider | |||
| { | |||
| DbConnection CreateConnection(); | |||
| DbCommand CreateCommand(); | |||
| #if NET6_0_OR_GREATER | |||
| DbBatch CreateBatch() => throw new NotImplementedException(); | |||
There was a problem hiding this comment.
Throwing not supported exception probably would be better here
src/NHibernate/Driver/IDriver.cs
Outdated
| /// </summary> | ||
| /// <returns></returns> | ||
| /// <exception cref="NotImplementedException"></exception> | ||
| DbBatch CreateBatch() => throw new NotImplementedException(); |
There was a problem hiding this comment.
Same, I think not supported is better.
src/NHibernate/Driver/IDriver.cs
Outdated
| /// May be a no-op if the driver does not support preparing commands, or for any other reason. | ||
| /// </summary> | ||
| /// <param name="dbBatch">The batch.</param> | ||
| void PrepareBatch(DbBatch dbBatch) => throw new NotImplementedException(); |
src/NHibernate/Driver/IDriver.cs
Outdated
| /// <param name="dbCommand"></param> | ||
| /// <returns></returns> | ||
| /// <exception cref="NotImplementedException"></exception> | ||
| DbBatchCommand CreateDbBatchCommandFromDbCommand(DbBatch dbBatch, DbCommand dbCommand) => throw new NotImplementedException(); |
There was a problem hiding this comment.
Better to throw not supported exception here.
Observations
A DbBatch looks like a DbCommand, but just on the surface.
Unlike batching which uses an underlying
DbCommand,DbBatchwill require special handling for things such asPrepare,Enlistetc. While theDbBatchcould theoretically be wrapped in aDbCommand"adapter" or even a NH specific "DbCommandOrBatch", sinceDbBatchis now a part of the core .NET 6+ API, I think the route forward is to be explicit about it, e.g. having bothEnlist(DbCommand)andEnlist(DbBatch).This means that a transparent move is harder, since custom derivatives of e.g MicrosoftDataSqlClientDriver may have vital implementations of e.g
AdjustCommandthat will have to be duplicated forDbBatch.No really good way to create a DbBatchCommand from a DbCommand
If I'm not missing something, I think this is a bit of an oversight on the part of the
DbBatchcreators, especially since they already use "CanDoXxxx" properties.All the parameters in a
DbCommandwill have to be recreated for theDbBatchCommand, since they can't be attached to twoDbParameterCollectionsat the same time. The good ol'SqlCommandSetuses a bit of trickery for that, but I just opted to make sure that theDbParameterisICloneable, and used that. Perhaps the value should be explicitly cloned too, if it'sICloneable. Making it possible to override this logic is probably a good idea.ReflectionBasedDriver doesn't use DbProviderFactoryDriveConnectionCommandProvider on .NET 5 and above
Any reason for this, or just an effect of NETSTANDARD2_1_OR_GREATER not including NET5 etc.?