Repository navigation
Conversation
…ieldType and RemoveAt (FirebirdSQL#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 <noreply@anthropic.com>
Member
|
Please create separate PRs for each issue (and separate issues to link to). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1282.
Four places where FirebirdClient departs from the documented ADO.NET contract. Each change quotes the sentence it follows:
HasRows(DbDataReader.HasRows: "true if the DbDataReader contains one or more rows; otherwise false.")It returned
_command.HasFields. Now, before the firstRead, it fetches one row and keeps it, and the nextRead/ReadAsyncreturns that row instead of fetching. After a row has been read,HasRowsstays true.SchemaOnly, a closed reader, and statements without fields still return false without fetching. Closing the reader discards the buffered row.RecordsAffected(DbDataReader.RecordsAffected: "-1 for SELECT statements; 0 if no rows were affected or the statement failed.")_recordsAffectednow starts at -1, the sentinelUpdateRecordsAffectedalready checks. The reader takes its count from a newFbCommand.StatementRecordsAffected, the INSERT/UPDATE/DELETE switch thatExecuteNonQueryandExecuteNonQueryAsyncalready used, moved into one place.GetFieldTypefor OCTETS (IDataRecord.GetFieldType: "the type of Object that would be returned from GetValue.")DbField.GetSystemTypereturnsbyte[]forCHAR/VARCHARin the OCTETS character set, matching whatDbField.SetValuestores.CHAR(16) OCTETSkeeps mapping toGuid.DbDataTypeis unchanged, so parameter handling isn't affected.RemoveAt(int)(implements IList.RemoveAt:ArgumentOutOfRangeExceptionwhen "index is not a valid index in the IList.")Now throws
ArgumentOutOfRangeException(ParamNameindex), and also forindex == Count, which the oldindex > Countcheck let through to the indexer.RemoveAt(string)keeps throwingIndexOutOfRangeExceptionfor an unknown name.Backward compatibility
HasRowsbeing true for an empty result, or onRecordsAffectedbeing0for a SELECT, sees the documented values. A text-modeEXECUTE PROCEDURErun through a reader now reports -1, asExecuteNonQueryalready did.catch (IndexOutOfRangeException)aroundParameters.RemoveAt(int)needsArgumentOutOfRangeException.HasRowscall fetches a row. That's one fetch the followingReadwould have done anyway.Tests
New tests:
FbDataReaderTests:HasRowsIsFalseForAnEmptyResultTest,HasRowsDoesNotSkipTheFirstRowTest,RecordsAffectedIsMinusOneForSelectTest,GetFieldTypeMatchesGetValueForOctetsTest.FbParameterCollectionTests:RemoveAtInvalidIndexThrowsArgumentOutOfRangeTest(-1, 0 on an empty collection,Count) andRemoveAtUnknownNameThrowsIndexOutOfRangeTest.I ran the whole
FirebirdSql.Data.FirebirdClient.Testssuite (the non-embedded fixtures) against Firebird 5.0.2 in Docker (WireCrypt = Enabled), before and after the change:The 72 failures are the same tests both times. They're environmental: backup/restore streaming, shutdown and services tests that need server-side file access this container doesn't give. I couldn't run the embedded fixtures here.
Found by the pengdows ADO.NET conformance suite, which runs the documented
System.Data.Commoncontract against every provider.AI disclosure: the analysis, the change, the tests and this description were produced with Claude Code (an AI assistant).