Skip to content

Fix SQLRDD: reconnect handling, paging/EOF, delete, and write-flush data-integrity bugs - #2044

Merged
RobertvanderHulst merged 8 commits into
X-Sharp:devfrom
ecosSystem:homebase/sqlrdd-reconnect-fix
Aug 12, 2026
Merged

Fix SQLRDD: reconnect handling, paging/EOF, delete, and write-flush data-integrity bugs#2044
RobertvanderHulst merged 8 commits into
X-Sharp:devfrom
ecosSystem:homebase/sqlrdd-reconnect-fix

Conversation

@ecosSystem

@ecosSystem ecosSystem commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Eight fixes found while diagnosing severe performance and correctness problems in a
production app using SQLRDD against SQL Server, following on from #2035:

  • Auto-reconnect and surface LastException: the connection now attempts to
    reconnect automatically and exposes the underlying exception instead of failing
    silently.
  • Destructor must not force-close the shared connection: a leaked/finalized work
    area no longer force-closes the shared SqlDbConnection regardless of KeepOpen,
    which used to kill the connection for every other still-open table sharing it.
  • Locking/commit gap and GoTo/Skip/OrderKeyNo semantics: closes a window between
    locking and committing, and matches DBF's GoTo/Skip/OrderKeyNo behavior for
    records positioned outside the current order's scope/condition.
  • Stale EOF flag, scope-blind RecCount, non-seekable concatenated-key conditions:
    _hasEOF could get stuck true and leak into an unrelated position, permanently
    blocking further forward paging; RecCount ignored the current order's scope,
    corrupting page/EOF math once a recount fired; and seek/scope conditions built
    against a fully concatenated key expression ([COL1]+[COL2] LIKE 'X%') couldn't use
    a normal index. Added a column-aware condition builder that expresses a prefix value
    as a per-column AND-chain instead, enabling a real composite-index seek. Also added
    a reverse-sorted last-page fetch for GoBottom() to avoid the classic large-OFFSET
    performance cliff on big tables.
  • SetOrder must flush pending changes before switching order: SetOrder() tore
    down the row buffer before the position-restoring GoTo()'s internal flush ran, so
    the flush saw an empty phantom row instead of the real modified one and silently
    skipped the write while still reporting success. Any write followed by a
    SetOrder() before the next natural flush (a common "write a record, then restore
    the caller's original order" pattern) was silently lost.
  • Seek() must not shrink the buffer to a single row: Seek() temporarily forced
    PageSize to 1 for an unfiltered seek to keep it cheap, then restored the normal
    PageSize right after - leaving the buffer with only one row while later paging
    math still assumed a full first page. Walking forward with Skip() while a key
    still matches (seek to the first record of a key, then iterate the rest of the
    group) jumped straight to the wrong absolute offset, silently skipping every other
    row sharing that key.
  • Delete()/Recall() never queued rows for write-back: neither method added the
    row's recno to the pending-writes list GoCold() iterates, so a delete/recall with
    no other field change on the row was silently lost - GoCold() saw nothing to
    flush and never sent the UPDATE/DELETE to the server. On tables with no
    DeletedColumn this was compounded by Deleted/_UpdateRow falling back to a
    Workarea.Deleted base stub that is hardcoded FALSE, and by GoCold()'s dirty
    check only looking at DataRowState, which these methods never touch when there's
    no column to write to - so even a queued delete could be skipped entirely. Also
    removed Recall()'s call into two Workarea methods that are unimplemented stubs,
    which made recalling a row with no DeletedColumn throw every time.
  • EOF lag on forward Skip() past the last record: skipping past the last record
    fetches the (nonexistent) next page; when that came back empty, the internal
    "no more data" flag was set but the public EOF flag was not, so it only became
    visible on a second Skip() call. In between, RecNo/CurrentRow reflect the blank
    phantom row - callers that check EOF immediately after Skip() (e.g. "if eof() then
    goto(oldRecno)") miss it on the first call, then capture the phantom row's blank
    recno on the second, landing the position anywhere. Separately, _hasEOF was only
    ever set when a fetched page came back shorter than PageSize, which never happens
    when the total record count is an exact multiple of PageSize - that last (exactly
    full) page never got flagged during sequential forward paging.

Test plan

  • Verified against a real SQL Server-backed app: reconnect behavior, shared
    connection lifetime across multiple work areas, scoped/ordered browsing and
    paging (GoTop/GoBottom/Skip/GoTo) at scale, record writes outside the active
    order/index, seek-then-iterate-group patterns, delete/recall on tables with and
    without a DeletedColumn, and paging past the last record.
  • Cross-checked generated SQL directly against the server to confirm index usage
    and result correctness.

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

ecosSystem and others added 6 commits August 12, 2026 08:26
Neither the schema/metadata methods (DoesTableExist, DoesDatabaseExist,
GetTables, GetMetaDataCollections, GetMetaDataCollection) nor SqlDbCommand's
Execute*/GetSchemaTable methods ever checked whether the underlying
DbConnection was still open before using it. ForceOpen() was only ever
called once, from the SqlDbConnection constructor. If the physical
connection dropped for any reason (idle timeout, transient network error,
server-side kill) after that, every later call failed and the connection
stayed dead for the rest of the process.

- Call ForceOpen() at the top of each of those methods so a dropped
  connection is transparently reopened before use.
- LastException is now a real property backed by the existing field
  instead of two disconnected stores (a private field some methods wrote
  to directly, and a separate auto-property Command.prg wrote to), and
  its setter traces the exception via System.Diagnostics.Trace so the
  underlying cause of a connection failure is visible without requiring
  caller changes.
Root cause of intermittently losing the SQL connection while checking
tables at startup: the explicit Close() path (SQLRDD-Main.prg) correctly
calls connection:UnregisterRdd(self), which only closes the physical
connection when it's both the last registered work area AND KeepOpen is
off.

The destructor (finalizer) took a different, more aggressive path:
connection:Dispose() -> Close(), which unconditionally closes the
physical connection and deregisters it from SqlDbConnection.Connections
entirely, ignoring KeepOpen. Any work area that got left for the GC to
finalize instead of being explicitly closed - e.g. a DBWindow/Datenbank
instance opened just to inspect a table's index/schema and never closed
- would, at finalization time, force-close and deregister the shared
"DEFAULT" connection out from under every other still-open table on it.

Once deregistered, SqlDbConnection.FindByName("DEFAULT") returns null,
so every later Open() on that connection name fails immediately via
_PrepareOpen() with no exception and no LastException set - it just
silently produces a work area with fCount=0, surfacing as "table cannot
be opened" for whichever table happened to be opened next.

Fix: destructor now mirrors Close() and calls UnregisterRdd(self)
instead of Dispose(), so a leaked/finalized work area only affects the
shared connection the same way an explicit Close() would.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…antics

Pending writes were never committed: transaction-end logic that only calls
Commit() when IsLocked(0) is true never actually committed SQLRDD tables,
because IsLocked()/RLockList rely on DBI_GETLOCKARRAY/DBI_LOCKCOUNT, which
SQLRDD never implemented (locking is tracked entirely in xs_locks, not the
base RDD's own lock bookkeeping). Info() now answers both from xs_locks, so
Commit() fires when it should instead of changes sitting unflushed until an
unrelated order change/close forced a GoCold.

Lock-table cleanup had two bugs: the periodic timer was kept only in a local
variable, so it could be silently garbage-collected and simply stop firing;
and its "stale" threshold equaled its own refresh interval, leaving no margin
before a still-active lock could be judged abandoned. The timer is now kept
in a field, disposed on Close(), swept once immediately on connect (so a
crashed process's locks don't linger for a full interval), and the refresh
interval/stale threshold are separate, overridable connection settings
(SqlRDDEventReason.LockRefreshInterval/LockStaleThreshold, in seconds,
default 120/600) instead of hardcoded equal constants.

GoTo() by physical recno was fully order-dependent: it built an order-filtered
ROW_NUMBER() query and failed whenever the target record didn't satisfy the
current order's FOR-condition, even though DBF's GoTo() is a physical
operation that must succeed regardless of order. It now falls back to a
direct, order-independent fetch by recno in that case, matching DBF: the
record is found (RecNo set, BOF/EOF false) but Found is false and OrderKeyNo
(DBOI_POSITION) is 0. Skip() from that position previously used the ad-hoc
single-row buffer's stale page/row numbers, which pointed nowhere meaningful;
it now matches DBF by treating that position like BOF - a negative skip lands
on the first record of the order, a positive Skip(n) lands on record n.

Also fixes _UpdateRow crashing (NullReferenceException) instead of failing
gracefully when the record it needs to flush is no longer in the buffer, and
GoTo() discarding its actual result and always returning TRUE.
…ncatenated-key conditions

_hasEOF could get stuck true from an earlier GoBottom()/paging call and then leak into an
unrelated position (fresh Seek(), a direct GoTo(), or a jump outside the current order),
permanently blocking all further forward paging from that point. Reset it in _OpenTable(),
_GotoRecord() and _GotoRecordOutsideOrder() so each reposition determines EOF for itself
instead of inheriting stale state.

_GetRecCount() ignored the current order's scope, so any recount triggered while a scope was
active (e.g. GoCold() flushing a "hot" row) silently overwrote RecCount with the whole table's
count instead of the scoped one, corrupting the page/EOF math for the rest of the browse. It
now uses OrderKeyCount when an order is active.

GoBottom() on a large table paged via the normal ascending, OFFSET-based query, forcing SQL
Server to walk/skip almost the entire ordered result to reach the end - cost grows with table
size. Added _FetchLastPage()/BuildLastPageStatement(), which sort descending and fetch at
OFFSET 0 instead (always cheap), reversing the rows back into ascending order client-side.
Falls back to the original approach for natural/descending order or on failure.

SqlDbOrder's seek/scope conditions were always built against the fully concatenated key
expression (e.g. [COL1]+[COL2] LIKE 'X%'), which SQL Server cannot use a normal index to seek
into - it has to evaluate the concatenation per row. Added BuildColumnAwareCondition(), which
expresses a value that covers one or more leading columns as a plain AND-chain of per-column
conditions (equality for fully-covered columns, a range/prefix condition on the last partial
one), allowing a real composite-index seek. Falls back to the original concatenation-based
condition when the key has functions in it or column metadata can't be resolved.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
OrderListFocus() (SetOrder) called _CloseCursor() - which nulls out the buffer table -
before the GoTo() further down triggered its internal GoCold() flush. CurrentRow reads
that same table, so at the moment GoCold() ran it saw the empty phantom row instead of
the real modified one, treated the row as unchanged, and skipped the actual write while
still reporting success. Any write followed by a SetOrder() before the next natural flush
(the common "save a record, then restore the caller's original order/position" pattern)
was silently lost. Fixed by flushing via GoCold() before tearing down the cursor, so the
write happens while the real row is still visible.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Seek() temporarily forced PageSize to 1 before fetching, to keep an unfiltered
existence-check cheap, then restored the normal PageSize right after. That left the
resulting buffer ("page 1") holding only one row while every later paging calculation
still assumed a full-size first page. A caller that finds a match and then walks
forward with Skip() while the key still matches - the standard "seek to the first
record of a key, then Skip() through the rest of the group" idiom used throughout the
app - triggers _FetchPage() for "page 2", whose offset ((CurrentPage-1) * PageSize) is
computed against the just-restored normal PageSize instead of the single row actually
consumed. That jumps straight to absolute offset PageSize, silently skipping every
other row that shares the seek's key. Fixed by always fetching a normal, full-size
page in Seek(), removing the mismatch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@ecosSystem
ecosSystem force-pushed the homebase/sqlrdd-reconnect-fix branch from 6c4ce40 to 0093391 Compare August 12, 2026 06:28
Delete() and Recall() only touched the DeletedColumn DataColumn (when one
exists) and never added the row's recno to _updatedRecNos, the list GoCold()
iterates to decide what to write back. A pure delete/recall with no other
field change on the row was therefore silently lost: GoCold() saw nothing to
flush, so no UPDATE/DELETE statement was ever sent to the server.

For tables without a DeletedColumn this was compounded by two more gaps:
- Deleted/_UpdateRow fell back to `super:Deleted`, but Workarea.Deleted is a
  hardcoded `GET FALSE` stub with no state of its own, so a plain delete could
  never be detected even if it had been queued.
- GoCold()'s lWasHot guard only looked at DataRowState, which Delete()/Recall()
  never change when there's no DeletedColumn to write to, so the write-back
  loop was skipped entirely regardless of _updatedRecNos.
- Recall() unconditionally called super:GoTo()/super:Recall(), both
  `THROW NotImplementedException` stubs on Workarea, so recalling a row with
  no DeletedColumn always crashed.

Fixes:
- Delete()/Recall() now always register the row in _updatedRecNos and call
  GoHot(), and track deleted-without-column rows in a new _deletedRowIds set.
- New _IsRowDeleted(row) checks the DeletedColumn when present, else
  _deletedRowIds; replaces the broken super:Deleted use in _UpdateRow and
  backs the Deleted property directly instead of delegating to the base stub.
- lWasHot also fires when _updatedRecNos is non-empty.
- Recall() no longer calls into the Workarea stubs.
Two related gaps let PgDn-past-the-end land on a bogus record instead of
staying on the last row:

- SkipRaw()'s "fetch the next page" branch never called _SetEOF(TRUE) itself,
  even when that fetch turned out empty. It relied on a *subsequent* Skip()
  noticing the already-set internal _hasEOF flag, so the first Skip() past
  the end left RowNumber pointing past RowCount with the public EOF flag
  still FALSE. Callers that check EOF right after Skip() (e.g. nextrec()'s
  "if eof() then goto(oldRecno)") don't catch it until one call too late -
  and by then oldRecno was captured from the phantom row, not a real record,
  so the eventual GoTo() lands wherever that blank value happens to point.
  SkipRaw() now sets EOF immediately when the fetched page is empty.

- _FetchPage() only ever flagged _hasEOF when the fetched page came back
  shorter than PageSize. When the total record count is an exact multiple
  of PageSize, the last page is exactly full, so that check never fires
  during sequential forward paging (unlike GoBottom(), which jumps straight
  to the last page via _FetchLastPage() and flags it unconditionally). Now
  also compares the page's absolute record range against the known total.
@ecosSystem ecosSystem changed the title Fix SQLRDD: reconnect handling, order/seek paging, and write-flush data-integrity bugs Fix SQLRDD: reconnect handling, paging/EOF, delete, and write-flush data-integrity bugs Aug 12, 2026
@RobertvanderHulst
RobertvanderHulst merged commit bf9f7c7 into X-Sharp:dev Aug 12, 2026
2 checks passed
RobertvanderHulst pushed a commit that referenced this pull request Aug 13, 2026
…detection and null-DataTable crashes (#2050)

* SQLRDD: auto-reconnect and surface LastException

Neither the schema/metadata methods (DoesTableExist, DoesDatabaseExist,
GetTables, GetMetaDataCollections, GetMetaDataCollection) nor SqlDbCommand's
Execute*/GetSchemaTable methods ever checked whether the underlying
DbConnection was still open before using it. ForceOpen() was only ever
called once, from the SqlDbConnection constructor. If the physical
connection dropped for any reason (idle timeout, transient network error,
server-side kill) after that, every later call failed and the connection
stayed dead for the rest of the process.

- Call ForceOpen() at the top of each of those methods so a dropped
  connection is transparently reopened before use.
- LastException is now a real property backed by the existing field
  instead of two disconnected stores (a private field some methods wrote
  to directly, and a separate auto-property Command.prg wrote to), and
  its setter traces the exception via System.Diagnostics.Trace so the
  underlying cause of a connection failure is visible without requiring
  caller changes.

* SQLRDD: destructor must not force-close the shared connection

Root cause of intermittently losing the SQL connection while checking
tables at startup: the explicit Close() path (SQLRDD-Main.prg) correctly
calls connection:UnregisterRdd(self), which only closes the physical
connection when it's both the last registered work area AND KeepOpen is
off.

The destructor (finalizer) took a different, more aggressive path:
connection:Dispose() -> Close(), which unconditionally closes the
physical connection and deregisters it from SqlDbConnection.Connections
entirely, ignoring KeepOpen. Any work area that got left for the GC to
finalize instead of being explicitly closed - e.g. a DBWindow/Datenbank
instance opened just to inspect a table's index/schema and never closed
- would, at finalization time, force-close and deregister the shared
"DEFAULT" connection out from under every other still-open table on it.

Once deregistered, SqlDbConnection.FindByName("DEFAULT") returns null,
so every later Open() on that connection name fails immediately via
_PrepareOpen() with no exception and no LastException set - it just
silently produces a work area with fCount=0, surfacing as "table cannot
be opened" for whichever table happened to be opened next.

Fix: destructor now mirrors Close() and calls UnregisterRdd(self)
instead of Dispose(), so a leaked/finalized work area only affects the
shared connection the same way an explicit Close() would.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* SQLRDD: fix locking/commit gap and match DBF GoTo/Skip/OrderKeyNo semantics

Pending writes were never committed: transaction-end logic that only calls
Commit() when IsLocked(0) is true never actually committed SQLRDD tables,
because IsLocked()/RLockList rely on DBI_GETLOCKARRAY/DBI_LOCKCOUNT, which
SQLRDD never implemented (locking is tracked entirely in xs_locks, not the
base RDD's own lock bookkeeping). Info() now answers both from xs_locks, so
Commit() fires when it should instead of changes sitting unflushed until an
unrelated order change/close forced a GoCold.

Lock-table cleanup had two bugs: the periodic timer was kept only in a local
variable, so it could be silently garbage-collected and simply stop firing;
and its "stale" threshold equaled its own refresh interval, leaving no margin
before a still-active lock could be judged abandoned. The timer is now kept
in a field, disposed on Close(), swept once immediately on connect (so a
crashed process's locks don't linger for a full interval), and the refresh
interval/stale threshold are separate, overridable connection settings
(SqlRDDEventReason.LockRefreshInterval/LockStaleThreshold, in seconds,
default 120/600) instead of hardcoded equal constants.

GoTo() by physical recno was fully order-dependent: it built an order-filtered
ROW_NUMBER() query and failed whenever the target record didn't satisfy the
current order's FOR-condition, even though DBF's GoTo() is a physical
operation that must succeed regardless of order. It now falls back to a
direct, order-independent fetch by recno in that case, matching DBF: the
record is found (RecNo set, BOF/EOF false) but Found is false and OrderKeyNo
(DBOI_POSITION) is 0. Skip() from that position previously used the ad-hoc
single-row buffer's stale page/row numbers, which pointed nowhere meaningful;
it now matches DBF by treating that position like BOF - a negative skip lands
on the first record of the order, a positive Skip(n) lands on record n.

Also fixes _UpdateRow crashing (NullReferenceException) instead of failing
gracefully when the record it needs to flush is no longer in the buffer, and
GoTo() discarding its actual result and always returning TRUE.

* SQLRDD: fix stale EOF flag, scope-blind RecCount, and non-seekable concatenated-key conditions

_hasEOF could get stuck true from an earlier GoBottom()/paging call and then leak into an
unrelated position (fresh Seek(), a direct GoTo(), or a jump outside the current order),
permanently blocking all further forward paging from that point. Reset it in _OpenTable(),
_GotoRecord() and _GotoRecordOutsideOrder() so each reposition determines EOF for itself
instead of inheriting stale state.

_GetRecCount() ignored the current order's scope, so any recount triggered while a scope was
active (e.g. GoCold() flushing a "hot" row) silently overwrote RecCount with the whole table's
count instead of the scoped one, corrupting the page/EOF math for the rest of the browse. It
now uses OrderKeyCount when an order is active.

GoBottom() on a large table paged via the normal ascending, OFFSET-based query, forcing SQL
Server to walk/skip almost the entire ordered result to reach the end - cost grows with table
size. Added _FetchLastPage()/BuildLastPageStatement(), which sort descending and fetch at
OFFSET 0 instead (always cheap), reversing the rows back into ascending order client-side.
Falls back to the original approach for natural/descending order or on failure.

SqlDbOrder's seek/scope conditions were always built against the fully concatenated key
expression (e.g. [COL1]+[COL2] LIKE 'X%'), which SQL Server cannot use a normal index to seek
into - it has to evaluate the concatenation per row. Added BuildColumnAwareCondition(), which
expresses a value that covers one or more leading columns as a plain AND-chain of per-column
conditions (equality for fully-covered columns, a range/prefix condition on the last partial
one), allowing a real composite-index seek. Falls back to the original concatenation-based
condition when the key has functions in it or column metadata can't be resolved.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* SQLRDD: SetOrder must flush pending changes before switching order

OrderListFocus() (SetOrder) called _CloseCursor() - which nulls out the buffer table -
before the GoTo() further down triggered its internal GoCold() flush. CurrentRow reads
that same table, so at the moment GoCold() ran it saw the empty phantom row instead of
the real modified one, treated the row as unchanged, and skipped the actual write while
still reporting success. Any write followed by a SetOrder() before the next natural flush
(the common "save a record, then restore the caller's original order/position" pattern)
was silently lost. Fixed by flushing via GoCold() before tearing down the cursor, so the
write happens while the real row is still visible.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* SQLRDD: Seek() must not shrink the buffer to a single row

Seek() temporarily forced PageSize to 1 before fetching, to keep an unfiltered
existence-check cheap, then restored the normal PageSize right after. That left the
resulting buffer ("page 1") holding only one row while every later paging calculation
still assumed a full-size first page. A caller that finds a match and then walks
forward with Skip() while the key still matches - the standard "seek to the first
record of a key, then Skip() through the rest of the group" idiom used throughout the
app - triggers _FetchPage() for "page 2", whose offset ((CurrentPage-1) * PageSize) is
computed against the just-restored normal PageSize instead of the single row actually
consumed. That jumps straight to absolute offset PageSize, silently skipping every
other row that shares the seek's key. Fixed by always fetching a normal, full-size
page in Seek(), removing the mismatch.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* SQLRDD: Delete()/Recall() never queued rows for write-back

Delete() and Recall() only touched the DeletedColumn DataColumn (when one
exists) and never added the row's recno to _updatedRecNos, the list GoCold()
iterates to decide what to write back. A pure delete/recall with no other
field change on the row was therefore silently lost: GoCold() saw nothing to
flush, so no UPDATE/DELETE statement was ever sent to the server.

For tables without a DeletedColumn this was compounded by two more gaps:
- Deleted/_UpdateRow fell back to `super:Deleted`, but Workarea.Deleted is a
  hardcoded `GET FALSE` stub with no state of its own, so a plain delete could
  never be detected even if it had been queued.
- GoCold()'s lWasHot guard only looked at DataRowState, which Delete()/Recall()
  never change when there's no DeletedColumn to write to, so the write-back
  loop was skipped entirely regardless of _updatedRecNos.
- Recall() unconditionally called super:GoTo()/super:Recall(), both
  `THROW NotImplementedException` stubs on Workarea, so recalling a row with
  no DeletedColumn always crashed.

Fixes:
- Delete()/Recall() now always register the row in _updatedRecNos and call
  GoHot(), and track deleted-without-column rows in a new _deletedRowIds set.
- New _IsRowDeleted(row) checks the DeletedColumn when present, else
  _deletedRowIds; replaces the broken super:Deleted use in _UpdateRow and
  backs the Deleted property directly instead of delegating to the base stub.
- lWasHot also fires when _updatedRecNos is non-empty.
- Recall() no longer calls into the Workarea stubs.

* SQLRDD: fix EOF lag on forward Skip() past the last record

Two related gaps let PgDn-past-the-end land on a bogus record instead of
staying on the last row:

- SkipRaw()'s "fetch the next page" branch never called _SetEOF(TRUE) itself,
  even when that fetch turned out empty. It relied on a *subsequent* Skip()
  noticing the already-set internal _hasEOF flag, so the first Skip() past
  the end left RowNumber pointing past RowCount with the public EOF flag
  still FALSE. Callers that check EOF right after Skip() (e.g. nextrec()'s
  "if eof() then goto(oldRecno)") don't catch it until one call too late -
  and by then oldRecno was captured from the phantom row, not a real record,
  so the eventual GoTo() lands wherever that blank value happens to point.
  SkipRaw() now sets EOF immediately when the fetched page is empty.

- _FetchPage() only ever flagged _hasEOF when the fetched page came back
  shorter than PageSize. When the total record count is an exact multiple
  of PageSize, the last page is exactly full, so that check never fires
  during sequential forward paging (unlike GoBottom(), which jumps straight
  to the last page via _FetchLastPage() and flags it unconditionally). Now
  also compares the page's absolute record range against the known total.

* SQLRDD: fix Date vs DateTime column-type detection for SQL Server

GetColumnInfo() told DBF "D" (Date) apart from "T" (DateTime) purely by
NumericPrecision, but SQL Server's `date` type isn't numeric so ADO.NET
reports NumericPrecision as the driver's "not applicable" sentinel (255 via
System.Data.SqlClient) - the same value `datetime2` reports, so a genuine
date-only column could never be recognized as "D" and always came back as
"T" instead. Reading it back through the RDD then returned an unconverted
raw DateTime instead of a DbDate, so Date fields appeared empty in the app.

NumericScale is the reliable signal instead: a real time-bearing column
(datetime/datetime2/smalldatetime, any fractional-seconds precision) always
reports a genuine small scale (0-7), while a `date` column keeps the 255
sentinel there too. Added as an addition to the existing NumericPrecision
check in GetStructureForQuery() rather than replacing it, so any other DBMS
provider relying on the old check is unaffected.

* SQLRDD: guard against a null DataTable left behind by a failed open

_OpenTable() can fail (e.g. the underlying SELECT throws, or GetDataTable()
swallows an ADO.NET exception into Connection:LastException) and leave
DataTable null - _OpenTable() itself now detects this and raises a proper
RDD error instead of returning TRUE with no data loaded, but several call
sites downstream never checked for a null DataTable and crashed with a bare
NullReferenceException instead of failing gracefully:

- Open()'s Query-mode branch: a failed GetDataTable() left DataTable null
  for the object's entire lifetime, since _ForceOpen() is a permanent no-op
  outside Table mode and never gets a chance to retry.
- Append()/PutValue(): the return value of _ForceOpen() was discarded, so a
  stale phantom row surviving a prior _CloseCursor() let GoCold() report
  success anyway.
- Seek(): indexed DataTable:Rows:Count right after _OpenTable() with no
  check at all.
- GoTo()/GoToId(): indexed into DataTable/CurrentRow with no check.
- _ClearTable(), _GotoRow(), _UpdateRow(): same unguarded pattern.

Each now treats a null DataTable the same way the method already treats an
empty one (no rows / nothing to persist) instead of crashing.

---------

Co-authored-by: ecosSystem <ecosSystem@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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.

2 participants