Improve performance of "queryunbuffered", and correctness of "first" …

…APIs (#2121) * see #2115 - correctness: do no use SingleRow by default; affects trailing errors - performance: QueryUnbufferedAsync can mirror cmd?.Cancel() in finally (this is consistent with all other scenarios) * remove settings tweak
DapperLib · Oct 9, 2024 · b4f80b6 · b4f80b6
1 parent e0479ba
commit b4f80b6
Show file tree

Hide file tree

Showing 6 changed files with 208 additions and 54 deletions.
diff --git a/Dapper/SqlMapper.Async.cs b/Dapper/SqlMapper.Async.cs
@@ -1333,6 +1333,11 @@ static async IAsyncEnumerable<T> Impl(IDbConnection cnn, Type effectiveType, Com
                 {
                     if (reader is not null)
                     {
+                        if (!reader.IsClosed)
+                        {
+                            try { cmd?.Cancel(); }
+                            catch { /* don't spoil any existing exception */ }
+                        }
                         await reader.DisposeAsync();
                     }
                     if (wasClosed) cnn.Close();

diff --git a/Dapper/SqlMapper.Settings.cs b/Dapper/SqlMapper.Settings.cs
@@ -11,9 +11,10 @@ public static partial class SqlMapper
         /// </summary>
         public static class Settings
         {
-            // disable single result by default; prevents errors AFTER the select being detected properly
-            private const CommandBehavior DefaultAllowedCommandBehaviors = ~CommandBehavior.SingleResult;
+            // disable single row/result by default; prevents errors AFTER the select being detected properly
+            private const CommandBehavior DefaultAllowedCommandBehaviors = ~(CommandBehavior.SingleResult | CommandBehavior.SingleRow);
             internal static CommandBehavior AllowedCommandBehaviors { get; private set; } = DefaultAllowedCommandBehaviors;
+
             private static void SetAllowedCommandBehaviors(CommandBehavior behavior, bool enabled)
             {
                 if (enabled) AllowedCommandBehaviors |= behavior;

diff --git a/Dapper/SqlMapper.cs b/Dapper/SqlMapper.cs
@@ -1148,7 +1148,7 @@ private static GridReader QueryMultipleImpl(this IDbConnection cnn, ref CommandD
                     if (!reader.IsClosed)
                     {
                         try { cmd?.Cancel(); }
-                        catch { /* don't spoil the existing exception */ }
+                        catch { /* don't spoil any existing exception */ }
                     }
                     reader.Dispose();
                 }
@@ -1229,7 +1229,7 @@ private static IEnumerable<T> QueryImpl<T>(this IDbConnection cnn, CommandDefini
                     if (!reader.IsClosed)
                     {
                         try { cmd?.Cancel(); }
-                        catch { /* don't spoil the existing exception */ }
+                        catch { /* don't spoil any existing exception */ }
                     }
                     reader.Dispose();
                 }
@@ -1321,7 +1321,7 @@ private static T QueryRowImpl<T>(IDbConnection cnn, Row row, ref CommandDefiniti
                     if (!reader.IsClosed)
                     {
                         try { cmd?.Cancel(); }
-                        catch { /* don't spoil the existing exception */ }
+                        catch { /* don't spoil any existing exception */ }
                     }
                     reader.Dispose();
                 }

diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -1,51 +1,51 @@
 <Project>
-    <ItemGroup>
-      <!-- note: 6.2.0 has regressions; don't force the update -->
-      <PackageVersion Include="EntityFramework" Version="6.1.3" />
-      <PackageVersion Include="Microsoft.CSharp" Version="4.7.0" />
-      <PackageVersion Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" />
-      <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
-      <PackageVersion Include="Microsoft.SqlServer.Types" Version="14.0.1016.290" />
-      <PackageVersion Include="Nerdbank.GitVersioning" Version="3.6.143" />
-      <PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.4" />
-      <PackageVersion Include="System.Reflection.Emit.Lightweight" Version="4.7.0" />
-      
-      <!-- tests -->
-      <PackageVersion Include="Azure.Identity" Version="1.12.1" />
-      <PackageVersion Include="Belgrade.Sql.Client" Version="1.1.4" />
-      <PackageVersion Include="BenchmarkDotNet" Version="0.14.0" />
-      <PackageVersion Include="Dashing" Version="2.10.1" />
-      <PackageVersion Include="Dapper.Contrib" Version="2.0.78" />
-      <PackageVersion Include="DuckDB.NET.Data.Full" Version="1.1.1" />
-      <PackageVersion Include="DevExpress.Xpo" Version="24.1.6" />
-      <PackageVersion Include="FirebirdSql.Data.FirebirdClient" Version="10.3.1" />
-      <PackageVersion Include="GitHubActionsTestLogger" Version="2.4.1" />
-      <PackageVersion Include="Iesi.Collections" Version="4.1.1" />
-      <PackageVersion Include="linq2db.SqlServer" Version="5.4.1" />
-      <PackageVersion Include="Microsoft.Data.SqlClient" Version="5.2.2" />
-      <PackageVersion Include="Microsoft.Data.Sqlite" Version="8.0.8" />
-      <PackageVersion Include="Microsoft.EntityFrameworkCore.SqlServer" Version="8.0.8" />
-      <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
-      <PackageVersion Include="Mighty" Version="3.2.0" />
-      <PackageVersion Include="MySqlConnector" Version="2.3.7" />
-      <PackageVersion Include="NHibernate" Version="5.5.2" />
-      <PackageVersion Include="Norm.net" Version="5.4.0" />
-      <PackageVersion Include="Npgsql" Version="8.0.4" />
-      <PackageVersion Include="PetaPoco" Version="5.1.306" />
-      <PackageVersion Include="RepoDb.SqlServer" Version="1.13.1" />
-      <PackageVersion Include="ServiceStack.OrmLite.SqlServer" Version="8.4.0" />
-      <PackageVersion Include="Snowflake.Data" Version="4.1.0" />
-      <PackageVersion Include="SqlMarshal" Version="0.5.0" />
-      <PackageVersion Include="SubSonic" Version="3.0.0.4" />
-      <PackageVersion Include="Susanoo.SqlServer" Version="1.2.4.2" />
-      <PackageVersion Include="System.Data.SqlClient" Version="4.8.6" />
-      <PackageVersion Include="System.Data.SQLite" Version="1.0.119" />
-      <PackageVersion Include="System.Net.Http" Version="4.3.4" />
-      <PackageVersion Include="System.Private.Uri" Version="4.3.2" />
-      <PackageVersion Include="System.Reflection.Metadata" Version="8.0.0" />
-      <PackageVersion Include="System.Text.RegularExpressions" Version="4.3.1" />
-      <PackageVersion Include="System.ValueTuple" Version="4.5.0" />
-      <PackageVersion Include="xunit" Version="2.9.2" />
-      <PackageVersion Include="xunit.runner.visualstudio" Version="2.8.2" />
-    </ItemGroup>
+  <ItemGroup>
+    <!-- note: 6.2.0 has regressions; don't force the update -->
+    <PackageVersion Include="EntityFramework" Version="6.1.3" />
+    <PackageVersion Include="FastMember" Version="1.5.0" />
+    <PackageVersion Include="Microsoft.CSharp" Version="4.7.0" />
+    <PackageVersion Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" />
+    <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
+    <PackageVersion Include="Microsoft.SqlServer.Types" Version="14.0.1016.290" />
+    <PackageVersion Include="Nerdbank.GitVersioning" Version="3.6.143" />
+    <PackageVersion Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" Version="3.3.4" />
+    <PackageVersion Include="System.Reflection.Emit.Lightweight" Version="4.7.0" />
+    <!-- tests -->
+    <PackageVersion Include="Azure.Identity" Version="1.12.1" />
+    <PackageVersion Include="Belgrade.Sql.Client" Version="1.1.4" />
+    <PackageVersion Include="BenchmarkDotNet" Version="0.14.0" />
+    <PackageVersion Include="Dashing" Version="2.10.1" />
+    <PackageVersion Include="Dapper.Contrib" Version="2.0.78" />
+    <PackageVersion Include="DuckDB.NET.Data.Full" Version="1.1.1" />
+    <PackageVersion Include="DevExpress.Xpo" Version="24.1.6" />
+    <PackageVersion Include="FirebirdSql.Data.FirebirdClient" Version="10.3.1" />
+    <PackageVersion Include="GitHubActionsTestLogger" Version="2.4.1" />
+    <PackageVersion Include="Iesi.Collections" Version="4.1.1" />
+    <PackageVersion Include="linq2db.SqlServer" Version="5.4.1" />
+    <PackageVersion Include="Microsoft.Data.SqlClient" Version="5.2.2" />
+    <PackageVersion Include="Microsoft.Data.Sqlite" Version="8.0.8" />
+    <PackageVersion Include="Microsoft.EntityFrameworkCore.SqlServer" Version="8.0.8" />
+    <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
+    <PackageVersion Include="Mighty" Version="3.2.0" />
+    <PackageVersion Include="MySqlConnector" Version="2.3.7" />
+    <PackageVersion Include="NHibernate" Version="5.5.2" />
+    <PackageVersion Include="Norm.net" Version="5.4.0" />
+    <PackageVersion Include="Npgsql" Version="8.0.4" />
+    <PackageVersion Include="PetaPoco" Version="5.1.306" />
+    <PackageVersion Include="RepoDb.SqlServer" Version="1.13.1" />
+    <PackageVersion Include="ServiceStack.OrmLite.SqlServer" Version="8.4.0" />
+    <PackageVersion Include="Snowflake.Data" Version="4.1.0" />
+    <PackageVersion Include="SqlMarshal" Version="0.5.0" />
+    <PackageVersion Include="SubSonic" Version="3.0.0.4" />
+    <PackageVersion Include="Susanoo.SqlServer" Version="1.2.4.2" />
+    <PackageVersion Include="System.Data.SqlClient" Version="4.8.6" />
+    <PackageVersion Include="System.Data.SQLite" Version="1.0.119" />
+    <PackageVersion Include="System.Net.Http" Version="4.3.4" />
+    <PackageVersion Include="System.Private.Uri" Version="4.3.2" />
+    <PackageVersion Include="System.Reflection.Metadata" Version="8.0.0" />
+    <PackageVersion Include="System.Text.RegularExpressions" Version="4.3.1" />
+    <PackageVersion Include="System.ValueTuple" Version="4.5.0" />
+    <PackageVersion Include="xunit" Version="2.9.2" />
+    <PackageVersion Include="xunit.runner.visualstudio" Version="2.8.2" />
+  </ItemGroup>
 </Project>
diff --git a/tests/Dapper.Tests/Dapper.Tests.csproj b/tests/Dapper.Tests/Dapper.Tests.csproj
@@ -6,6 +6,7 @@
     <DefineConstants>$(DefineConstants);MSSQLCLIENT</DefineConstants>
     <NoWarn>$(NoWarn);IDE0017;IDE0034;IDE0037;IDE0039;IDE0042;IDE0044;IDE0051;IDE0052;IDE0059;IDE0060;IDE0063;IDE1006;xUnit1004;CA1806;CA1816;CA1822;CA1825;CA2208;CA1861</NoWarn>
     <Nullable>enable</Nullable>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(TargetFramework)' == 'net462' OR '$(TargetFramework)' == 'net472'">
@@ -16,6 +17,7 @@
     <ProjectReference Include="../../Dapper.ProviderTools/Dapper.ProviderTools.csproj" />
     <ProjectReference Include="../../Dapper.SqlBuilder/Dapper.SqlBuilder.csproj" />
     <PackageReference Include="DuckDB.NET.Data.Full" />
+    <PackageReference Include="FastMember" />
     <PackageReference Include="FirebirdSql.Data.FirebirdClient" />
     <PackageReference Include="Microsoft.Data.SqlClient" />
     <PackageReference Include="Microsoft.Data.Sqlite" />

diff --git a/tests/Dapper.Tests/SingleRowTests.cs b/tests/Dapper.Tests/SingleRowTests.cs
@@ -0,0 +1,146 @@
+using System;
+using System.Collections.Generic;
+using System.Data.Common;
+using System.Diagnostics;
+using System.IO;
+using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
+using FastMember;
+using Xunit;
+using Xunit.Abstractions;
+using static Dapper.SqlMapper;
+
+namespace Dapper.Tests;
+
+[Collection("SingleRowTests")]
+public sealed class SystemSqlClientSingleRowTests(ITestOutputHelper log) : SingleRowTests<SystemSqlClientProvider>(log)
+{
+    protected override async Task InjectDataAsync(DbConnection conn, DbDataReader source)
+    {
+        using var bcp = new System.Data.SqlClient.SqlBulkCopy((System.Data.SqlClient.SqlConnection)conn);
+        bcp.DestinationTableName = "#mydata";
+        bcp.EnableStreaming = true;
+        await bcp.WriteToServerAsync(source);
+    }
+}
+#if MSSQLCLIENT
+[Collection("SingleRowTests")]
+public sealed class MicrosoftSqlClientSingleRowTests(ITestOutputHelper log) : SingleRowTests<MicrosoftSqlClientProvider>(log)
+{
+    protected override async Task InjectDataAsync(DbConnection conn, DbDataReader source)
+    {
+        using var bcp = new Microsoft.Data.SqlClient.SqlBulkCopy((Microsoft.Data.SqlClient.SqlConnection)conn);
+        bcp.DestinationTableName = "#mydata";
+        bcp.EnableStreaming = true;
+        await bcp.WriteToServerAsync(source);
+    }
+}
+#endif
+public abstract class SingleRowTests<TProvider>(ITestOutputHelper log) : TestBase<TProvider> where TProvider : DatabaseProvider
+{
+    protected abstract Task InjectDataAsync(DbConnection connection, DbDataReader source);
+
+    [Fact]
+    public async Task QueryFirst_PerformanceAndCorrectness()
+    {
+        using var conn = GetOpenConnection();
+        conn.Execute("create table #mydata(id int not null, name nvarchar(250) not null)");
+
+        var rand = new Random();
+        var data = from id in Enumerable.Range(1, 500_000)
+                    select new MyRow { Id = rand.Next(), Name = CreateName(rand) };
+
+        Stopwatch watch;
+        using (var reader = ObjectReader.Create(data))
+        {
+            await InjectDataAsync(conn, reader);
+            watch = Stopwatch.StartNew();
+            var count = await conn.QuerySingleAsync<int>("""select count(1) from #mydata""");
+            watch.Stop();
+            log.WriteLine($"bulk-insert complete; {count} rows in {watch.ElapsedMilliseconds}ms");
+        }
+
+        // just errors
+        var ex = Assert.ThrowsAny<DbException>(() => conn.Execute("raiserror('bad things', 16, 1)"));
+        log.WriteLine(ex.Message);
+        ex = await Assert.ThrowsAnyAsync<DbException>(async () => await conn.ExecuteAsync("raiserror('bad things', 16, 1)"));
+        log.WriteLine(ex.Message);
+
+        // just data
+        watch = Stopwatch.StartNew();
+        var row = conn.QueryFirst<MyRow>("select top 1 * from #mydata");
+        watch.Stop();
+        log.WriteLine($"sync top 1 read first complete; row {row.Id} in {watch.ElapsedMilliseconds}ms");
+
+        watch = Stopwatch.StartNew();
+        row = await conn.QueryFirstAsync<MyRow>("select top 1 * from #mydata");
+        watch.Stop();
+        log.WriteLine($"async top 1 read first complete; row {row.Id} in {watch.ElapsedMilliseconds}ms");
+
+        watch = Stopwatch.StartNew();
+        row = conn.QueryFirst<MyRow>("select * from #mydata");
+        watch.Stop();
+        log.WriteLine($"sync read first complete; row {row.Id} in {watch.ElapsedMilliseconds}ms");
+
+        watch = Stopwatch.StartNew();
+        row = await conn.QueryFirstAsync<MyRow>("select * from #mydata");
+        watch.Stop();
+        log.WriteLine($"async read first complete; row {row.Id} in {watch.ElapsedMilliseconds}ms");
+
+        // data with trailing errors
+
+        watch = Stopwatch.StartNew();
+        ex = Assert.ThrowsAny<DbException>(() => conn.QueryFirst<MyRow>("select * from #mydata; raiserror('bad things', 16, 1)"));
+        watch.Stop();
+        log.WriteLine($"sync read with error complete in {watch.ElapsedMilliseconds}ms; {ex.Message}");
+
+        watch = Stopwatch.StartNew();
+        ex = await Assert.ThrowsAnyAsync<DbException>(async () => await conn.QueryFirstAsync<MyRow>("select * from #mydata; raiserror('bad things', 16, 1)"));
+        watch.Stop();
+        log.WriteLine($"async read with error complete in {watch.ElapsedMilliseconds}ms; {ex.Message}");
+
+        // unbuffered read with trailing errors - do not expect to see this unless we consume all!
+
+        watch = Stopwatch.StartNew();
+        row = conn.Query<MyRow>("select * from #mydata", buffered: false).First();
+        watch.Stop();
+        log.WriteLine($"sync unbuffered LINQ read first complete; row {row.Id} in {watch.ElapsedMilliseconds}ms");
+
+#if NET5_0_OR_GREATER
+        watch = Stopwatch.StartNew();
+        row = await conn.QueryUnbufferedAsync<MyRow>("select * from #mydata").FirstAsync();
+        watch.Stop();
+        log.WriteLine($"async unbuffered LINQ read first complete; row {row.Id} in {watch.ElapsedMilliseconds}ms");
+#endif
+
+        static unsafe string CreateName(Random rand)
+        {
+            const string Alphabet = "abcdefghijklmnopqrstuvwxyz 0123456789,;-";
+            var len = rand.Next(5, 251);
+            char* ptr = stackalloc char[len];
+            for (int i = 0; i < len; i++)
+            {
+                ptr[i] = Alphabet[rand.Next(Alphabet.Length)];
+            }
+            return new string(ptr, 0, len);
+        }
+
+    }
+
+    public class MyRow
+    {
+        public int Id { get; set; }
+        public string Name { get; set; } = "";
+    }
+}
+
+internal static class AsyncLinqHelper
+{
+    public static async ValueTask<T> FirstAsync<T>(this IAsyncEnumerable<T> source, CancellationToken cancellationToken = default)
+    {
+        await using var iter = source.GetAsyncEnumerator(cancellationToken);
+        if (!await iter.MoveNextAsync()) Array.Empty<T>().First(); // for consistent error
+        return iter.Current;
+    }
+}