From e631220f615c45b304884e5a0df97133f51bd282 Mon Sep 17 00:00:00 2001 From: "Dailey, Alaric" Date: Tue, 20 Oct 2026 07:31:32 -0500 Subject: [PATCH] Follow the ADO.NET contract for HasRows, RecordsAffected, OCTETS GetFieldType and RemoveAt (#1282) - HasRows reports whether the result has rows: before the first Read it fetches one row ahead, which Read then returns - RecordsAffected starts at -1 and counts only INSERT, UPDATE and DELETE, as ExecuteNonQuery already does - GetFieldType is byte[] for CHAR and VARCHAR in the OCTETS character set, the type GetValue returns - RemoveAt(int) throws ArgumentOutOfRangeException for an index outside the collection, including Count; RemoveAt(string) keeps IndexOutOfRangeException for an unknown name Co-Authored-By: Claude Opus 5.5 --- .../FbDataReaderTests.cs | 66 +++++++++++++++++++ .../FbParameterCollectionTests.cs | 24 +++++++ .../Common/DbField.cs | 6 ++ .../FirebirdClient/FbCommand.cs | 34 ++++++---- .../FirebirdClient/FbDataReader.cs | 47 +++++++++++-- .../FirebirdClient/FbParameterCollection.cs | 13 +++- 6 files changed, 167 insertions(+), 23 deletions(-) diff --git a/src/FirebirdSql.Data.FirebirdClient.Tests/FbDataReaderTests.cs b/src/FirebirdSql.Data.FirebirdClient.Tests/FbDataReaderTests.cs index c87c804bc..d4a248ba0 100644 --- a/src/FirebirdSql.Data.FirebirdClient.Tests/FbDataReaderTests.cs +++ b/src/FirebirdSql.Data.FirebirdClient.Tests/FbDataReaderTests.cs @@ -535,4 +535,70 @@ public async Task GetFieldValueTest() await transaction.RollbackAsync(); } } + + [Test] + public async Task HasRowsIsFalseForAnEmptyResultTest() + { + // DbDataReader.HasRows: "true if the DbDataReader contains one or more rows; otherwise false." + await using (var command = new FbCommand("select INT_FIELD from TEST where 1 = 0", Connection)) + { + await using (var reader = await command.ExecuteReaderAsync()) + { + Assert.IsFalse(reader.HasRows); + Assert.IsFalse(await reader.ReadAsync()); + } + } + } + + [Test] + public async Task HasRowsDoesNotSkipTheFirstRowTest() + { + await using (var command = new FbCommand("select INT_FIELD from TEST where INT_FIELD in (1, 2) order by INT_FIELD", Connection)) + { + await using (var reader = await command.ExecuteReaderAsync()) + { + Assert.IsTrue(reader.HasRows); + Assert.IsTrue(reader.HasRows); + Assert.IsTrue(await reader.ReadAsync()); + Assert.AreEqual(1, reader.GetInt32(0)); + Assert.IsTrue(reader.Read()); + Assert.AreEqual(2, reader.GetInt32(0)); + Assert.IsFalse(reader.Read()); + Assert.IsTrue(reader.HasRows, "HasRows describes the result, not the current position"); + } + } + } + + [Test] + public async Task RecordsAffectedIsMinusOneForSelectTest() + { + // DbDataReader.RecordsAffected: "-1 for SELECT statements; 0 if no rows were affected or the statement failed." + await using (var command = new FbCommand("select INT_FIELD from TEST where 1 = 0", Connection)) + { + var reader = await command.ExecuteReaderAsync(); + while (await reader.ReadAsync()) + { } + await reader.CloseAsync(); + Assert.AreEqual(-1, reader.RecordsAffected); + } + } + + [Test] + public async Task GetFieldTypeMatchesGetValueForOctetsTest() + { + // IDataRecord.GetFieldType: "the type of Object that would be returned from GetValue." + await using (var command = new FbCommand("select cast('ab' as varchar(10) character set octets), cast('ab' as char(4) character set octets) from rdb$database", Connection)) + { + await using (var reader = await command.ExecuteReaderAsync()) + { + Assert.IsTrue(await reader.ReadAsync()); + for (var i = 0; i < reader.FieldCount; i++) + { + Assert.AreEqual(typeof(byte[]), reader.GetValue(i).GetType()); + Assert.AreEqual(typeof(byte[]), reader.GetFieldType(i)); + } + } + } + } } + diff --git a/src/FirebirdSql.Data.FirebirdClient.Tests/FbParameterCollectionTests.cs b/src/FirebirdSql.Data.FirebirdClient.Tests/FbParameterCollectionTests.cs index 9fa3617e7..756772dc5 100644 --- a/src/FirebirdSql.Data.FirebirdClient.Tests/FbParameterCollectionTests.cs +++ b/src/FirebirdSql.Data.FirebirdClient.Tests/FbParameterCollectionTests.cs @@ -26,6 +26,30 @@ namespace FirebirdSql.Data.FirebirdClient.Tests; [NoServerCategory] public class FbParameterCollectionTests { + [TestCase(-1)] + [TestCase(0)] + [TestCase(1)] + public void RemoveAtInvalidIndexThrowsArgumentOutOfRangeTest(int index) + { + // IList.RemoveAt: ArgumentOutOfRangeException when "index is not a valid index in the IList". + var command = new FbCommand(); + if (index == 1) + { + command.Parameters.Add("@p1", 1); + } + + var ex = Assert.Throws(() => command.Parameters.RemoveAt(index)); + Assert.AreEqual("index", ex.ParamName); + Assert.AreEqual(index == 1 ? 1 : 0, command.Parameters.Count); + } + + [Test] + public void RemoveAtUnknownNameThrowsIndexOutOfRangeTest() + { + var command = new FbCommand(); + Assert.Throws(() => command.Parameters.RemoveAt("@missing")); + } + [Test] public void AddTest() { diff --git a/src/FirebirdSql.Data.FirebirdClient/Common/DbField.cs b/src/FirebirdSql.Data.FirebirdClient/Common/DbField.cs index 3be89be74..3579216a7 100644 --- a/src/FirebirdSql.Data.FirebirdClient/Common/DbField.cs +++ b/src/FirebirdSql.Data.FirebirdClient/Common/DbField.cs @@ -549,6 +549,12 @@ public void FixNull() public Type GetSystemType() { + // SetValue stores CHAR and VARCHAR in the OCTETS character set as byte[], so that is the type GetValue returns. + if ((SqlType == IscCodes.SQL_TEXT || SqlType == IscCodes.SQL_VARYING) && (Charset?.IsOctetsCharset ?? false) && DbDataType != DbDataType.Guid) + { + return typeof(byte[]); + } + return TypeHelper.GetTypeFromDbDataType(DbDataType); } diff --git a/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbCommand.cs b/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbCommand.cs index 551c6ea64..f70580289 100644 --- a/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbCommand.cs +++ b/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbCommand.cs @@ -263,6 +263,24 @@ internal int RecordsAffected } } + /// + /// The rows an INSERT, UPDATE or DELETE affected, or -1 for any other statement (DbCommand.ExecuteNonQuery: "For all + /// other types of statements, the return value is -1"; DbDataReader.RecordsAffected: "-1 for SELECT statements"). + /// + internal int StatementRecordsAffected + { + get + { + return _statement?.StatementType switch + { + DbStatementType.Insert => RecordsAffected, + DbStatementType.Update => RecordsAffected, + DbStatementType.Delete => RecordsAffected, + _ => -1, + }; + } + } + internal bool IsDisposed { get { return _disposed; } @@ -529,13 +547,7 @@ public override int ExecuteNonQuery() } } - return _statement.StatementType switch - { - DbStatementType.Insert => RecordsAffected, - DbStatementType.Update => RecordsAffected, - DbStatementType.Delete => RecordsAffected, - _ => -1, - }; + return StatementRecordsAffected; } public override async Task ExecuteNonQueryAsync(CancellationToken cancellationToken) { @@ -566,13 +578,7 @@ public override async Task ExecuteNonQueryAsync(CancellationToken cancellat } } - return _statement.StatementType switch - { - DbStatementType.Insert => RecordsAffected, - DbStatementType.Update => RecordsAffected, - DbStatementType.Delete => RecordsAffected, - _ => -1, - }; + return StatementRecordsAffected; } public new FbDataReader ExecuteReader() => ExecuteReader(CommandBehavior.Default); diff --git a/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbDataReader.cs b/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbDataReader.cs index 0cf1d40c4..df83c2e00 100644 --- a/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbDataReader.cs +++ b/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbDataReader.cs @@ -51,7 +51,10 @@ public sealed class FbDataReader : DbDataReader private bool _eof; private bool _isClosed; private int _position; - private int _recordsAffected; + private int _recordsAffected = -1; + private DbValue[] _peekedRow; + private bool _hasPeekedRow; + private bool _hasRows; private Dictionary _columnsIndexesOrdinal; private Dictionary _columnsIndexesOrdinalCI; @@ -102,9 +105,25 @@ public override int Depth } } + /// + /// DbDataReader.HasRows: "true if the DbDataReader contains one or more rows; otherwise false." Before the first + /// Read this fetches one row ahead; Read then returns that row. + /// public override bool HasRows { - get { return _command.HasFields; } + get + { + if (!_hasRows && !_isClosed && !_hasPeekedRow && _position == StartPosition && _command != null && _command.HasFields && !IsCommandBehavior(CommandBehavior.SchemaOnly)) + { + using (var explicitCancellation = ExplicitCancellation.Enter(CancellationToken.None, _command.Cancel)) + { + _peekedRow = _command.Fetch(); + } + _hasPeekedRow = true; + _hasRows = _peekedRow != null; + } + return _hasRows; + } } public override bool IsClosed @@ -166,6 +185,8 @@ public override void Close() _command = null; _connection = null; _row = null; + _peekedRow = null; + _hasPeekedRow = false; _schemaTable = null; _fields = null; } @@ -195,6 +216,8 @@ public override async Task CloseAsync() _command = null; _connection = null; _row = null; + _peekedRow = null; + _hasPeekedRow = false; _schemaTable = null; _fields = null; } @@ -229,10 +252,11 @@ public override bool Read() { using (var explicitCancellation = ExplicitCancellation.Enter(CancellationToken.None, _command.Cancel)) { - _row = _command.Fetch(); + _row = TakePeekedRow(out var peeked) ? peeked : _command.Fetch(); if (_row != null) { _position++; + _hasRows = true; return true; } else @@ -259,10 +283,11 @@ public override async Task ReadAsync(CancellationToken cancellationToken) { using (var explicitCancellation = ExplicitCancellation.Enter(cancellationToken, _command.Cancel)) { - _row = await _command.FetchAsync(explicitCancellation.CancellationToken).ConfigureAwait(false); + _row = TakePeekedRow(out var peeked) ? peeked : await _command.FetchAsync(explicitCancellation.CancellationToken).ConfigureAwait(false); if (_row != null) { _position++; + _hasRows = true; return true; } else @@ -1012,14 +1037,24 @@ private FbDbType GetProviderType(int i) return (FbDbType)_fields[i].DbDataType; } + private bool TakePeekedRow(out DbValue[] row) + { + row = _peekedRow; + var hadPeekedRow = _hasPeekedRow; + _peekedRow = null; + _hasPeekedRow = false; + return hadPeekedRow; + } + private void UpdateRecordsAffected() { if (_command != null && !_command.IsDisposed) { - if (_command.RecordsAffected != -1) + var recordsAffected = _command.StatementRecordsAffected; + if (recordsAffected != -1) { _recordsAffected = _recordsAffected == -1 ? 0 : _recordsAffected; - _recordsAffected += _command.RecordsAffected; + _recordsAffected += recordsAffected; } } } diff --git a/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbParameterCollection.cs b/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbParameterCollection.cs index b159fa679..1499d95ca 100644 --- a/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbParameterCollection.cs +++ b/src/FirebirdSql.Data.FirebirdClient/FirebirdClient/FbParameterCollection.cs @@ -254,9 +254,10 @@ public override void Remove(object value) public override void RemoveAt(int index) { - if (index < 0 || index > Count) + // IList.RemoveAt: ArgumentOutOfRangeException when "index is not a valid index in the IList". + if (index < 0 || index >= Count) { - throw new IndexOutOfRangeException("The specified index does not exist."); + throw new ArgumentOutOfRangeException(nameof(index), index, "The specified index does not exist."); } var parameter = this[index]; @@ -266,7 +267,13 @@ public override void RemoveAt(int index) public override void RemoveAt(string parameterName) { - RemoveAt(IndexOf(parameterName)); + var index = IndexOf(parameterName); + if (index < 0) + { + throw new IndexOutOfRangeException("The specified index does not exist."); + } + + RemoveAt(index); } public void CopyTo(FbParameter[] array, int index)