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

Missing functionality in batch API #2222

Closed
mgravell opened this issue Nov 16, 2023 · 3 comments · Fixed by #2223
Closed

Missing functionality in batch API #2222

mgravell opened this issue Nov 16, 2023 · 3 comments · Fixed by #2223
Labels
🆕 Public API Use this label for new API additions to the driver.

Comments

@mgravell
Copy link
Member

mgravell commented Nov 16, 2023

Relates to #1825

Testing local build (main) with Dapper shows a few problems, i.e. "it doesn't work"; severity: IMO critical, this should absolutely be fixed before next build; the batch API is not usable by non-provider scenarios without it

  1. SqlConnection fails to override the CanCreateBatch and CreateDbBatch() members (net6+)
  2. SqlBatchCommand does override the CanCreateParameter and CreateParameter() members (net8+), but: the library does not currently build for net8, so: this code is never actually compiled as far as I can see

Suggestions:

for SqlConnection:

#if NET6_0_OR_GREATER
        /// <inheritdoc/>
        public override bool CanCreateBatch => true;

        /// <inheritdoc/>
        protected override SqlBatch CreateDbBatch() => new SqlBatch(this);
#endif

(you may have local rules on docs, especially since we're doing type variance here)


in the csproj:

  • add net8.0 to <TargetFrameworks>

tests:

for net6+

using var cnn = new SqlConnection();
Assert.True(cnn.CanCreateBatch);
var batch = cnn.CreateBatch();
Assert.Is<SqlBatch>(batch);
Assert.Same(cnn, batch.Connection);
Assert.Null(batch.Transaction);

for net8+

var cmd = new SqlBatchCommand();
Assert.True(cnn.CanCreateParameter);
Assert.Is<SqlParameter>(cnn.CreateParameter());
@mgravell
Copy link
Member Author

mgravell commented Nov 16, 2023

note that adding net8 triggers a few [Obsolete] warnings, CS0672; requires

#if NET8_0_OR_GREATER
        [Obsolete]
#endif

on

  • SqlException.GetObjectData
  • SqlException.ctor(SerializationInfo, StreamingContext)
  • OperationAbortedException.ctor(SerializationInfo, StreamingContext)

I also get a build error from Microsoft.Data.SqlClient.notsupported.cs via GenAPI; not sure what to do about that one, but: it needs fixing. It looks like the file gets regenerated on build, so manual hack may not be sufficient.

@Wraith2
Copy link
Contributor

Wraith2 commented Nov 16, 2023

I'll take a look.

@mgravell
Copy link
Member Author

mgravell commented Nov 16, 2023

awesome; I'll go ahead and do what I can with my locally hacked version that targets net7 with the first half of the fixes; I can't test the net8 parameter fix (because that single error is blocking the build), but... that's the small end of the wedge for this

@JRahnama JRahnama added this to Needs triage in SqlClient Triage Board via automation Nov 20, 2023
@JRahnama JRahnama moved this from Needs triage to Under Investigation in SqlClient Triage Board Nov 21, 2023
@JRahnama JRahnama added the 🆕 Public API Use this label for new API additions to the driver. label Nov 21, 2023
SqlClient Triage Board automation moved this from Under Investigation to Closed Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 Public API Use this label for new API additions to the driver.
Projects
Development

Successfully merging a pull request may close this issue.

3 participants