diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/AlwaysEncryptedHelperClasses.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/AlwaysEncryptedHelperClasses.cs index 0268f59f39..5d8cda08e4 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/AlwaysEncryptedHelperClasses.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/AlwaysEncryptedHelperClasses.cs @@ -234,13 +234,9 @@ sealed internal partial class _SqlMetaDataSet internal readonly SqlTceCipherInfoTable? cekTable; // table of "column encryption keys" used for this metadataset internal _SqlMetaDataSet(int count, SqlTceCipherInfoTable? cipherTable) + : this(count) { cekTable = cipherTable; - _metaDataArray = new _SqlMetaData[count]; - for (int i = 0; i < _metaDataArray.Length; ++i) - { - _metaDataArray[i] = new _SqlMetaData(i); - } } } 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 4896d8e9cb..11fd13c0b3 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 @@ -278,8 +278,8 @@ internal virtual SmiExtendedMetaData[] GetInternalSmiMetaData() if (null != metaData && 0 < metaData.Length) { - metaDataReturn = new SmiExtendedMetaData[metaData.visibleColumns]; - + metaDataReturn = new SmiExtendedMetaData[metaData.VisibleColumnCount]; + int returnIndex = 0; for (int index = 0; index < metaData.Length; index++) { _SqlMetaData colMetaData = metaData[index]; @@ -318,7 +318,7 @@ internal virtual SmiExtendedMetaData[] GetInternalSmiMetaData() length /= ADP.CharSize; } - metaDataReturn[index] = + metaDataReturn[returnIndex] = new SmiQueryMetaData( colMetaData.type, length, @@ -346,6 +346,8 @@ internal virtual SmiExtendedMetaData[] GetInternalSmiMetaData() colMetaData.IsExpression, colMetaData.IsDifferentName, colMetaData.IsHidden); + + returnIndex += 1; } } } @@ -408,7 +410,7 @@ override public int VisibleFieldCount { return 0; } - return (md.visibleColumns); + return md.VisibleColumnCount; } } @@ -1143,31 +1145,6 @@ private bool TryConsumeMetaData() Debug.Assert(!ignored, "Parser read a row token while trying to read metadata"); } - // we hide hidden columns from the user so build an internal map - // that compacts all hidden columns from the array - if (null != _metaData) - { - if (_snapshot != null && object.ReferenceEquals(_snapshot._metadata, _metaData)) - { - _metaData = (_SqlMetaDataSet)_metaData.Clone(); - } - - _metaData.visibleColumns = 0; - - Debug.Assert(null == _metaData.indexMap, "non-null metaData indexmap"); - int[] indexMap = new int[_metaData.Length]; - for (int i = 0; i < indexMap.Length; ++i) - { - indexMap[i] = _metaData.visibleColumns; - - if (!(_metaData[i].IsHidden)) - { - _metaData.visibleColumns++; - } - } - _metaData.indexMap = indexMap; - } - return true; } @@ -2623,11 +2600,11 @@ virtual public int GetSqlValues(object[] values) SetTimeout(_defaultTimeoutMilliseconds); - int copyLen = (values.Length < _metaData.visibleColumns) ? values.Length : _metaData.visibleColumns; + int copyLen = (values.Length < _metaData.VisibleColumnCount) ? values.Length : _metaData.VisibleColumnCount; for (int i = 0; i < copyLen; i++) { - values[_metaData.indexMap[i]] = GetSqlValueInternal(i); + values[i] = GetSqlValueInternal(_metaData.GetVisibleColumnIndex(i)); } return copyLen; } @@ -2966,7 +2943,7 @@ override public int GetValues(object[] values) CheckMetaDataIsReady(); - int copyLen = (values.Length < _metaData.visibleColumns) ? values.Length : _metaData.visibleColumns; + int copyLen = (values.Length < _metaData.VisibleColumnCount) ? values.Length : _metaData.VisibleColumnCount; int maximumColumn = copyLen - 1; SetTimeout(_defaultTimeoutMilliseconds); @@ -2984,12 +2961,19 @@ override public int GetValues(object[] values) for (int i = 0; i < copyLen; i++) { // Get the usable, TypeSystem-compatible value from the internal buffer - values[_metaData.indexMap[i]] = GetValueFromSqlBufferInternal(_data[i], _metaData[i]); + int fieldIndex = _metaData.GetVisibleColumnIndex(i); + values[i] = GetValueFromSqlBufferInternal(_data[fieldIndex], _metaData[fieldIndex]); // If this is sequential access, then we need to wipe the internal buffer if ((sequentialAccess) && (i < maximumColumn)) { - _data[i].Clear(); + _data[fieldIndex].Clear(); + if (fieldIndex > i && fieldIndex>0) + { + // 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(); + } } } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs index 9070b88fa6..6c0a38e4ed 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -4435,7 +4435,6 @@ internal bool TryProcessAltMetaData(int cColumns, TdsParserStateObject stateObj, metaData = null; _SqlMetaDataSet altMetaDataSet = new _SqlMetaDataSet(cColumns, null); - int[] indexMap = new int[cColumns]; if (!stateObj.TryReadUInt16(out altMetaDataSet.id)) { @@ -4479,13 +4478,8 @@ internal bool TryProcessAltMetaData(int cColumns, TdsParserStateObject stateObj, { return false; } - - indexMap[i] = i; } - altMetaDataSet.indexMap = indexMap; - altMetaDataSet.visibleColumns = cColumns; - metaData = altMetaDataSet; return true; } diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs index 66c0966e7d..0e0913ee71 100644 --- a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs +++ b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParserHelperClasses.cs @@ -542,14 +542,17 @@ public object Clone() internal sealed partial class _SqlMetaDataSet { internal ushort id; // for altrow-columns only - internal int[] indexMap; - internal int visibleColumns; + internal DataTable schemaTable; private readonly _SqlMetaData[] _metaDataArray; internal ReadOnlyCollection dbColumnSchema; + private int _hiddenColumnCount; + private int[] _visibleColumnMap; + internal _SqlMetaDataSet(int count) { + _hiddenColumnCount = -1; _metaDataArray = new _SqlMetaData[count]; for (int i = 0; i < _metaDataArray.Length; ++i) { @@ -559,11 +562,10 @@ internal _SqlMetaDataSet(int count) private _SqlMetaDataSet(_SqlMetaDataSet original) { - this.id = original.id; - // although indexMap is not immutable, in practice it is initialized once and then passed around - this.indexMap = original.indexMap; - this.visibleColumns = original.visibleColumns; - this.dbColumnSchema = original.dbColumnSchema; + id = original.id; + _hiddenColumnCount = original._hiddenColumnCount; + _visibleColumnMap = original._visibleColumnMap; + dbColumnSchema = original.dbColumnSchema; if (original._metaDataArray == null) { _metaDataArray = null; @@ -586,6 +588,18 @@ internal int Length } } + internal int VisibleColumnCount + { + get + { + if (_hiddenColumnCount == -1) + { + SetupHiddenColumns(); + } + return Length - _hiddenColumnCount; + } + } + internal _SqlMetaData this[int index] { get @@ -599,10 +613,54 @@ internal _SqlMetaData this[int index] } } - public object Clone() + public int GetVisibleColumnIndex(int index) + { + if (_hiddenColumnCount == -1) + { + SetupHiddenColumns(); + } + if (_visibleColumnMap is null) + { + return index; + } + else + { + return _visibleColumnMap[index]; + } + } + + public _SqlMetaDataSet Clone() { return new _SqlMetaDataSet(this); } + + private void SetupHiddenColumns() + { + int hiddenColumnCount = 0; + for (int index = 0; index < Length; index++) + { + if (_metaDataArray[index].IsHidden) + { + hiddenColumnCount += 1; + } + } + + if (hiddenColumnCount > 0) + { + int[] visibleColumnMap = new int[Length - hiddenColumnCount]; + int mapIndex = 0; + for (int metaDataIndex = 0; metaDataIndex < Length; metaDataIndex++) + { + if (!_metaDataArray[metaDataIndex].IsHidden) + { + visibleColumnMap[mapIndex] = metaDataIndex; + mapIndex += 1; + } + } + _visibleColumnMap = visibleColumnMap; + } + _hiddenColumnCount = hiddenColumnCount; + } } internal sealed class _SqlMetaDataSetCollection @@ -649,10 +707,10 @@ internal _SqlMetaDataSet GetAltMetaData(int id) public object Clone() { _SqlMetaDataSetCollection result = new _SqlMetaDataSetCollection(); - result.metaDataSet = metaDataSet == null ? null : (_SqlMetaDataSet)metaDataSet.Clone(); + result.metaDataSet = metaDataSet == null ? null : metaDataSet.Clone(); foreach (_SqlMetaDataSet set in _altMetaDataSetArray) { - result._altMetaDataSetArray.Add((_SqlMetaDataSet)set.Clone()); + result._altMetaDataSetArray.Add(set.Clone()); } return result; } diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs index b7787b5855..30f88bd151 100644 --- a/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs +++ b/src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Generic; using System.Data; using System.Text; @@ -225,5 +226,81 @@ private static bool IsColumnBitSet(SqlConnection con, string selectQuery, int in } return columnSetPresent; } + + [ConditionalFact(typeof(DataTestUtility), nameof(DataTestUtility.AreConnStringsSetup))] + public static void CheckHiddenColumns() + { + // hidden columns can be found by using CommandBehavior.KeyInfo or at the sql level + // by using the FOR BROWSE option. These features return the column information requested and + // also include any key information required to be able to find the row containing the data + // requested. The additional key information is provided as hidden columns and can be seen using + // the difference between VisibleFieldCount and FieldCount on the reader + + string tempTableName = DataTestUtility.GenerateObjectName(); + + string createQuery = $@" +create table [{tempTableName}] ( + user_id int not null identity(1,1), + first_name varchar(100) null, + last_name varchar(100) null); + +alter table [{tempTableName}] add constraint pk_{tempTableName}_user_id primary key (user_id); + +insert into [{tempTableName}] (first_name,last_name) values ('Joe','Smith') +"; + + string dataQuery = $@"select first_name, last_name from [{tempTableName}]"; + + + int fieldCount = 0; + int visibleFieldCount = 0; + Type[] types = null; + string[] names = null; + + using (SqlConnection connection = new SqlConnection(DataTestUtility.TCPConnectionString)) + { + connection.Open(); + + try + { + using (SqlCommand createCommand = new SqlCommand(createQuery, connection)) + { + createCommand.ExecuteNonQuery(); + } + + using (SqlCommand queryCommand = new SqlCommand(dataQuery, connection)) + { + using (SqlDataReader reader = queryCommand.ExecuteReader(CommandBehavior.KeyInfo)) + { + fieldCount = reader.FieldCount; + visibleFieldCount = reader.VisibleFieldCount; + types = new Type[fieldCount]; + names = new string[fieldCount]; + for (int index = 0; index < fieldCount; index++) + { + types[index] = reader.GetFieldType(index); + names[index] = reader.GetName(index); + } + } + } + } + finally + { + DataTestUtility.DropTable(connection, tempTableName); + } + } + + Assert.Equal(3, fieldCount); + Assert.Equal(2, visibleFieldCount); + Assert.NotNull(types); + Assert.NotNull(names); + + // requested fields + Assert.Contains("first_name", names, StringComparer.Ordinal); + Assert.Contains("last_name", names, StringComparer.Ordinal); + + // hidden field + Assert.Contains("user_id", names, StringComparer.Ordinal); + } } }