Skip to content

Make HasRows report whether rows exist - #1288

Open
alaricd wants to merge 2 commits into
FirebirdSQL:masterfrom
pengdows:fix/has-rows-contract
Open

alaricd wants to merge 2 commits into
FirebirdSQL:masterfrom
pengdows:fix/has-rows-contract

Conversation

@alaricd

@alaricd alaricd commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #1284.

Root cause

FbDataReader.HasRows returned _command.HasFields, which is true for any query with columns, including empty result sets.

Change

HasRows now fetches one row ahead before the first Read, and Read/ReadAsync return that buffered row first. Empty, closed, schema-only, and fieldless results report false. The peek is synchronous because HasRows is a synchronous property.

Contract

DbDataReader.HasRows: "true if the DbDataReader contains one or more rows; otherwise false."

Tests

  • HasRowsIsFalseForAnEmptyResultTest: fails without the fix (Assert.IsFalse(reader.HasRows) fails on the upstream code).
  • HasRowsDoesNotSkipTheFirstRowTest: guard for the peek logic. It also passes on the upstream code, because the old HasRows was true for that result.
  • Run locally against Firebird 5.0.2 (firebirdsql/firebird:5.0.2 container): all runnable variants pass. Variants that need the embedded client or a wire-encryption mismatch could not run here.

Verification gaps

  • Firebird 3.0 and 4.0 not run locally. Upstream CI covers them.
  • Embedded and CryptRequired variants not run locally.

AI disclosure

This change was prepared with AI assistance (Claude Code).

🤖 Generated with Claude Code

@cincuranet cincuranet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an explicit EXECUTE PROCEDURE command with 1CommandType.StoredProcedure, _command.Fetch()dequeues the output-parameter values into_peekedRow. If the caller checks HasRowsand then closes/disposes the reader without reading,ClosecallsSetOutputParameters`, but the statement’s queue is now empty. Output parameters remain unset; previously, closing the unread reader populated them.

@alaricd
alaricd force-pushed the fix/has-rows-contract branch from b6075f1 to ad32f38 Compare November 4, 2026 13:50
HasRows fetches one row ahead. For a stored procedure that row is the
output-parameter row, so closing the reader without calling Read left
SetOutputParameters with an empty queue and the output parameters unset.
Close and CloseAsync now pass the peeked row to SetOutputParameters.

Also drop a duplicated HasRows assertion and document why _hasPeekedRow
is kept.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@alaricd
alaricd force-pushed the fix/has-rows-contract branch from 8cf4269 to a31867d Compare November 5, 2026 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make FbDataReader.HasRows report whether rows exist

2 participants