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.

Sign up for GitHub

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

Jump to bottom

Fix bug with LegacyRowVersionNullBehavior #1182

Merged
merged 6 commits into from
Aug 19, 2021
Merged

Fix bug with LegacyRowVersionNullBehavior #1182

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8b0e836
Fix bug with LegacyRowVersionNullBehavior
cheenamalhotra Jul 21, 2021
c043536
Fix rowversion issue properly
cheenamalhotra Jul 22, 2021
5a9d629
Legacy cast support
cheenamalhotra Jul 22, 2021
34ffacd
Fix other usecases
cheenamalhotra Jul 22, 2021
f7c3443
Simplify condition
cheenamalhotra Jul 22, 2021
a6be676
Address review comments
cheenamalhotra Aug 17, 2021
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
33 changes: 13 additions & 20 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ protected override void Dispose(bool disposing)
}
base.Dispose(disposing);
}
catch(SqlException ex)
catch (SqlException ex)
{
SqlClientEventSource.Log.TryTraceEvent("SqlDataReader.Dispose | ERR | Error Message: {0}, Stack Trace: {1}", ex.Message, ex.StackTrace);
}
Expand Down Expand Up @@ -3767,26 +3767,19 @@ private bool TryReadColumnInternal(int i, bool readHeaderOnly = false)
_sharedState._nextColumnDataToRead = _sharedState._nextColumnHeaderToRead;
_sharedState._nextColumnHeaderToRead++; // We read this one

if (isNull)
// Trigger new behavior for RowVersion to send DBNull.Value - allow entry for Timestamp here.
if ((!LocalAppContextSwitches.LegacyRowVersionNullBehavior && isNull)
// Legacy behavior - check again for App Context switch and discard entry for Timestamp for "IsNull" data
|| (LocalAppContextSwitches.LegacyRowVersionNullBehavior && isNull && columnMetaData.type != SqlDbType.Timestamp))
{
if (columnMetaData.type == SqlDbType.Timestamp)
{
if (!LocalAppContextSwitches.LegacyRowVersionNullBehavior)
{
_data[i].SetToNullOfType(SqlBuffer.StorageType.SqlBinary);
}
}
else
{
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
columnMetaData,
_command != null ? _command.ColumnEncryptionSetting : SqlCommandColumnEncryptionSetting.UseConnectionSetting,
_parser.Connection);

if (!readHeaderOnly)
{
_sharedState._nextColumnDataToRead++;
}
if (!readHeaderOnly)
{
_sharedState._nextColumnDataToRead++;
}
}
else
Expand Down Expand Up @@ -4098,8 +4091,8 @@ internal bool TrySetMetaData(_SqlMetaDataSet metaData, bool moreInfo)

if (_parser != null)
{ // There is a valid case where parser is null
// Peek, and if row token present, set _hasRows true since there is a
// row in the result
// Peek, and if row token present, set _hasRows true since there is a
// row in the result
byte b;
if (!_stateObj.TryPeekByte(out b))
{
Expand Down Expand Up @@ -5021,7 +5014,7 @@ override public Task<T> GetFieldValueAsync<T>(int i, CancellationToken cancellat
{
_stateObj._shouldHaveEnoughData = true;
#endif
return Task.FromResult(GetFieldValueInternal<T>(i));
return Task.FromResult(GetFieldValueInternal<T>(i));
#if DEBUG
}
finally
Expand Down Expand Up @@ -5115,7 +5108,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi

#endif

internal abstract class SqlDataReaderAsyncCallContext<T> : AAsyncCallContext<SqlDataReader,T>
internal abstract class SqlDataReaderAsyncCallContext<T> : AAsyncCallContext<SqlDataReader, T>
{
internal static readonly Action<Task<T>, object> s_completeCallback = CompleteAsyncCallCallback;

Expand Down
4 changes: 4 additions & 0 deletions src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5429,6 +5429,10 @@ internal static object GetNullSqlValue(SqlBuffer nullVal, SqlMetaDataPriv md, Sq
break;

case SqlDbType.Timestamp:
if (!LocalAppContextSwitches.LegacyRowVersionNullBehavior)
{
nullVal.SetToNullOfType(SqlBuffer.StorageType.SqlBinary);
}
break;

default:
Expand Down
23 changes: 8 additions & 15 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4279,26 +4279,19 @@ private bool TryReadColumnInternal(int i, bool readHeaderOnly = false)
_sharedState._nextColumnDataToRead = _sharedState._nextColumnHeaderToRead;
_sharedState._nextColumnHeaderToRead++; // We read this one

if (isNull)
// Trigger new behavior for RowVersion to send DBNull.Value - allow entry for Timestamp here.
if ((!LocalAppContextSwitches.LegacyRowVersionNullBehavior && isNull)
// Legacy behavior - Discard entry for Timestamp for "IsNull" data
|| (LocalAppContextSwitches.LegacyRowVersionNullBehavior && isNull && columnMetaData.type != SqlDbType.Timestamp))
{
if (columnMetaData.type == SqlDbType.Timestamp)
{
if (!LocalAppContextSwitches.LegacyRowVersionNullBehavior)
{
_data[i].SetToNullOfType(SqlBuffer.StorageType.SqlBinary);
}
}
else
{
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
TdsParser.GetNullSqlValue(_data[_sharedState._nextColumnDataToRead],
columnMetaData,
_command != null ? _command.ColumnEncryptionSetting : SqlCommandColumnEncryptionSetting.UseConnectionSetting,
_parser.Connection);

if (!readHeaderOnly)
{
_sharedState._nextColumnDataToRead++;
}
if (!readHeaderOnly)
{
_sharedState._nextColumnDataToRead++;
}
}
else
Expand Down
6 changes: 4 additions & 2 deletions src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6208,8 +6208,10 @@ internal static object GetNullSqlValue(
break;

case SqlDbType.Timestamp:
// Dev10 Bug #479607 - this should have been the same as SqlDbType.Binary, but it's a rejected breaking change
// Dev10 Bug #752790 - don't assert when it does happen
if (!LocalAppContextSwitches.LegacyRowVersionNullBehavior)
{
nullVal.SetToNullOfType(SqlBuffer.StorageType.SqlBinary);
}
break;

default:
Expand Down
Loading