Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
66c2cd6
Change all possible references from SqlInternalConnection to SqlInter…
benrr101 Nov 12, 2025
2d664a6
Move BeginTransaction and BeginSqlTransaction into SqlInternalConnect…
benrr101 Nov 12, 2025
c796212
Move ChangeDatabase into SqlInternalConnectionTds, move ChangeDatabas…
benrr101 Nov 12, 2025
13c373c
Move EnlistTransaction to SqlInternalConnectionTds
benrr101 Nov 12, 2025
a176e2f
Remove abstract ValidateConnectionForExecute
benrr101 Nov 12, 2025
c5863e3
Move Enlist into SqlInternalConnectionTds
benrr101 Nov 12, 2025
8002ec7
Move EnlistNonNull into SqlInternalConnectionTds
benrr101 Nov 12, 2025
202beb5
Move EnlistNull into SqlInternalConnectionTds
benrr101 Nov 12, 2025
bb3d541
Removed no longer necessary abstractions in SqlInternalConnection
benrr101 Nov 12, 2025
4d39842
Move OnError into SqlInternalConnectionTds
benrr101 Nov 13, 2025
58c358f
Move Deactivate into SqlInternalConnectionTds, merge InternalDeactiva…
benrr101 Nov 13, 2025
07750f8
Move GetTransactionCookie into SqlInternalConnectionTds
benrr101 Nov 13, 2025
dbe9d9e
Move FindLiveReader into SqlInternalConnectionTds
benrr101 Nov 13, 2025
355f2d2
Merge SqlInternalConnection.Dispose into SqlInternalConnectionTds
benrr101 Nov 13, 2025
74b0c92
Move CleanupTransactionOnCompletion, CreateReferenceCollection into S…
benrr101 Nov 13, 2025
ce46515
Introduce a CachedContexts class, move the cached call contexts from …
benrr101 Nov 13, 2025
79cf052
Migrate usages of CachedCommandExecuteNonQueryAsyncContext to the new…
benrr101 Nov 13, 2025
0a45c97
Migrate usages of CachedCommandExecuteXmlReaderAsyncContext to the ne…
benrr101 Nov 13, 2025
7b5a637
Migrate usage of ReadAsyncCallContext to the new class
benrr101 Nov 13, 2025
ba4c7df
Migrate usage of IsDBNullAsyncCallContext to the new class.
benrr101 Nov 13, 2025
a0a1f24
Comments on CachedContexts
benrr101 Nov 13, 2025
41b7f2a
Sure why not move the reader snapshot into the CachedContexts class? …
benrr101 Nov 13, 2025
1b52999
Move _whereAbouts and s_globalTransactionTMID
benrr101 Nov 13, 2025
8fb7b0e
Merge constructors
benrr101 Nov 13, 2025
0e78a6e
Merge AvailableInternalTransaction, CurrentTransaction, HasLocalTrans…
benrr101 Nov 13, 2025
858d98e
Merge Connection, ConnectionOptions, CurrentDatabase, CurrentDataSour…
benrr101 Nov 13, 2025
6dfbd5d
Remove SqlInternalConnection
benrr101 Nov 13, 2025
a83bd7c
Move SqlInternalConnectionTds.cs to SqlConnectionInternal.cs
benrr101 Nov 13, 2025
c4db055
Renane Microsoft.Data.SqlClient.SqlInternalConnectionTds to Microsoft…
benrr101 Nov 13, 2025
d010d48
Fix one reference to SqlInternalConnectionTds in unit tests
benrr101 Nov 14, 2025
4d02cc2
As per copilot comment, rewriting a block in SqlConnectionFactory to …
benrr101 Nov 18, 2025
04e5cdf
As per copilot comment, adding more comments to CachedContexts class
benrr101 Nov 18, 2025
64766f4
Addressing some feedback from @mdiagle
benrr101 Dec 1, 2025
89b92b9
Merge branch 'main' into dev/russellben/flatten/sqlinternalconnection
benrr101 Dec 2, 2025
3b795ae
Rename Clear*Context to Take*Context
benrr101 Dec 2, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ internal class CachedContexts
/// </summary>
private SqlDataReader.ReadAsyncCallContext? _dataReaderReadAsyncContext;

/// <summary>
/// Stores a data reader snapshot.
/// </summary>
private SqlDataReader.Snapshot? _dataReaderSnapshot;

#endregion

#region Access Methods
Expand Down Expand Up @@ -80,6 +85,13 @@ internal class CachedContexts
internal SqlDataReader.IsDBNullAsyncCallContext? ClearDataReaderIsDbNullContext() =>
Interlocked.Exchange(ref _dataReaderIsDbNullContext, null);

/// <summary>
/// Removes and returns the cached data reader snapshot.
/// </summary>
/// <returns>The previously cached snapshot or null when empty.</returns>
internal SqlDataReader.Snapshot? ClearDataReaderSnapshot() =>
Interlocked.Exchange(ref _dataReaderSnapshot, null);

