From ca1a943b2218a5f960b3dc87fbe88820f31ddb19 Mon Sep 17 00:00:00 2001 From: Michel Zehnder Date: Wed, 6 Nov 2024 04:01:46 +0100 Subject: [PATCH] Align comments between netfx and netcore codebases (#2966) --- .../Microsoft/Data/SqlClient/SqlDataReader.cs | 10 +-- .../Microsoft/Data/SqlClient/SqlDataReader.cs | 64 ++++--------------- 2 files changed, 18 insertions(+), 56 deletions(-) diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs index 1d1309076b..5a877d61e9 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -5,6 +5,7 @@ using System; using System.Collections; using System.Collections.ObjectModel; +using System.ComponentModel; using System.Data; using System.Data.Common; using System.Data.SqlTypes; @@ -917,7 +918,7 @@ public override void Close() { if (_stateObj != null) { // reader not closed while we waited for the lock - // TryCloseInternal will clear out the snapshot when it is done + // TryCloseInternal will clear out the snapshot when it is done if (_snapshot != null) { #if DEBUG @@ -2045,6 +2046,7 @@ override public TextReader GetTextReader(int i) } /// + [EditorBrowsableAttribute(EditorBrowsableState.Never)] override public char GetChar(int i) { throw ADP.NotSupported(); @@ -3020,7 +3022,7 @@ private T GetFieldValueFromSqlBufferInternal(SqlBuffer data, _SqlMetaData met // Stream is only for Binary, Image, VarBinary, Udt, Xml and Timestamp(RowVersion) types MetaType metaType = metaData.metaType; if ( - (!metaType.IsBinType || metaType.SqlDbType == SqlDbType.Timestamp) && + (!metaType.IsBinType || metaType.SqlDbType == SqlDbType.Timestamp) && metaType.SqlDbType != SqlDbType.Variant ) { @@ -4360,7 +4362,7 @@ internal TdsOperationStatus TrySetMetaData(_SqlMetaDataSet metaData, bool moreIn // simply rip the order token off the wire if (b == TdsEnums.SQLORDER) { - // same logic as SetAltMetaDataSet + // same logic as SetAltMetaDataSet result = _parser.TryRun(RunBehavior.ReturnImmediately, null, null, null, _stateObj, out _); if (result != TdsOperationStatus.Done) { @@ -5381,7 +5383,7 @@ internal void CompletePendingReadWithFailure(int errorCode, bool resetForcePendi } #endif - + internal abstract class SqlDataReaderBaseAsyncCallContext : AAsyncBaseCallContext { internal static readonly Action, object> s_completeCallback = CompleteAsyncCallCallback; diff --git a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs index a918f61a1f..d3a2e605da 100644 --- a/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -76,10 +76,6 @@ internal class SharedState private static int s_objectTypeCount; // EventSource Counter internal readonly int ObjectID = Interlocked.Increment(ref s_objectTypeCount); - // context - // undone: we may still want to do this...it's nice to pass in an lpvoid (essentially) and just have the reader keep the state - // private object _context = null; // this is never looked at by the stream object. It is used by upper layers who wish - // to remain stateless // metadata (no explicit table, use 'Table') private MultiPartTableName[] _tableNames = null; @@ -353,7 +349,7 @@ internal virtual SmiExtendedMetaData[] GetInternalSmiMetaData() } else if (SqlDbType.Udt == colMetaData.type) { - Connection.CheckGetExtendedUDTInfo(colMetaData, true); // SQLBUDT #370593 ensure that colMetaData.udtType is set + Connection.CheckGetExtendedUDTInfo(colMetaData, true); // Ensure that colMetaData.udtType is set typeSpecificNamePart1 = colMetaData.udt?.DatabaseName; typeSpecificNamePart2 = colMetaData.udt?.SchemaName; @@ -500,9 +496,6 @@ internal void Bind(TdsParserStateObject stateObj) _defaultLCID = _parser.DefaultLCID; } - // Fills in a schema table with meta data information. This function should only really be called by - // UNDONE: need a way to refresh the table with more information as more data comes online for browse info like - // table names and key information #if !NETFRAMEWORK [SuppressMessage("ReflectionAnalysis", "IL2111", Justification = "System.Type.TypeInitializer would not be used in dataType and providerSpecificDataType columns.")] @@ -1193,7 +1186,7 @@ private TdsOperationStatus TryCloseInternal(bool closeReader) CleanupAfterAsyncInvocationInternal(stateObj); } - // SQLBUDT #284712 - Note the order here is extremely important: + // Note the order here is extremely important: // // (1) First, we remove the reader from the reference collection // to prevent it from being forced closed by the parser if @@ -1271,6 +1264,7 @@ private TdsOperationStatus TryCloseInternal(bool closeReader) } throw; } + // DO NOT USE stateObj after this point - it has been returned to the TdsParser's session pool and potentially handed out to another thread // do not retry here result = TrySetMetaData(null, false); @@ -1657,7 +1651,7 @@ override public int GetOrdinal(string name) CheckMetaDataIsReady(); _fieldNameLookup = new FieldNameLookup(this, _defaultLCID); } - return _fieldNameLookup.GetOrdinal(name); // MDAC 71470 + return _fieldNameLookup.GetOrdinal(name); } finally { @@ -2344,7 +2338,7 @@ override public TextReader GetTextReader(int i) } /// - [EditorBrowsableAttribute(EditorBrowsableState.Never)] // MDAC 69508 + [EditorBrowsableAttribute(EditorBrowsableState.Never)] override public char GetChar(int i) { throw ADP.NotSupported(); @@ -2423,7 +2417,6 @@ override public long GetChars(int i, long dataIndex, char[] buffer, int bufferIn } catch (Exception ex) { - // Dev11 Bug #315513: Exception type breaking change from 4.0 RTM when calling GetChars on null xml // We need to wrap all exceptions inside a TargetInvocationException to simulate calling CreateSqlReader via MethodInfo.Invoke if (ADP.IsCatchableExceptionType(ex)) { @@ -2504,7 +2497,6 @@ override public long GetChars(int i, long dataIndex, char[] buffer, int bufferIn } catch (Exception e) { - // UNDONE - should not be catching all exceptions!!! if (!ADP.IsCatchableExceptionType(e)) { throw; @@ -2895,7 +2887,6 @@ virtual public SqlSingle GetSqlSingle(int i) } /// - // UNDONE: need non-unicode SqlString support virtual public SqlString GetSqlString(int i) { ReadColumn(i); @@ -2985,8 +2976,6 @@ private object GetSqlValueInternal(int i) // Always make sure to take reference copies of anything set to null in TryCloseInternal() private object GetSqlValueFromSqlBufferInternal(SqlBuffer data, _SqlMetaData metaData) { - // Dev11 Bug #336820, Dev10 Bug #479607 (SqlClient: IsDBNull always returns false for timestamp datatype) - // Due to a bug in TdsParser.GetNullSqlValue, Timestamps' IsNull is not correctly set - so we need to bypass the following check Debug.Assert(!data.IsEmpty || data.IsNull || metaData.type == SqlDbType.Timestamp, "Data has been read, but the buffer is empty"); // Convert 2008 types to string @@ -3177,8 +3166,6 @@ private object GetValueInternal(int i) // Always make sure to take reference copies of anything set to null in TryCloseInternal() private object GetValueFromSqlBufferInternal(SqlBuffer data, _SqlMetaData metaData) { - // Dev11 Bug #336820, Dev10 Bug #479607 (SqlClient: IsDBNull always returns false for timestamp datatype) - // Due to a bug in TdsParser.GetNullSqlValue, Timestamps' IsNull is not correctly set - so we need to bypass the following check Debug.Assert(!data.IsEmpty || data.IsNull || metaData.type == SqlDbType.Timestamp, "Data has been read, but the buffer is empty"); if (_typeSystem <= SqlConnectionString.TypeSystem.SQLServer2005 && metaData.Is2008DateTimeType) @@ -3503,7 +3490,7 @@ override public int GetValues(object[] values) _data[i].Clear(); if (fieldIndex > i && fieldIndex > 0) { - // if we jumped an index forward because of a hidden column see if the buffer before the + // if we jumped an index forward because of a hidden column see if the buffer before the // current one was populated by the seek forward and clear it if it was _data[fieldIndex - 1].Clear(); } @@ -3595,10 +3582,6 @@ private TdsOperationStatus TryHasMoreResults(out bool moreResults) // always happens if there is a row following an altrow moreResults = true; return TdsOperationStatus.Done; - - // VSTFDEVDIV 926281: DONEINPROC case is missing here; we have decided to reject this bug as it would result in breaking change - // from VS2008 RTM/SP1 and Dev10 RTM. See the bug for more details. - // case TdsEnums.DONEINPROC: case TdsEnums.SQLDONE: Debug.Assert(_altRowStatus == ALTROWSTATUS.Done || _altRowStatus == ALTROWSTATUS.Null, "invalid AltRowStatus"); _altRowStatus = ALTROWSTATUS.Null; @@ -3628,8 +3611,6 @@ private TdsOperationStatus TryHasMoreResults(out bool moreResults) return TdsOperationStatus.Done; } - // Dev11 Bug 316483: Stuck at SqlDataReader::TryHasMoreResults using MARS - // http://vstfdevdiv:8080/web/wi.aspx?pcguid=22f9acc9-569a-41ff-b6ac-fac1b6370209&id=316483 // TryRun() will immediately return if the TdsParser is closed/broken, causing us to enter an infinite loop // Instead, we will throw a closed connection exception if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) @@ -3674,19 +3655,6 @@ private TdsOperationStatus TryHasMoreRows(out bool moreRows) if (_stateObj.HasPendingData) { // Consume error's, info's, done's on HasMoreRows, so user obtains error on Read. - // Previous bug where Read() would return false with error on the wire in the case - // of metadata and error immediately following. See MDAC 78285 and 75225. - - // BUGBUG - currently in V1 the if (_parser.PendingData) does not - // exist, so under certain conditions HasMoreRows can timeout. However, - // this should only occur when executing as SqlBatch and returning a reader. - // Updated - SQL Bug: 20001249 - // Modifed while loop and added parsedDoneToken, to revert a regression from everettfs. - // "Error Exceptions" are now only thown in read when a error occures in the exception, otherwise the exception will be thrown on the call to get the next result set. - // resultset. - - // process any done, doneproc and doneinproc token streams and - // any order, error or info token preceeding the first done, doneproc or doneinproc token stream byte b; TdsOperationStatus result = _stateObj.TryPeekByte(out b); if (result != TdsOperationStatus.Done) @@ -3713,8 +3681,6 @@ private TdsOperationStatus TryHasMoreRows(out bool moreRows) ParsedDoneToken = true; } - // Dev11 Bug 316483: Stuck at SqlDataReader::TryHasMoreResults when using MARS - // http://vstfdevdiv:8080/web/wi.aspx?pcguid=22f9acc9-569a-41ff-b6ac-fac1b6370209&id=316483 // TryRun() will immediately return if the TdsParser is closed/broken, causing us to enter an infinite loop // Instead, we will throw a closed connection exception if (_parser.State == TdsParserState.Broken || _parser.State == TdsParserState.Closed) @@ -4895,8 +4861,8 @@ internal TdsOperationStatus TrySetMetaData(_SqlMetaDataSet metaData, bool moreIn 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; TdsOperationStatus result = _stateObj.TryPeekByte(out b); if (result != TdsOperationStatus.Done) @@ -4904,16 +4870,10 @@ internal TdsOperationStatus TrySetMetaData(_SqlMetaDataSet metaData, bool moreIn return result; } - // UNDONE - should we be consuming tokens here??? Maybe we should be calling HasMoreRows? - // Would that have other side effects? - // simply rip the order token off the wire if (b == TdsEnums.SQLORDER) - { + { // same logic as SetAltMetaDataSet - // Devnote: That's not the right place to process TDS - // Can this result in Reentrance to Run? - result = _parser.TryRun(RunBehavior.ReturnImmediately, null, null, null, _stateObj, out _); if (result != TdsOperationStatus.Done) { @@ -5151,7 +5111,7 @@ private static Task NextResultAsyncExecute(Task task, object state) if (context.Reader.TryNextResult(out bool more) == TdsOperationStatus.Done) { - // completed + // completed return more ? ADP.TrueTask : ADP.FalseTask; } @@ -5573,7 +5533,7 @@ private static Task ReadAsyncExecute(Task task, object state) // If there are no more rows, or this is Sequential Access, then we are done if (!hasMoreData || (reader._commandBehavior & CommandBehavior.SequentialAccess) == CommandBehavior.SequentialAccess) { - // completed + // completed return hasMoreData ? ADP.TrueTask : ADP.FalseTask; } else @@ -5590,7 +5550,7 @@ private static Task ReadAsyncExecute(Task task, object state) TdsOperationStatus result = reader.TryReadColumn(reader._metaData.Length - 1, true); if (result == TdsOperationStatus.Done) { - // completed + // completed return ADP.TrueTask; } }