QueryUnbufferedAsync cancels command unnecessarily for Npgsql, causing performance degradation

[alexgrudanov]

Description

A change introduced in version 2.1.66 to cancel the command in a finally block (https://github.com/DapperLib/Dapper/blob/main/Dapper/SqlMapper.Async.cs#L1336) causes significant performance degradation when used with the Npgsql provider (PostgreSQL).

The issue is that for NpgsqlDataReader, the reader.IsClosed property returns false even after all data has been completely read and the reader is functionally finished. This triggers the explicit cmd.Cancel() in the finally block unnecessarily.

For PostgreSQL, canceling a command is not a lightweight operation. It requires opening a new physical connection to the server to send the cancellation request. This leads to:

1. Performance overhead on the application side due to the extra connection setup.
2. Performance overhead on the database server, which has to process the cancellation command and abort the already-finished query.
3. Connection pool churn, as the cancellation process uses up additional connections temporarily.

This behavior negates the performance benefits of using Unbuffered asynchronous reads.

Proposed Solution

The cancellation logic should be more intelligent. Instead of relying solely on !reader.IsClosed, it should also consider if the reader has already consumed the entire result set naturally.

A better condition might be to check !reader.IsClosed && !reader.NextResult(). Alternatively, a more sophisticated approach could be taken to determine if the reader has been fully enumerated without being explicitly closed.

Steps to Reproduce

1. Use Dapper 2.1.66+ with Npgsql.
2. Execute a QueryUnbufferedAsync query.
3. Observe in SQL logs or with a profiler (like log_statement = 'all' in postgresql.conf) that a CANCEL command is issued for the query even after it has completed successfully.

This behavior is specific to how the Npgsql provider works. Other providers (e.g., SQL Server) may correctly set IsClosed to true after full enumeration, making the Cancel() call a no-op and avoiding this performance issue.

[gunjambi]

I'm seeing the same issue with MySqlConnector and its MySqlDataReader. It has a similar behavior, except the connection for the cancellation request seems to bypass connection pool, causing issues of its own.

I seem to be able work around this by implementing a simplified version QueryUnbufferedAsync. This is not ideal, as this simplified version lacks the nuance needed for the variety of real world databases and providers. It works on my machine though :)

static async IAsyncEnumerable<T> ManualReadAsync<T>(System.Data.Common.DbConnection conn, string query, object queryParams)
{
    System.Data.Common.DbDataReader reader = await conn.ExecuteReaderAsync(new CommandDefinition(query, queryParams, flags: CommandFlags.None), CommandBehavior.SingleResult | CommandBehavior.SequentialAccess).ConfigureAwait(false);
    try
    {
        Func<System.Data.Common.DbDataReader, object> deserializer = Dapper.SqlMapper.GetTypeDeserializer(typeof(T), reader);
        while (await reader.ReadAsync().ConfigureAwait(false))
        {
            object v = deserializer(reader);
            yield return (T)v;
        }
        await reader.CloseAsync().ConfigureAwait(false);
    }
    finally
    {
        await reader.DisposeAsync().ConfigureAwait(false);
    }
    await conn.CloseAsync().ConfigureAwait(false);
}