Annotate DbDataReader.GetSchemaTable as nullable (#41082)

Reverts decision made in #509
dotnet · Aug 21, 2020 · ab225ee · ab225ee
1 parent bd7ae78
commit ab225ee
Show file tree

Hide file tree

Showing 13 changed files with 31 additions and 36 deletions.
diff --git a/src/libraries/Common/src/System/Data/ProviderBase/DbMetaDataFactory.cs b/src/libraries/Common/src/System/Data/ProviderBase/DbMetaDataFactory.cs
@@ -168,7 +168,7 @@ private DataTable ExecuteCommand(DataRow requestedCollectionRow, string?[]? rest
                     Locale = CultureInfo.InvariantCulture
                 };
 
-                schemaTable = reader.GetSchemaTable();
+                schemaTable = reader.GetSchemaTable()!;
                 foreach (DataRow row in schemaTable.Rows)
                 {
                     resultTable.Columns.Add(row["ColumnName"] as string, (Type)row["DataType"]);

diff --git a/src/libraries/System.Data.Common/ref/System.Data.Common.cs b/src/libraries/System.Data.Common/ref/System.Data.Common.cs
@@ -856,7 +856,7 @@ public override void Close() { }
         public override System.Type GetProviderSpecificFieldType(int ordinal) { throw null; }
         public override object GetProviderSpecificValue(int ordinal) { throw null; }
         public override int GetProviderSpecificValues(object[] values) { throw null; }
-        public override System.Data.DataTable GetSchemaTable() { throw null; }
+        public override System.Data.DataTable? GetSchemaTable() { throw null; }
         public override string GetString(int ordinal) { throw null; }
         public override object GetValue(int ordinal) { throw null; }
         public override int GetValues(object[] values) { throw null; }
@@ -1236,7 +1236,7 @@ public partial interface IDataReader : System.Data.IDataRecord, System.IDisposab
         bool IsClosed { get; }
         int RecordsAffected { get; }
         void Close();
-        System.Data.DataTable GetSchemaTable();
+        System.Data.DataTable? GetSchemaTable();
         bool NextResult();
         bool Read();
     }
@@ -1935,7 +1935,7 @@ protected override void Dispose(bool disposing) { }
         protected abstract string GetParameterName(int parameterOrdinal);
         protected abstract string GetParameterName(string parameterName);
         protected abstract string GetParameterPlaceholder(int parameterOrdinal);
-        protected virtual System.Data.DataTable GetSchemaTable(System.Data.Common.DbCommand sourceCommand) { throw null; }
+        protected virtual System.Data.DataTable? GetSchemaTable(System.Data.Common.DbCommand sourceCommand) { throw null; }
         public System.Data.Common.DbCommand GetUpdateCommand() { throw null; }
         public System.Data.Common.DbCommand GetUpdateCommand(bool useColumnsForParameterNames) { throw null; }
         protected virtual System.Data.Common.DbCommand InitializeCommand(System.Data.Common.DbCommand? command) { throw null; }
@@ -2156,8 +2156,8 @@ protected virtual void Dispose(bool disposing) { }
         public virtual object GetProviderSpecificValue(int ordinal) { throw null; }
         [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
         public virtual int GetProviderSpecificValues(object[] values) { throw null; }
-        public virtual System.Data.DataTable GetSchemaTable() { throw null; }
-        public virtual System.Threading.Tasks.Task<System.Data.DataTable> GetSchemaTableAsync(System.Threading.CancellationToken cancellationToken = default) { throw null; }
+        public virtual System.Data.DataTable? GetSchemaTable() { throw null; }
+        public virtual System.Threading.Tasks.Task<System.Data.DataTable?> GetSchemaTableAsync(System.Threading.CancellationToken cancellationToken = default) { throw null; }
         public virtual System.Threading.Tasks.Task<System.Collections.ObjectModel.ReadOnlyCollection<System.Data.Common.DbColumn>> GetColumnSchemaAsync(System.Threading.CancellationToken cancellationToken = default) { throw null; }
         public virtual System.IO.Stream GetStream(int ordinal) { throw null; }
         public abstract string GetString(int ordinal);

diff --git a/src/libraries/System.Data.Common/src/System/Data/Common/DBCommandBuilder.cs b/src/libraries/System.Data.Common/src/System/Data/Common/DBCommandBuilder.cs
@@ -646,7 +646,7 @@ private void BuildCache(bool closeConnection, DataRow? dataRow, bool useColumnsF
             ADP.BuildSchemaTableInfoTableNames(srcColumnNames);
         }
 
-        protected virtual DataTable GetSchemaTable(DbCommand sourceCommand)
+        protected virtual DataTable? GetSchemaTable(DbCommand sourceCommand)
         {
             using (IDataReader dataReader = sourceCommand.ExecuteReader(CommandBehavior.SchemaOnly | CommandBehavior.KeyInfo))
             {

diff --git a/src/libraries/System.Data.Common/src/System/Data/Common/DbDataReader.cs b/src/libraries/System.Data.Common/src/System/Data/Common/DbDataReader.cs
@@ -75,12 +75,13 @@ public virtual ValueTask DisposeAsync()
 
         /// <summary>
         /// Returns a <see cref="DataTable" /> that describes the column metadata of the ><see cref="DbDataReader" />.
+        ///
+        /// Returns <see langword="null" /> if the executed command returned no resultset, or after
+        /// <see cref="NextResult" /> returns <see langword="false" />.
         /// </summary>
         /// <returns>A <see cref="DataTable" /> that describes the column metadata.</returns>
         /// <exception cref="InvalidOperationException">The <see cref="DbDataReader" /> is closed.</exception>
-        /// <exception cref="IndexOutOfRangeException">The column index is out of range.</exception>
-        /// <exception cref="NotSupportedException">.NET Core only: This member is not supported.</exception>
-        public virtual DataTable GetSchemaTable()
+        public virtual DataTable? GetSchemaTable()
         {
             throw new NotSupportedException();
         }
@@ -97,11 +98,11 @@ public virtual DataTable GetSchemaTable()
         /// </summary>
         /// <param name="cancellationToken">The cancellation instruction.</param>
         /// <returns>A task representing the asynchronous operation.</returns>
-        public virtual Task<DataTable> GetSchemaTableAsync(CancellationToken cancellationToken = default)
+        public virtual Task<DataTable?> GetSchemaTableAsync(CancellationToken cancellationToken = default)
         {
             if (cancellationToken.IsCancellationRequested)
             {
-                return Task.FromCanceled<DataTable>(cancellationToken);
+                return Task.FromCanceled<DataTable?>(cancellationToken);
             }
 
             try
@@ -110,7 +111,7 @@ public virtual Task<DataTable> GetSchemaTableAsync(CancellationToken cancellatio
             }
             catch (Exception e)
             {
-                return Task.FromException<DataTable>(e);
+                return Task.FromException<DataTable?>(e);
             }
         }
 

diff --git a/src/libraries/System.Data.Common/src/System/Data/Common/DbDataReaderExtensions.cs b/src/libraries/System.Data.Common/src/System/Data/Common/DbDataReaderExtensions.cs
@@ -73,12 +73,15 @@ public static bool CanGetColumnSchema(this DbDataReader reader)
         private static ReadOnlyCollection<DbColumn> GetColumnSchemaCompatibility(DbDataReader reader)
         {
             var columnSchema = new List<DbColumn>();
-            DataTable schemaTable = reader.GetSchemaTable();
-            DataColumnCollection schemaTableColumns = schemaTable.Columns;
-            foreach (DataRow row in schemaTable.Rows)
+            DataTable? schemaTable = reader.GetSchemaTable();
+            if (schemaTable != null)
             {
-                Debug.Assert(row != null);
-                columnSchema.Add(new DataRowDbColumn(row, schemaTableColumns));
+                DataColumnCollection schemaTableColumns = schemaTable.Columns;
+                foreach (DataRow row in schemaTable.Rows)
+                {
+                    Debug.Assert(row != null);
+                    columnSchema.Add(new DataRowDbColumn(row, schemaTableColumns));
+                }
             }
             return new ReadOnlyCollection<DbColumn>(columnSchema);
         }

diff --git a/src/libraries/System.Data.Common/src/System/Data/IDataReader.cs b/src/libraries/System.Data.Common/src/System/Data/IDataReader.cs
@@ -9,7 +9,7 @@ public interface IDataReader : IDisposable, IDataRecord
         bool IsClosed { get; }
         int RecordsAffected { get; }
         void Close();
-        DataTable GetSchemaTable();
+        DataTable? GetSchemaTable();
         bool NextResult();
         bool Read();
     }

diff --git a/src/libraries/System.Data.Common/src/System/Data/ProviderBase/DataReaderContainer.cs b/src/libraries/System.Data.Common/src/System/Data/ProviderBase/DataReaderContainer.cs
@@ -51,7 +51,7 @@ internal string GetName(int ordinal)
             Debug.Assert(null != fieldName, "null GetName");
             return ((null != fieldName) ? fieldName : "");
         }
-        internal DataTable GetSchemaTable()
+        internal DataTable? GetSchemaTable()
         {
             return _dataReader.GetSchemaTable();
         }

diff --git a/src/libraries/System.Data.Common/tests/System/Data/Common/DbDataReaderTest.cs b/src/libraries/System.Data.Common/tests/System/Data/Common/DbDataReaderTest.cs
@@ -516,7 +516,7 @@ public async Task GetSchemaTableAsync_calls_GetSchemaTable()
             var readerTable = new DataTable();
             readerTable.Columns.Add("text_col", typeof(string));
 
-            var table = await new SchemaDbDataReaderMock(readerTable).GetSchemaTableAsync();
+            var table = (await new SchemaDbDataReaderMock(readerTable).GetSchemaTableAsync())!;
 
             DataRow textColRow = table.Rows.Cast<DataRow>().Single()!;
             Assert.Equal("text_col", textColRow["ColumnName"]);

diff --git a/src/libraries/System.Data.Odbc/ref/System.Data.Odbc.cs b/src/libraries/System.Data.Odbc/ref/System.Data.Odbc.cs
@@ -180,7 +180,7 @@ protected override void Dispose(bool disposing) { }
         public override long GetInt64(int i) { throw null; }
         public override string GetName(int i) { throw null; }
         public override int GetOrdinal(string value) { throw null; }
-        public override System.Data.DataTable GetSchemaTable() { throw null; }
+        public override System.Data.DataTable? GetSchemaTable() { throw null; }
         public override string GetString(int i) { throw null; }
         public System.TimeSpan GetTime(int i) { throw null; }
         public override object GetValue(int i) { throw null; }

diff --git a/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcDataReader.cs b/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcDataReader.cs
@@ -405,12 +405,9 @@ public override string GetDataTypeName(int i)
                 DbSchemaInfo info = _dataCache.GetSchema(i);
                 if (info._typename == null)
                 {
-                    info._typename = GetColAttributeStr(i, ODBC32.SQL_DESC.TYPE_NAME, ODBC32.SQL_COLUMN.TYPE_NAME, ODBC32.HANDLER.THROW);
+                    info._typename = GetColAttributeStr(i, ODBC32.SQL_DESC.TYPE_NAME, ODBC32.SQL_COLUMN.TYPE_NAME, ODBC32.HANDLER.THROW)!;
                 }
-// TODO-NULLABLE: Behavior change probably needed here - when there's no data type, we mostly probably need to throw instead of returning null.
-#nullable disable
                 return info._typename;
-#nullable enable
             }
             throw ADP.DataReaderNoData();
         }