/// <summary>
/// Attempts to cache the provided ExecuteNonQueryAsync context.
/// </summary>
Expand Down Expand Up @@ -130,6 +142,16 @@ internal bool TrySetDataReaderReadAsyncContext(SqlDataReader.ReadAsyncCallContex
internal bool TrySetDataReaderIsDbNullContext(SqlDataReader.IsDBNullAsyncCallContext value) =>
TrySetContext(value, ref _dataReaderIsDbNullContext);

/// <summary>
/// Attempts to cache the provided data reader snapshot context.
/// </summary>
/// <param name="value">Context instance to store.</param>
/// <returns>
/// True when the snapshot is cached; false if an existing snapshot is preserved.
/// </returns>
internal bool TrySetDataReaderSnapshot(SqlDataReader.Snapshot value) =>
TrySetContext(value, ref _dataReaderSnapshot);

#endregion

private static bool TrySetContext<TContext>(TContext value, ref TContext? location)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4101,10 +4101,11 @@ internal TdsOperationStatus TryReadColumnInternal(int i, bool readHeaderOnly = f
{
// reset snapshot to save memory use. We can safely do that here because all SqlDataReader values are stable.
// The retry logic can use the current values to get back to the right state.
if (_connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
if (_connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection)
{
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

[nitpick] The logic for caching the snapshot has changed subtly. Previously, the code checked sqlInternalConnection.CachedDataReaderSnapshot is null before attempting to cache. Now it unconditionally calls TrySetDataReaderSnapshot. While functionally equivalent due to the CompareExchange inside TrySetContext, the semantic meaning is less clear. The previous pattern explicitly checked for null, making the intent clearer. Consider adding a comment explaining that TrySetDataReaderSnapshot will only cache if no snapshot is already present.

Suggested change
{
{
// TrySetDataReaderSnapshot will only cache the snapshot if no snapshot is already present.

Copilot uses AI. Check for mistakes.
sqlInternalConnection.CachedDataReaderSnapshot = _snapshot;
sqlInternalConnection.CachedContexts.TrySetDataReaderSnapshot(_snapshot);
}

_snapshot = null;
PrepareAsyncInvocation(useSnapshot: true);
}
Expand Down Expand Up @@ -5085,10 +5086,11 @@ private static Task<bool> ReadAsyncExecute(Task task, object state)
if (!hasReadRowToken)
{
hasReadRowToken = true;
if (reader.Connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
if (reader.Connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection)
{
sqlInternalConnection.CachedDataReaderSnapshot = reader._snapshot;
sqlInternalConnection.CachedContexts.TrySetDataReaderSnapshot(reader._snapshot);
}

reader._snapshot = null;
reader.PrepareAsyncInvocation(useSnapshot: true);
}
Expand Down Expand Up @@ -5794,7 +5796,7 @@ private void PrepareAsyncInvocation(bool useSnapshot)
{
if (_connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection)
{
_snapshot = Interlocked.Exchange(ref sqlInternalConnection.CachedDataReaderSnapshot, null) ?? new Snapshot();
_snapshot = sqlInternalConnection.CachedContexts.ClearDataReaderSnapshot() ?? new Snapshot();
}
else
{
Expand Down Expand Up @@ -5872,10 +5874,11 @@ private void CleanupAfterAsyncInvocationInternal(TdsParserStateObject stateObj,
stateObj._permitReplayStackTraceToDiffer = false;
#endif

if (_connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
if (_connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection)
{
sqlInternalConnection.CachedDataReaderSnapshot = _snapshot;
sqlInternalConnection.CachedContexts.TrySetDataReaderSnapshot(_snapshot);
}

// We are setting this to null inside the if-statement because stateObj==null means that the reader hasn't been initialized or has been closed (either way _snapshot should already be null)
_snapshot = null;
}
Expand Down Expand Up @@ -5914,10 +5917,11 @@ private void SwitchToAsyncWithoutSnapshot()
Debug.Assert(_snapshot != null, "Should currently have a snapshot");
Debug.Assert(_stateObj != null && !_stateObj._asyncReadWithoutSnapshot, "Already in async without snapshot");

if (_connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection && sqlInternalConnection.CachedDataReaderSnapshot is null)
if (_connection?.InnerConnection is SqlInternalConnectionTds sqlInternalConnection)
{
sqlInternalConnection.CachedDataReaderSnapshot = _snapshot;
sqlInternalConnection.CachedContexts.TrySetDataReaderSnapshot(_snapshot);
}

_snapshot = null;
_stateObj.ResetSnapshot();
_stateObj._asyncReadWithoutSnapshot = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ internal abstract class SqlInternalConnection : DbConnectionInternal
/// </summary>
protected static readonly Guid s_globalTransactionTMID = new("1c742caf-6680-40ea-9c26-6b6846079764");

internal SqlDataReader.Snapshot CachedDataReaderSnapshot;

/// <summary>
/// Constructs a new SqlInternalConnection object using the provided connection options.
/// </summary>
Expand Down