From f876312aaf3910c4c7e7be9e82ae488c7151f0a9 Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Mon, 10 Aug 2026 09:30:50 +0200 Subject: [PATCH 01/10] 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. --- src/Runtime/XSharp.SQLRdd/Classes/Command.prg | 4 ++ .../XSharp.SQLRdd/Classes/Connection.prg | 39 +++++++++++++------ 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/Runtime/XSharp.SQLRdd/Classes/Command.prg b/src/Runtime/XSharp.SQLRdd/Classes/Command.prg index 3c589696fb..677694f7b3 100644 --- a/src/Runtime/XSharp.SQLRdd/Classes/Command.prg +++ b/src/Runtime/XSharp.SQLRdd/Classes/Command.prg @@ -103,6 +103,7 @@ class SqlDbCommand inherit SqlDbHandleObject implements IDisposable /// method GetSchemaTable() as DataTable try + self:Connection:ForceOpen() if ! SELF:_TryBindParameters() return null endif @@ -150,6 +151,7 @@ class SqlDbCommand inherit SqlDbHandleObject implements IDisposable /// method ExecuteScalar(cTable := "" as string) as object try + self:Connection:ForceOpen() if String.IsNullOrEmpty(cTable) cTable := SELF:Name endif @@ -176,6 +178,7 @@ class SqlDbCommand inherit SqlDbHandleObject implements IDisposable /// method ExecuteReader(cTable := "" as string) as DbDataReader try + self:Connection:ForceOpen() if String.IsNullOrEmpty(cTable) cTable := SELF:Name endif @@ -202,6 +205,7 @@ class SqlDbCommand inherit SqlDbHandleObject implements IDisposable /// method ExecuteNonQuery(cTable := "" as string) as LOGIC try + self:Connection:ForceOpen() if String.IsNullOrEmpty(cTable) cTable := SELF:Name endif diff --git a/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg b/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg index d7f0d349d7..e582ec2ec5 100644 --- a/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg +++ b/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg @@ -74,7 +74,17 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable /// Provider for the Metadata, such as columnlist, maxrecords etc. property MetadataProvider as ISqlMetadataProvider auto /// Last exception that occurred in the RDD - property LastException as Exception auto get internal set + property LastException as Exception + get + return _lastException + end get + internal set + _lastException := value + if value != null + System.Diagnostics.Trace.WriteLine(String.Format("SqlDbConnection '{0}': {1}", self:Name, value:ToString())) + endif + end set + end property /// Connection State PROPERTY State as ConnectionState get iif(self:DbConnection == null, ConnectionState.Closed, self:DbConnection:State) @@ -368,7 +378,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable return false endif catch e as Exception - _lastException := e + self:LastException := e self:DbTransaction := null end try return self:DbTransaction != null @@ -388,7 +398,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable return false endif catch e as Exception - _lastException := e + self:LastException := e self:DbTransaction := null end try return self:DbTransaction != null @@ -447,7 +457,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable _command:CommandText := cCommand result := _command:ExecuteScalar(cTable) catch e as Exception - _lastException := e + self:LastException := e result := null end try return result @@ -465,7 +475,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable _command:CommandText := cCommand result := _command:ExecuteNonQuery(cTable) catch e as Exception - _lastException := e + self:LastException := e result := false end try return result @@ -483,7 +493,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable _command:CommandText := cCommand result := _command:ExecuteReader(cTable) catch e as Exception - _lastException := e + self:LastException := e result := null end try return result @@ -501,7 +511,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable _command:CommandText := cCommand result := _command:GetDataTable(cTable) catch e as Exception - _lastException := e + self:LastException := e result := null end try return result @@ -583,7 +593,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable next return oTd catch e as Exception - _lastException := e + self:LastException := e end try return null end method @@ -613,7 +623,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable self:Schema:Add(TableName, oTd) return oTd catch e as Exception - _lastException := e + self:LastException := e end try return null end method @@ -630,6 +640,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable // DROP TABLE) causes DoesTableExist() to keep reporting a just-dropped table as // existing, which then skips re-creating it. try + self:ForceOpen() if !SELF:HasCollection(TABLECOLLECTION) return false endif @@ -650,7 +661,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable next endif catch e as Exception - _lastException := e + self:LastException := e end try return false end method @@ -662,6 +673,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable /// Name of the database to check method DoesDatabaseExist(cDatabase as string) as logic try + self:ForceOpen() if !SELF:HasCollection(DATABASECOLLECTION) return false endif @@ -682,7 +694,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable next endif catch e as Exception - _lastException := e + self:LastException := e end try return false end method @@ -693,6 +705,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable /// List of table names that match the filter method GetTables(filter := "" as string) as List try + self:ForceOpen() var result := List{} if self:HasCollection(TABLECOLLECTION) var dt := self:DbConnection:GetSchema(TABLECOLLECTION) @@ -709,7 +722,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable endif return result catch e as Exception - _lastException := e + self:LastException := e end try return null end method @@ -719,6 +732,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable /// /// List of metadata collections method GetMetaDataCollections() as List + self:ForceOpen() var dt := self:DbConnection:GetSchema(DbMetaDataCollectionNames.MetaDataCollections) var result := List{} foreach row as DataRow in dt:Rows @@ -737,6 +751,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable /// /// List of metadata collections method GetMetaDataCollection(cCollection as string) as DataTable + self:ForceOpen() var dt := self:DbConnection:GetSchema(cCollection) return dt end method From 1117eedd674459c17c857417f1a050294f131fa8 Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Mon, 10 Aug 2026 09:58:44 +0200 Subject: [PATCH 02/10] 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 --- src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg index aee3b2297b..65b2bdc999 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg @@ -137,7 +137,14 @@ partial class SQLRDD destructor() Command?:Dispose() - _connection?:Dispose() + // Mirror the explicit Close() path (SQLRDD-Main.prg): unregister from the + // shared connection so a leaked/finalized work area only closes the physical + // connection when it was truly the last one AND KeepOpen is off. Calling + // Dispose() here instead used to force-close and deregister the shared + // SqlDbConnection unconditionally, ignoring KeepOpen, whenever this work area + // happened to be the last one registered at finalization time - killing the + // connection for every other still-open table on the same connection. + _connection?:UnregisterRdd(self) end destructor internal method _ClearTable() AS VOID From 22d16a89e1ee4b157dce30c119fa2eb57ee1bb3a Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Tue, 11 Aug 2026 10:38:01 +0200 Subject: [PATCH 03/10] 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. --- .../XSharp.SQLRdd/Classes/Connection.prg | 56 ++++++++++++++-- .../XSharp.SQLRdd/Metadata/Abstract.prg | 2 + src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg | 65 ++++++++++++++++++- .../XSharp.SQLRdd/RDD/SQLRDD-Orders.prg | 10 ++- .../XSharp.SQLRdd/RDD/SQLRDD-Private.prg | 37 +++++++++++ src/Runtime/XSharp.SQLRdd/Support/Enums.prg | 4 ++ .../Support/SqlDbTableCommandBuilder.prg | 22 +++++++ 7 files changed, 188 insertions(+), 8 deletions(-) diff --git a/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg b/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg index e582ec2ec5..e7bef071a4 100644 --- a/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg +++ b/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg @@ -47,6 +47,9 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable private _metadataCollections as List private _databaseRestrictions as int private _tableRestrictions as int + // Must be kept in a field: a System.Timers.Timer with no other root gets garbage + // collected (it stops firing silently), which would disable the periodic stale-lock cleanup. + private _lockTimer as System.Timers.Timer #region Connection Only Properties /// Dictionary with properties defined by the Ado.Net provider @@ -274,6 +277,8 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable BufferSize := DEFAULT_BUFFERSIZE DeletedColumn := DEFAULT_DELETEDCOLUMN RecnoColumn := DEFAULT_RECNOCOLUMN + LockRefreshInterval:= DEFAULT_LOCKREFRESHINTERVAL + LockStaleThreshold := DEFAULT_LOCKSTALETHRESHOLD cConnectionString := self:AnalyzeConnectionString(cConnectionString) self:ConnectionString := cConnectionString DbConnection := Provider:CreateConnection() @@ -294,6 +299,9 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable SELF:_FillDataSourceProperties() SELF:_CreateLockTable() SELF:InitializeLockTimer() + // Sweep stale locks (e.g. left behind by a crashed/killed previous process) right away, + // instead of waiting for the first 120-second timer tick. + SELF:LockTimerElapsedEvent(null, null) // Todo: Check for # of open users and close the connection when no users are left and then throw an exception return end constructor @@ -309,6 +317,11 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable if self:RDDs:Count > 0 return false endif + if _lockTimer != null + _lockTimer:Enabled := false + _lockTimer:Dispose() + _lockTimer := null + endif // Logout the workstation from the Open Connections table if _command != null _command:Dispose() @@ -766,6 +779,39 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable #endregion + // Interval at which a connection refreshes the timestamp on its own locks, and the age at + // which another connection's lock is considered abandoned and cleared. The threshold must + // stay comfortably above the refresh interval: with no margin, a lock could be judged + // abandoned just before its owner's own refresh tick runs, e.g. under GC/scheduler jitter, + // and a still-active user on another workstation (or another instance) would lose their lock. + // Both are overridable per connection, via ini or callback, same as PageSize/BufferSize - + // see SqlRDDEventReason.LockRefreshInterval/LockStaleThreshold and Metadata/Abstract.prg. + INTERNAL CONST DEFAULT_LOCKREFRESHINTERVAL := 120 AS INT + INTERNAL CONST DEFAULT_LOCKSTALETHRESHOLD := 600 AS INT + + /// + /// The interval (in seconds) at which this connection refreshes the timestamp on its own + /// locks and sweeps locks older than (also in seconds). + /// + property LockRefreshInterval as int + get + return _lockRefreshInterval + end get + set + _lockRefreshInterval := value + if _lockTimer != null + _lockTimer:Interval := value * 1000 + endif + end set + end property + private _lockRefreshInterval as int + + /// + /// The age (in seconds) after which another connection's lock is considered abandoned and + /// cleared. Must stay comfortably above (see remarks above). + /// + property LockStaleThreshold as int auto + INTERNAL CONST DEFAULT_ALLOWUPDATES := TRUE AS LOGIC INTERNAL CONST DEFAULT_COMPAREMEMO := TRUE AS LOGIC INTERNAL CONST DEFAULT_DELETEDCOLUMN := "" AS STRING @@ -869,10 +915,10 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable end method private method InitializeLockTimer() as void - var timer := System.Timers.Timer{120000} // 120 sec - timer:Elapsed += System.Timers.ElapsedEventHandler{ @@LockTimerElapsedEvent } - timer:AutoReset := true - timer:Enabled := true + _lockTimer := System.Timers.Timer{SELF:LockRefreshInterval * 1000} + _lockTimer:Elapsed += System.Timers.ElapsedEventHandler{ @@LockTimerElapsedEvent } + _lockTimer:AutoReset := true + _lockTimer:Enabled := true return method LockTimerElapsedEvent(sender as object, e as System.Timers.ElapsedEventArgs) as void @@ -898,7 +944,7 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable endif cmdClear:CommandText := SELF:Provider:DeleteStatement:Replace(SqlDbProvider.TableNameMacro, LockTableName):Replace(SqlDbProvider.WhereMacro, "LockDateTime < " + parameterName1) cmdClear:Parameters := List{} - cmdClear:Parameters:Add(SqlDbParameter{parameterName1, DateTime.Now.AddSeconds(-120)}) + cmdClear:Parameters:Add(SqlDbParameter{parameterName1, DateTime.Now.AddSeconds(-SELF:LockStaleThreshold)}) cmdClear:ExecuteNonQuery() return diff --git a/src/Runtime/XSharp.SQLRdd/Metadata/Abstract.prg b/src/Runtime/XSharp.SQLRdd/Metadata/Abstract.prg index b6e4de2e5a..76eebd0637 100644 --- a/src/Runtime/XSharp.SQLRdd/Metadata/Abstract.prg +++ b/src/Runtime/XSharp.SQLRdd/Metadata/Abstract.prg @@ -81,6 +81,8 @@ ABSTRACT CLASS SqlMetadataProviderAbstract IMPLEMENTS ISqlMetadataProvider _connection:AllowUpdates := SELF:GetLogic(oPar, SqlRDDEventReason.AllowUpdates, _connection:AllowUpdates) _connection:PageSize := SELF:GetInt(oPar, SqlRDDEventReason.PageSize, _connection:PageSize) _connection:BufferSize := SELF:GetInt(oPar, SqlRDDEventReason.BufferSize, _connection:BufferSize) + _connection:LockRefreshInterval := SELF:GetInt(oPar, SqlRDDEventReason.LockRefreshInterval, _connection:LockRefreshInterval) + _connection:LockStaleThreshold := SELF:GetInt(oPar, SqlRDDEventReason.LockStaleThreshold, _connection:LockStaleThreshold) _connection:RecnoColumn := SELF:GetString(oPar, SqlRDDEventReason.RecnoColumn, _connection:RecnoColumn) _connection:DeletedColumn := SELF:GetString(oPar, SqlRDDEventReason.DeletedColumn, _connection:DeletedColumn) _connection:CompareMemo := SELF:GetLogic(oPar, SqlRDDEventReason.CompareMemo, _connection:CompareMemo) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg index 76d20fe292..5da6ae156a 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg @@ -488,10 +488,53 @@ partial class SQLRDD inherit Workarea return false elseif uiOrdinal == DbInfo.DBI_ISDBF return false + elseif uiOrdinal == DbInfo.DBI_GETLOCKARRAY + return SELF:_GetMyLockedRecords() + elseif uiOrdinal == DbInfo.DBI_LOCKCOUNT + return (int) SELF:_GetMyLockedRecords():Length endif return super:Info(uiOrdinal, oNewValue) end method + /// + /// Records currently locked by this connection in this table, per the xs_locks table. + /// + /// + /// Backs DBI_GETLOCKARRAY/DBI_LOCKCOUNT (and therefore VO's RLockList/IsLocked()), since + /// SQLRDD's locking is tracked entirely in xs_locks rather than in the base RDD's own + /// bookkeeping. Without this, IsLocked() always reports false for SQLRDD tables, and + /// callers relying on it (e.g. HomeBase's transaction end) never commit pending changes. + /// + private method _GetMyLockedRecords() as DWORD[] + var recNos := List{} + if Connection?:Provider is null .or. !self:Connection:IsOpen .or. _oTd == null + return recNos:ToArray() + endif + try + var sb := StringBuilder{} + sb:AppendLine("select recno from " + SqlDbConnection.LockTableName) + sb:AppendLine("where tablename = "+self:Provider:ParameterPrefix+"p1") + sb:AppendLine(" and connectionid = "+self:Provider:ParameterPrefix+"p2") + sb:AppendLine(" and workarea = "+self:Provider:ParameterPrefix+"p3") + + using var cmd := SqlDbCommand{"GetLockArray", self:Connection, false} + cmd:CommandText := sb:ToString() + cmd:AddParameter(self:Provider:ParameterPrefix+"p1", _oTd:RealName) + cmd:AddParameter(self:Provider:ParameterPrefix+"p2", self:Connection:ConnectionId:ToString()) + cmd:AddParameter(self:Provider:ParameterPrefix+"p3", (int)super:Area) + + using var reader := cmd:ExecuteReader() + do while reader:Read() + var recNoTemp := (int) reader["recno"] + if recNoTemp > 0 + recNos:Add((DWORD) recNoTemp) + endif + end do + catch + nop + end try + return recNos:ToArray() + end method /// /// @@ -501,6 +544,7 @@ partial class SQLRDD inherit Workarea if !self:_ForceOpen() return false endif + SELF:_outsideOrder := FALSE SELF:_ClearTable() SELF:_FetchPage( 1) SELF:RowNumber := 1 @@ -525,6 +569,7 @@ partial class SQLRDD inherit Workarea if !self:_ForceOpen() return false endif + SELF:_outsideOrder := FALSE SELF:_ClearTable() local nMaxRecNo as dword if self:CurrentOrder = Null @@ -622,6 +667,7 @@ partial class SQLRDD inherit Workarea ENDIF // Normal positioning, VO resets FOUND to FALSE after a recprd movement SELF:_Found := FALSE + SELF:_outsideOrder := FALSE IF SELF:_tableMode == TableMode.Query .and. self:_recnoColumNo == -1 RETURN SELF:_GotoRow((LONG) nRec) ENDIF @@ -638,9 +684,9 @@ partial class SQLRDD inherit Workarea rowIndex++ next - SELF:_GotoRecord(nRec) + var found := SELF:_GotoRecord(nRec) SELF:_CheckEofBof() - RETURN TRUE + RETURN found end method @@ -678,6 +724,21 @@ partial class SQLRDD inherit Workarea SELF:_Bottom := FALSE IF nToSkip == 0 result := SELF:GoCold() + ELSEIF SELF:_outsideOrder + // Positioned (via GoTo()) on a record outside the current order (OrderKeyNo 0). + // RowNumber/_currentPageNo do not correspond to any real position in the order's + // sequence, so a relative skip from here is meaningless. Matching DBF: a negative + // skip always lands on the first record of the order; a positive skip lands on + // record nToSkip, as if skipping forward from just before the top. + SELF:GoCold() + result := SELF:GoTop() + IF result + IF nToSkip < 0 + SELF:BoF := TRUE + ELSEIF nToSkip > 1 + result := SELF:Skip(nToSkip - 1) + ENDIF + ENDIF ELSE result := SELF:SkipRaw( nToSkip ) if result diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg index 5989d8e71a..09b27cbad3 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg @@ -441,7 +441,15 @@ partial class SQLRDD self:_ForceOpen() info:Result := self:OrderKeyCount case DBOI_POSITION - info:Result := self:RowNumber + (self:_currentPageNo-1) * self:_oTd:PageSize + // OrdKeyNo()/DBOI_POSITION reports the record's position within the current order. + // When the cursor sits on a record outside the order (see GoTo()/_outsideOrder), + // RowNumber/_currentPageNo just reflect the ad-hoc single-row buffer we loaded for + // it, not a real position - matching DBF, that must report 0, not a bogus row number. + if self:_outsideOrder + info:Result := 0 + else + info:Result := self:RowNumber + (self:_currentPageNo-1) * self:_oTd:PageSize + endif case DBOI_RECNO // our position is the row number in the local cursor info:Result := self:RowNumber + (self:_currentPageNo-1) * self:_oTd:PageSize diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg index 65b2bdc999..ff5ff13cf4 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg @@ -56,6 +56,13 @@ partial class SQLRDD private _numHiddenColumns as long private _serverReccount as dword + /// + /// TRUE when the cursor is positioned (via GoTo()) on a record that physically exists but + /// does not satisfy the current order's FOR-condition/scope, i.e. OrderKeyNo is 0. Skip() + /// needs this because RowNumber/_currentPageNo no longer correspond to any real position in + /// the order's sequence, so the normal relative-skip logic cannot be used from here. + /// + private _outsideOrder as logic #region Properties internal property Connection as SqlDbConnection get _connection @@ -620,6 +627,13 @@ partial class SQLRDD // the page that actually contains nRec, so this brute walk must always run. SELF:_command:CommandText := _builder:BuildRowNumberStatement(nRec) var result := SELF:_command:ExecuteScalar(SELF:_oTd:Name) + if result == null .or. result == DBNull.Value + // nRec does not satisfy the current order's FOR-condition/scope. DBF's GoTo() is a + // physical positioning operation, independent of the active order: it must still + // succeed when the record exists at all - Found/OrderKeyNo separately reflect that + // it has no valid position in this order. + return SELF:_GotoRecordOutsideOrder(nRec) + endif var iResult := Convert.ToInt64(result) // determine correct page @@ -637,6 +651,24 @@ partial class SQLRDD ENDDO RETURN FALSE + PRIVATE METHOD _GotoRecordOutsideOrder(nRec as DWORD) AS LOGIC + try + SELF:_command:CommandText := _builder:BuildDirectRecnoStatement(nRec) + SELF:_command:ClearParameters() + var oTable := SELF:_command:GetDataTable(SELF:Alias) + if oTable == null .or. oTable:Rows:Count == 0 + // Does not exist even physically. + return false + endif + SELF:_ClearTable() + SELF:DataTable := oTable + SELF:RowNumber := 1 + SELF:_outsideOrder := true + return true + catch as Exception + return false + end try + PRIVATE METHOD _GotoRow(nRow as LONG) AS LOGIC SELF:_Found := FALSE var nCount := SELF:DataTable:Rows:Count @@ -671,6 +703,11 @@ partial class SQLRDD endif next + if row == null + self:_dbfError(ERDD.WRITE, XSharp.Gencode.EG_WRITE, "SqlRDD:GoCold", "Record "+nRecNo:ToString()+" no longer in buffer, cannot save changes" ) + return false + endif + // Check row lock var dbLockInfo := DbLockInfo{} dbLockInfo:RecId := row[_oTd:RecnoColumn] diff --git a/src/Runtime/XSharp.SQLRdd/Support/Enums.prg b/src/Runtime/XSharp.SQLRdd/Support/Enums.prg index 479d7f2734..96414f5466 100644 --- a/src/Runtime/XSharp.SQLRdd/Support/Enums.prg +++ b/src/Runtime/XSharp.SQLRdd/Support/Enums.prg @@ -76,6 +76,10 @@ enum SqlRDDEventReason member TagName /// Retrieve Buffersize in pages member BufferSize +/// Specifies the interval (in seconds) at which a connection refreshes its own locks and sweeps stale ones + member LockRefreshInterval +/// Specifies the age (in seconds) after which another connection's lock is considered abandoned and cleared + member LockStaleThreshold end enum diff --git a/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg b/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg index 937ae5632a..ccf0534517 100644 --- a/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg +++ b/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg @@ -246,6 +246,28 @@ internal class SqlDbTableCommandBuilder sb:Replace(SqlDbProvider.WhereMacro, nRec:ToString()) return sb:ToString() + /// + /// Build a fetch of a single record by its physical recno, ignoring the current order's + /// FOR-condition/scope and any filter. + /// + /// + /// DBF's GoTo() is a physical positioning operation: it must succeed even for a record that + /// does not match the active order, unlike BuildRowNumberStatement/BuildSqlStatement which + /// always fold the order's condition into the query. Found and OrderKeyNo separately reflect + /// that the record has no valid position in the current order. + /// + method BuildDirectRecnoStatement(nRec as DWORD) as string + var sb := System.Text.StringBuilder{} + sb:Append(SqlDbProvider.SelectClause) + sb:Append(self:ColumnList()) + sb:Append(SqlDbProvider.FromClause) + sb:Append(Provider:QuoteIdentifier(self:_oTable:RealName)) + sb:Append(SqlDbProvider.WhereClause) + sb:Append(Provider:QuoteIdentifier(self:_oTable:RecnoColumn)) + sb:Append(" = ") + sb:Append(nRec:ToString()) + return sb:ToString() + method ColumnList() as string var sb := StringBuilder{} From 40e77da7e0d8006b104c7e84b478ab883607b8fe Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Tue, 11 Aug 2026 14:40:22 +0200 Subject: [PATCH 04/10] 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 --- src/Runtime/XSharp.SQLRdd/RDD/SQLDbOrder.prg | 76 +++++++++++++++++++ src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg | 15 +++- .../XSharp.SQLRdd/RDD/SQLRDD-Private.prg | 59 +++++++++++++- .../Support/SqlDbTableCommandBuilder.prg | 72 ++++++++++++++++++ 4 files changed, 219 insertions(+), 3 deletions(-) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLDbOrder.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLDbOrder.prg index 69ef435396..320fedadc7 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLDbOrder.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLDbOrder.prg @@ -258,6 +258,15 @@ internal class SqlDbOrder inherit SqlDbObject if seekInfo:Value is string var strValue var strLen := strValue:Length if strLen < self:KeyLength + var cColumnCondition := SELF:BuildColumnAwareCondition(strValue, seekInfo:SoftSeek, cComp) + if cColumnCondition != null + return cColumnCondition + endif + // Fallback for keys we can't map onto individual columns (functions in the key + // expression, unknown column metadata, ...): the original, always-correct + // condition against the fully concatenated SQLKey expression. SQL Server cannot + // use a normal index to seek into this - it has to compute the concatenation for + // every row - so prefer the column-aware path above whenever possible. if (! seekInfo:SoftSeek) cComp := " like " strValue += "%" @@ -278,5 +287,72 @@ internal class SqlDbOrder inherit SqlDbObject return cWhereClause end method + /// + /// Build a seek/scope condition against the individual columns that make up a plain + /// concatenated key (e.g. ALORT+NAMEUMLAUT), instead of against the fully concatenated + /// SQLKey expression. + /// + /// + /// A condition like `[ALORT]+[NAMEUMLAUT] LIKE 'X%'` cannot use a normal index on + /// (ALORT, NAMEUMLAUT, ...) - SQL Server has to evaluate the concatenation for every row. + /// A value that covers one or more leading columns exactly can instead be expressed as + /// `[ALORT] = 'X' AND [NAMEUMLAUT] LIKE 'Y%'`, which is a normal composite-index seek. + /// Returns NULL when the key has functions in it, or column metadata/widths can't be + /// determined - callers must fall back to the concatenation-based condition in that case. + /// + private method BuildColumnAwareCondition(strValue as string, lSoftSeek as logic, cRangeComp as string) as string + if self:HasFunctions + return null + endif + var columns := self:ColumnList + if columns == null .or. columns:Count == 0 + return null + endif + var widths := List{} + foreach var cCol in columns + var cName := cCol:Trim({'[',']','"','`'}) + var oCol := self:RDD:TableColumns:FirstOrDefault({ c => String.Compare(c:Name, cName, true) == 0 }) + if oCol == null + return null + endif + widths:Add((int) oCol:Length) + next + + var sb := StringBuilder{} + var cRemaining := strValue + for var i := 0 upto columns:Count-1 + if cRemaining:Length == 0 + exit + endif + var nWidth := widths[i] + var cColExpr := columns[i] + if sb:Length > 0 + sb:Append(SqlDbProvider.AndClause) + endif + if cRemaining:Length >= nWidth + // The seek value fully covers this column - pin it down with equality and + // carry on with whatever's left over into the next column. + var cPart := cRemaining:Substring(0, nWidth) + sb:Append(cColExpr + " = " + Functions.XsValueToSqlValue(cPart)) + cRemaining := cRemaining:Substring(nWidth) + else + // Partial match on this column - the same range/prefix logic the concatenation + // fallback uses, just scoped to this one column so the index can still be used. + if !lSoftSeek + sb:Append(cColExpr + " like " + Functions.XsValueToSqlValue(cRemaining + "%")) + else + var cSubstr := Provider:GetFunction("SUBSTR(%1%,%2%,%3%)") + cSubstr := cSubstr:Replace("%1%", cColExpr):Replace("%2%","1"):Replace("%3%", cRemaining:Length:ToString()) + sb:Append(cSubstr + cRangeComp + Functions.XsValueToSqlValue(cRemaining)) + endif + cRemaining := "" + endif + next + if sb:Length == 0 + return null + endif + return sb:ToString() + end method + end class end namespace // XSharp.RDD.SqlRDD diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg index 5da6ae156a..065518910b 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg @@ -581,7 +581,19 @@ partial class SQLRDD inherit Workarea if SELF:RecCount % self:_oTd:PageSize != 0 nPage += 1 ENDIF - SELF:_FetchPage((INT) nPage) + // For a large table, fetching the last page via the normal ascending, OFFSET-based + // query (_FetchPage) forces the server to walk/skip almost the entire ordered result - + // the further from the top, the worse. _FetchLastPage avoids that by sorting in + // reverse and asking for OFFSET 0 instead, which is always cheap. Only safe when the + // order isn't already descending (that would invert the reversal); falls back to the + // original approach for natural order, descending orders, or if it fails outright. + if self:CurrentOrder == null .or. !self:CurrentOrder:Descending + if !SELF:_FetchLastPage((INT) nPage) + SELF:_FetchPage((INT) nPage) + endif + else + SELF:_FetchPage((INT) nPage) + endif SELF:RowNumber := SELF:RowCount SELF:_Top := FALSE SELF:_Bottom := TRUE @@ -603,7 +615,6 @@ partial class SQLRDD inherit Workarea return false endif LOCAL isOK := TRUE AS LOGIC - // SELF:GoCold() IF nToSkip == 0 NOP diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg index ff5ff13cf4..f2ac29999b 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg @@ -446,6 +446,42 @@ partial class SQLRDD return true end method + /// + /// Fetch the last page of the current order/scope/filter directly, without going through + /// the ascending, huge-OFFSET query GoBottom() would otherwise need for a large table. + /// + /// See SqlDbTableCommandBuilder.BuildLastPageStatement for why this exists. + private method _FetchLastPage(nPage as int) as logic + try + SELF:_command:CommandText := _builder:BuildLastPageStatement() + SELF:_command:ClearParameters() + var newTable := SELF:_command:GetDataTable(SELF:Alias) + if newTable == null + return false + endif + // The query is sorted DESCENDING (to avoid the large OFFSET) - insert forwards at + // position 0 so the rows end up in the normal ascending order the rest of the RDD + // (Skip, RecNo lookups, ...) expects from the buffer. + for var nRow := 0 upto newTable:Rows:Count-1 + var row := newTable:Rows[nRow] + var newRow := SELF:DataTable:NewRow() + newRow:ItemArray := row:ItemArray + SELF:DataTable:Rows:InsertAt(newRow, 0) + newRow:AcceptChanges() + next + SELF:_currentPageNo := nPage + SELF:_firstPageNo := nPage + // This IS the last page by definition - without this, a Skip(1) right after + // GoBottom() (e.g. GoTo(0)'s GoBottom()+Skip(1)) would not know it's already at + // the end, and would fall through to fetching "the next page" via the normal + // ascending, huge-OFFSET query - the exact cost this method exists to avoid. + SELF:_hasEOF := true + return true + catch as Exception + return false + end try + end method + protected method _ForceOpen() as logic if self:_tableMode != TableMode.Table return true @@ -473,6 +509,11 @@ partial class SQLRDD try SELF:_currentPageNo := 1 SELF:_firstPageNo := 1 + // A fresh open/reposition (e.g. from Seek()) must not inherit a stale "no more + // rows" flag left over from whatever this buffer was doing before - otherwise + // Skip() refuses to fetch the next page and reports EOF even though the new + // WHERE clause has plenty more rows past the first page. + SELF:_hasEOF := false SELF:DataTable := self:_ReadTable(sWhereClause) self:_GetRecCount() catch as Exception @@ -556,7 +597,16 @@ partial class SQLRDD return private method _GetRecCount() as void - self:_serverReccount := self:_builder:GetRecCount() + // Must respect the current order's scope/condition, same as GoBottom() already does - + // otherwise a scope/seek-scoped browse (e.g. one city's streets) gets its RecCount + // silently overwritten with the whole unscoped table's count the moment anything + // triggers a recount (GoCold() does, on every flush of a "hot" row), corrupting the + // page/EOF math for the rest of the browse. + if self:CurrentOrder == null + self:_serverReccount := self:_builder:GetRecCount() + else + self:_serverReccount := self:OrderKeyCount + endif end method private method _FetchPage(nNewPageNo as int ) as logic @@ -638,6 +688,12 @@ partial class SQLRDD // determine correct page SELF:_currentPageNo := (INT) ((iResult - 1) / SELF:_oTd:PageSize) + 1 + // This is a freshly loaded page - whether it happens to be the last one needs to be + // re-determined from here, not inherited from whatever a previous, unrelated GoBottom() + // (e.g. on a completely different page) left behind. Without this, a stale _hasEOF=true + // makes every subsequent forward Skip() from this page falsely believe it's already at + // the end and never fetch the next page. + SELF:_hasEOF := false SELF:_ClearTable() SELF:DataTable := SELF:_ReadTable("") @@ -664,6 +720,7 @@ partial class SQLRDD SELF:DataTable := oTable SELF:RowNumber := 1 SELF:_outsideOrder := true + SELF:_hasEOF := false return true catch as Exception return false diff --git a/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg b/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg index ccf0534517..a082f9f303 100644 --- a/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg +++ b/src/Runtime/XSharp.SQLRdd/Support/SqlDbTableCommandBuilder.prg @@ -194,6 +194,78 @@ internal class SqlDbTableCommandBuilder return sb:ToString() + /// + /// Build a fetch of the last page of the current order/scope/filter, without the classic + /// "large OFFSET" performance problem. + /// + /// + /// GoBottom() needs the last PageSize rows of a potentially huge result set. The normal + /// paging query (BuildSqlStatement) would ask for that via `ORDER BY ... OFFSET (millions) + /// ROWS FETCH NEXT PageSize ROWS`, which forces SQL Server to walk/skip almost the entire + /// ordered result before it can return anything - the cost grows with table size even + /// though the caller only wants a handful of rows. Sorting in reverse and asking for + /// OFFSET 0 is always cheap regardless of table size; the caller is responsible for + /// reversing the returned rows back into ascending order. + /// + method BuildLastPageStatement() as string + var sb := System.Text.StringBuilder{} + local scopeWhere := null as string + var CurrentOrder := _oRdd:CurrentOrder + var whereClauses := List{} + sb:Append(SqlDbProvider.SelectClause) + sb:Append(self:ColumnList()) + sb:Append(SqlDbProvider.FromClause) + sb:Append(Provider:QuoteIdentifier(self:_oTable:RealName)) + if CurrentOrder != null + scopeWhere := CurrentOrder:GetScopeClause() + if ! String.IsNullOrEmpty(CurrentOrder:SqlWhere) + whereClauses:Add(CurrentOrder:SqlWhere) + endif + endif + if ! String.IsNullOrEmpty(scopeWhere) + whereClauses:Add(scopeWhere) + endif + if SELF:_oTable:HasServerFilter + whereClauses:Add(_oTable:ServerFilter) + endif + var sWhereClause := self:CombineWhereClauses(whereClauses) + sWhereClause := _connection:RaiseStringEvent(_connection, SqlRDDEventReason.WhereClause, _cTable, sWhereClause) + if ! String.IsNullOrEmpty(sWhereClause) + sb:Append(SqlDbProvider.WhereClause) + sb:Append(sWhereClause) + endif + local cOrderby := "" AS string + if CurrentOrder != null + sb:Append(Provider.OrderByClause) + foreach var cCol in CurrentOrder:OrderList + if !String.IsNullOrEmpty(cOrderby) + cOrderby += ", " + endif + cOrderby += cCol + " DESC" + next + if SELF:_oTable:HasRecnoColumn + var cRecnoCol := Provider:QuoteIdentifier(self:_oTable:RecnoColumn) + if ! String.IsNullOrEmpty(cOrderby) + if !cOrderby:Contains(cRecnoCol) + cOrderby := cOrderby + ", " + cRecnoCol + " DESC" + endif + else + cOrderby := cRecnoCol + " DESC" + endif + endif + elseif SELF:_oTable:HasRecnoColumn + sb:Append(Provider:OrderByClause) + cOrderby := Provider:QuoteIdentifier(self:_oTable:RecnoColumn) + " DESC" + endif + cOrderby :=_connection:RaiseStringEvent(_connection, SqlRDDEventReason.OrderByClause, _cTable, cOrderby) + sb:Replace(SqlDbProvider.ColumnsMacro, cOrderby) + + sb:Append(Provider:PagingClause) + sb:Replace(SqlDbProvider.PagesizeMacro, _oTable:PageSize:ToString()) + sb:Replace(SqlDbProvider.StartRecMacro, "0") + + return sb:ToString() + METHOD BuildRowNumberStatement(nRec as DWORD) AS STRING var sb := System.Text.StringBuilder{} From d46d9e8effcb36904107860a16a6ed2dbe00022d Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Tue, 11 Aug 2026 15:15:33 +0200 Subject: [PATCH 05/10] 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 --- src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg index 09b27cbad3..fb59dbbf74 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg @@ -168,6 +168,14 @@ partial class SQLRDD local result := false as logic if self:_tableMode == TableMode.Table var currentRecord := SELF:RecNo + // Flush any pending field changes on the CURRENTLY loaded row/order BEFORE + // tearing down the cursor. _CloseCursor() below nulls out the table the + // CurrentRow property reads from, so GoCold() called any later (e.g. via the + // GoTo() further down) would see the phantom row instead of the real dirty one + // and silently report success without writing anything - a change made via + // SetOrder()/RestDB() around a write (the standard "write outside the active + // index/order" pattern used throughout the app) would be lost. + SELF:GoCold() self:_CloseCursor() if (orderInfo:Order is long var nOrder0 .and. nOrder0 == 0) .or. ; (orderInfo:Order is string var cOrder0 .and. String.IsNullOrEmpty(cOrder0)) From 009339115ccde360a51ee43f364f593de666ea3c Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Tue, 11 Aug 2026 15:35:10 +0200 Subject: [PATCH 06/10] 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 --- src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg index fb59dbbf74..6123f471aa 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg @@ -496,17 +496,14 @@ partial class SQLRDD SELF:_ClearTable() SELF:_currentPageNo := 1 - // save PageSize - var nPageSize := SELF:_oTd:PageSize - // When a filter is active we may need to skip past several candidate - // keys to find one that also satisfies the filter, so we need more - // than a single row in the buffer. Only shrink the page to 1 row - // when there is no filter to evaluate. - if ! SELF:_FilterInfo:Active - SELF:_oTd:PageSize := 1 - endif + // Fetch a normal, full-size page here - NOT a single-row buffer. _FetchPage()'s + // paging math ((CurrentPage-1) * PageSize) assumes every page, including this first + // one, holds a full PageSize worth of rows; a caller that finds a match and then + // walks forward with Skip() past this buffer (the common "seek to the first record + // of a key, then Skip() while the key still matches" idiom) would otherwise jump + // straight to absolute offset PageSize on the next fetch instead of to row 2, + // silently skipping every other row that shares this seek's key. self:_OpenTable(cSeekWhere) - SELF:_oTd:PageSize := nPageSize IF SELF:DataTable:Rows:Count = 0 .and. !seekInfo.SoftSeek SELF:GoTo(0) From 618f0353ee0e166d2e4b56d7bd35b394b770c20e Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Wed, 12 Aug 2026 09:48:32 +0200 Subject: [PATCH 07/10] 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. --- src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg | 64 +++++++++++++------ .../XSharp.SQLRdd/RDD/SQLRDD-Private.prg | 6 +- 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg index 065518910b..a1a103cddb 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg @@ -389,7 +389,9 @@ partial class SQLRDD inherit Workarea if current == null return false endif - var lWasHot := current:RowState != DataRowState.Unchanged + // RowState alone misses rows only marked via Delete()/Recall() without a DeletedColumn, + // since those touch no DataColumn and leave RowState Unchanged. + var lWasHot := current:RowState != DataRowState.Unchanged .or. _updatedRecNos:Count > 0 local lOk := TRUE as logic if lWasHot .and. self:DataTable != null @@ -441,6 +443,29 @@ partial class SQLRDD inherit Workarea return lOk end method + /// Is the given row marked for deletion? + /// + /// When a DeletedColumn is defined, then the value of that column is checked. + /// Otherwise the row's recno is looked up in the internal _deletedRowIds set, + /// since Workarea.Deleted is a hardcoded stub that cannot track this state. + /// + private method _IsRowDeleted(row as DataRow) as logic + if self:_deletedColumnNo > -1 + var res := row[self:_deletedColumnNo] + if res is logic + return (logic) res + else + try + var iRes := Convert.ToInt64(res) + return iRes != 0 + catch + return false + end try + endif + endif + return self:_deletedRowIds:Contains((int)row[self:_recnoColumNo]) + end method + /// Mark the row at the current cursor position for deletion. /// /// @@ -449,14 +474,20 @@ partial class SQLRDD inherit Workarea /// override method Delete() as logic + var row := self:CurrentRow if self:_deletedColumnNo > -1 - var row := self:CurrentRow if self:_deletedColumnIsLogic row[_deletedColumnNo] := true else row[_deletedColumnNo] := 1 endif + else + self:_deletedRowIds:Add((int)row[self:_recnoColumNo]) endif + if !_updatedRecNos:Contains((int)row[self:_recnoColumNo]) + _updatedRecNos:Add((int)row[self:_recnoColumNo]) + endif + self:GoHot() return true end method @@ -467,17 +498,21 @@ partial class SQLRDD inherit Workarea /// Otherwise when the current row is deleted and not persisted to the server yet, then the deletion is undone. /// override method Recall() as logic + var row := self:CurrentRow if self:_deletedColumnNo >= 0 - var row := self:CurrentRow if self:_deletedColumnIsLogic row[_deletedColumnNo] := false else row[_deletedColumnNo] := 0 endif + else + self:_deletedRowIds:Remove((int)row[self:_recnoColumNo]) endif - // Must position the DBF on the right row for the recall - super:GoTo((DWORD) SELF:RowNumber) - return super:Recall() + if !_updatedRecNos:Contains((int)row[self:_recnoColumNo]) + _updatedRecNos:Add((int)row[self:_recnoColumNo]) + endif + self:GoHot() + return true end method /// Retrieve and optionally change information about a work area. @@ -782,21 +817,10 @@ partial class SQLRDD inherit Workarea /// override property Deleted as logic get - if self:_deletedColumnNo > 0 - var res:= CurrentRow[self:_deletedColumnNo] - if res is logic - return (logic) res - else - try - var iRes := Convert.ToInt64(res) - return iRes != 0 - catch - return false - end try - endif - else - return super:Deleted + if self:CurrentRow == null + return false endif + return self:_IsRowDeleted(self:CurrentRow) end get end property diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg index f2ac29999b..e5435dc4dd 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg @@ -40,6 +40,9 @@ partial class SQLRDD private _updatableColumns as List private _keyColumns as List private _updatedRecNos as List + /// Recnos of rows marked for deletion via Delete() when there is no DeletedColumn on the table. + /// Workarea.Deleted is a hardcoded stub (always FALSE), so this state cannot be tracked in the base class. + private _deletedRowIds as HashSet private _orderBagList as List private _rowNumber as long @@ -135,6 +138,7 @@ partial class SQLRDD SELF:_firstPageNo := 1 self:_trimValues := true // trim String Valuess SELF:_updatedRecNos := List{} + SELF:_deletedRowIds := HashSet{} SELF:_keyColumns := List{} SELF:_updatableColumns := List{} SELF:_orderBagList := List{} @@ -780,7 +784,7 @@ partial class SQLRDD endif lOk := true - if super:Deleted + if self:_IsRowDeleted(row) local wasNew := false as logic // Append from may add deleted rows if row:RowState.HasFlag(DataRowState.Added) From d64e8161f1661bdb3c885ddba1622388cbd26bda Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Wed, 12 Aug 2026 10:10:19 +0200 Subject: [PATCH 08/10] 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. --- src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg | 11 +++++++++++ src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg | 10 +++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg index a1a103cddb..ce38474003 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg @@ -665,6 +665,17 @@ partial class SQLRDD inherit Workarea ELSE SELF:RowNumber := newRow SELF:_FetchPage(SELF:_currentPageNo +1) + if SELF:RowNumber > SELF:RowCount + // The page we just fetched turned out to be empty (we were already on + // the last real row) - report EOF right away instead of leaving RowNumber + // past the end with EOF still FALSE, which otherwise sits stale until a + // second Skip() happens to see _hasEOF. In between, RecNo/CurrentRow + // reflect the phantom row - e.g. nextrec()'s "IF EOF THEN GOTO(oldRecno)" + // never fires on the first PgDn past the end, and the recno it captures + // on the following call is the phantom row's blank value, not a real one. + SELF:RowNumber := 0 + SELF:_SetEOF(TRUE) + endif endif ELSE IF SELF:_currentPageNo == 1 diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg index e5435dc4dd..a8b8b011b2 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg @@ -666,7 +666,15 @@ partial class SQLRDD newRow:AcceptChanges() next endif - if lForward .and. newTable:Rows:Count < _oTd:PageSize + // A short page always means EOF. But when the total record count is an exact + // multiple of PageSize, the last page comes back FULL - "short page" never fires, + // so also check whether this page's absolute record range already reaches the + // known total. Without this, sequential forward paging (unlike GoBottom(), which + // jumps straight to the last page and marks it via _FetchLastPage) never sets + // _hasEOF on that exactly-full last page: the next Skip() then fetches a + // nonexistent page past it, landing on a bogus RowNumber instead of staying put. + var nAbsoluteRowsSeen := ((nNewPageNo - 1) * _oTd:PageSize) + newTable:Rows:Count + if lForward .and. (newTable:Rows:Count < _oTd:PageSize .or. nAbsoluteRowsSeen >= SELF:_serverReccount) SELF:_hasEOF := true else _currentPageNo := nNewPageNo From 7fe2d6073b63c124012aadf0970b054a50b81c6a Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Thu, 13 Aug 2026 10:24:52 +0200 Subject: [PATCH 09/10] 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. --- .../XSharp.SQLRdd/Classes/Connection.prg | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg b/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg index e7bef071a4..984363d51d 100644 --- a/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg +++ b/src/Runtime/XSharp.SQLRdd/Classes/Connection.prg @@ -564,6 +564,26 @@ class SqlDbConnection inherit SqlDbHandleObject implements IDisposable var fieldNames := List{} foreach row as DataRow in schema:Rows local colInfo := SQLHelpers.GetColumnInfoFromSchemaRow(row, fieldNames, longFieldNames) as DbColumnInfo + if colInfo:FieldType == DbFieldType.DateTime + // SQLHelpers.GetColumnInfo() tells DBF "D" (Date) apart from "T" (DateTime) + // purely by NumericPrecision, but a plain SQL Server `date` column reports + // the very same "not applicable" sentinel precision (255, via + // System.Data.SqlClient) as `datetime2` does, so it always fails that check + // and comes back as "T" - a Date column can never round-trip correctly. + // NumericScale tells them apart reliably instead: any genuine time-bearing + // column (datetime/datetime2/smalldatetime, whatever fractional-seconds + // precision) reports a real, small scale (0-7), while a `date` column keeps + // the 255 sentinel there too - so an implausibly high scale means "no time + // component", i.e. this is really a Date. + local nScale := 0 as short + if row:Table:Columns:Contains("NumericScale") .and. row["NumericScale"] is short var nScaleVal + nScale := nScaleVal + endif + if nScale >= 100 + colInfo:FieldType := DbFieldType.Date + colInfo:Length := 8 + endif + endif if SELF:LegacyFieldTypes // Map back to the old field types switch colInfo:FieldType From d46eac1916af47a008537fd1fea11482e8a7c857 Mon Sep 17 00:00:00 2001 From: ecosSystem Date: Thu, 13 Aug 2026 10:25:09 +0200 Subject: [PATCH 10/10] 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. --- src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg | 50 +++++++++++++------ .../XSharp.SQLRdd/RDD/SQLRDD-Orders.prg | 5 +- .../XSharp.SQLRdd/RDD/SQLRDD-Private.prg | 23 ++++++++- 3 files changed, 60 insertions(+), 18 deletions(-) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg index ce38474003..290c3d649f 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Main.prg @@ -237,6 +237,14 @@ partial class SQLRDD inherit Workarea self:_CloseCursor() else self:DataTable := _command:GetDataTable(self:Alias) + if self:DataTable == null + // Unlike Table mode, _ForceOpen() is a permanent no-op once _tableMode is + // Query (see _ForceOpen() below), so a failed SELECT here has no later retry + // path - DataTable would stay null for the object's entire lifetime and every + // subsequent access (Rows:Count right below, _GotoRow, GoTop, ...) would NPE. + self:_dbfError(self:Connection:LastException, Subcodes.EDB_USE, Gencode.EG_OPEN, "SQLRDD.Open", FALSE) + return false + endif SELF:_serverReccount := (DWORD) self:DataTable:Rows:Count SELF:_ReadOnly := true SELF:_hasEOF := TRUE @@ -255,7 +263,9 @@ partial class SQLRDD inherit Workarea /// When the area is in Tablemode, and no data has been read before, then this will trigger fetching the data from the database /// override method Append(lReleaseLock as logic) as logic - self:_ForceOpen() + if !self:_ForceOpen() + return false + endif var lResult := SELF:GoCold() if lResult var key := (dword) self:_builder:GetNextKey() @@ -351,7 +361,9 @@ partial class SQLRDD inherit Workarea /// override method PutValue(nFldPos as int, oValue as object) as logic // nFldPos is 1 based, the RDD compiles with /az+ - SELF:_ForceOpen() + if !SELF:_ForceOpen() + return false + endif if self:_ReadOnly self:_dbfError(ERDD.READONLY, XSharp.Gencode.EG_READONLY, "SqlRDD:PutValue", "Table is not Updatable" ) return false @@ -698,7 +710,10 @@ partial class SQLRDD inherit Workarea LOCAL result AS LOGIC TRY VAR nRec := Convert.ToUInt32( oRec ) - if oRec != self:CurrentRow[self:_recnoColumNo] + // CurrentRow is null when the table has never been successfully opened (the + // phantom row is only built once a SELECT actually loaded a schema - see the + // DataTable setter) - same case GetValue() already guards against above. + if self:CurrentRow == null .or. oRec != self:CurrentRow[self:_recnoColumNo] result := SELF:GoTo( (DWORD) nRec ) endif CATCH ex AS Exception @@ -728,18 +743,23 @@ partial class SQLRDD inherit Workarea IF SELF:_tableMode == TableMode.Query .and. self:_recnoColumNo == -1 RETURN SELF:_GotoRow((LONG) nRec) ENDIF - // Check to see if we have the record in the current buffer - var rowIndex := 1 - foreach oRow as DataRow in SELF:DataTable:Rows - if (int)oRow[self:_recnoColumNo] = nRec - SELF:RowNumber := rowIndex - SELF:_SetEOF(FALSE) - SELF:_SetBOF(FALSE) - SELF:_Found := TRUE - return true - endif - rowIndex++ - next + // Check to see if we have the record in the current buffer. DataTable can be null + // here if _ForceOpen() above reported success while the underlying SELECT actually + // failed (see _OpenTable()) - treat that as "not in the current buffer" rather than + // crashing, and fall through to the direct single-record fetch below. + if SELF:DataTable != null + var rowIndex := 1 + foreach oRow as DataRow in SELF:DataTable:Rows + if (int)oRow[self:_recnoColumNo] = nRec + SELF:RowNumber := rowIndex + SELF:_SetEOF(FALSE) + SELF:_SetBOF(FALSE) + SELF:_Found := TRUE + return true + endif + rowIndex++ + next + endif var found := SELF:_GotoRecord(nRec) SELF:_CheckEofBof() diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg index 6123f471aa..4d4a2b727d 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Orders.prg @@ -505,7 +505,10 @@ partial class SQLRDD // silently skipping every other row that shares this seek's key. self:_OpenTable(cSeekWhere) - IF SELF:DataTable:Rows:Count = 0 .and. !seekInfo.SoftSeek + // _OpenTable() can fail (e.g. the underlying SELECT errors out) and leave DataTable + // null instead of an empty table - treat that the same as "no rows found" instead of + // crashing on DataTable:Rows below, same fix as GoTo()/_ClearTable() already got. + IF (SELF:DataTable == null .or. SELF:DataTable:Rows:Count = 0) .and. !seekInfo.SoftSeek SELF:GoTo(0) SELF:_Found := false SELF:_SetEOF(true) diff --git a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg index a8b8b011b2..90a1d8eb72 100644 --- a/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg +++ b/src/Runtime/XSharp.SQLRdd/RDD/SQLRDD-Private.prg @@ -159,7 +159,9 @@ partial class SQLRDD end destructor internal method _ClearTable() AS VOID - SELF:DataTable:Rows:Clear() + IF SELF:DataTable != null + SELF:DataTable:Rows:Clear() + ENDIF RETURN @@ -519,6 +521,15 @@ partial class SQLRDD // WHERE clause has plenty more rows past the first page. SELF:_hasEOF := false SELF:DataTable := self:_ReadTable(sWhereClause) + if SELF:DataTable == null + // _ReadTable() -> Command:GetDataTable()/ExecuteReader() swallow the real + // ADO.NET exception into Connection:LastException instead of throwing it. + // Without this check we'd return TRUE here with no data loaded, and the + // first caller to touch DataTable (GoTo(), GoTop(), ...) would crash with + // a bare NullReferenceException that hides the actual database error. + self:_dbfError(self:Connection:LastException, Subcodes.EDB_USE, Gencode.EG_OPEN, "SQLRDD._OpenTable", FALSE) + return false + endif self:_GetRecCount() catch as Exception return false @@ -740,7 +751,10 @@ partial class SQLRDD PRIVATE METHOD _GotoRow(nRow as LONG) AS LOGIC SELF:_Found := FALSE - var nCount := SELF:DataTable:Rows:Count + // DataTable can be null here for a Query-mode table whose SELECT failed (see Open()) + // and that has no recno column to route through _GotoRecord() instead - treat that + // the same as an empty result set rather than crashing. + var nCount := IIF(SELF:DataTable == null, 0, SELF:DataTable:Rows:Count) IF nRow <= nCount .AND. nRow > 0 SELF:RowNumber := nRow SELF:_SetEOF(FALSE) @@ -765,6 +779,11 @@ partial class SQLRDD PRIVATE METHOD _UpdateRow(nRecNo AS INT) AS LOGIC local row as DataRow local lOk := TRUE as logic + // Reachable via UnLock() -> Close(), which never checks _ForceOpen()/DataTable itself - + // if the buffer was already torn down (_CloseCursor()) there is nothing left to persist. + if self:DataTable == null + return true + endif try foreach tableRow as DataRow in self:DataTable:Rows if (int)tableRow[self:_recnoColumNo] = nRecNo