@@ -2245,18 +2242,15 @@ private DataTable NewSchemaTable()
         // The default values are already defined in DbSchemaRows (see DbSchemaRows.cs) so there is no need to set any default value
         //
 
-        public override DataTable GetSchemaTable()
+        public override DataTable? GetSchemaTable()
         {
             if (IsClosed)
             { // MDAC 68331
                 throw ADP.DataReaderClosed("GetSchemaTable");           // can't use closed connection
             }
             if (_noMoreResults)
             {
-// TODO-NULLABLE: Behavior change (https://github.com/dotnet/runtime/issues/509)
-#nullable disable
                 return null;                                            // no more results
-#nullable enable
             }
             if (null != _schemaTable)
             {

diff --git a/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcMetaDataFactory.cs b/src/libraries/System.Data.Odbc/src/System/Data/Odbc/OdbcMetaDataFactory.cs
@@ -1101,7 +1101,7 @@ private DataTable NewDataTableFromReader(IDataReader reader, out object[] values
         {
             DataTable resultTable = new DataTable(tableName);
             resultTable.Locale = System.Globalization.CultureInfo.InvariantCulture;
-            DataTable schemaTable = reader.GetSchemaTable();
+            DataTable schemaTable = reader.GetSchemaTable()!;
             foreach (DataRow row in schemaTable.Rows)
             {
                 resultTable.Columns.Add(row["ColumnName"] as string, (Type)row["DataType"]);

diff --git a/src/libraries/System.Data.OleDb/ref/System.Data.OleDb.cs b/src/libraries/System.Data.OleDb/ref/System.Data.OleDb.cs
@@ -204,7 +204,7 @@ public override void Close() { }
         public override long GetInt64(int ordinal) { throw null; }
         public override string GetName(int index) { throw null; }
         public override int GetOrdinal(string name) { throw null; }
-        public override System.Data.DataTable GetSchemaTable() { throw null; }
+        public override System.Data.DataTable? GetSchemaTable() { throw null; }
         public override string GetString(int ordinal) { throw null; }
         public System.TimeSpan GetTimeSpan(int ordinal) { throw null; }
         public override object GetValue(int ordinal) { throw null; }

diff --git a/src/libraries/System.Data.OleDb/src/OleDbDataReader.cs b/src/libraries/System.Data.OleDb/src/OleDbDataReader.cs
@@ -252,7 +252,7 @@ private UnsafeNativeMethods.IRow IRow()
             return irow;
         }
 
-        public override DataTable GetSchemaTable()
+        public override DataTable? GetSchemaTable()
         {
             DataTable? schemaTable = _dbSchemaTable;
             if (null == schemaTable)
@@ -273,10 +273,7 @@ public override DataTable GetSchemaTable()
                 //GetSchemaTable() is defined to return null after NextResult returns false
                 //throw ADP.DataReaderNoData();
             }
-// TODO-NULLABLE: Behavior change (https://github.com/dotnet/runtime/issues/509)
-#nullable disable
             return schemaTable;
-#nullable enable
         }
 
         internal void BuildMetaInfo()