Improved fidelity of cursors when there's no unique key.

Author: RyanLamansky

CLAUDE.md

@@ -128,7 +128,7 @@ Three entry points share one per-connection undo log: implicit (statement-level
 - `MERGE INTO target USING <source> ON … WHEN [NOT] MATCHED [BY TARGET|SOURCE] [AND …] THEN UPDATE|DELETE|INSERT … [OUTPUT $action, …]` — source = VALUES / SELECT / set-op / bare-table; multiple per-family AND-conditioned clauses; trailing `;` required. Msg 5324 / 8672 / 10713 / 10714 enforced. Triggers fire one INSERT → UPDATE → DELETE pass with combined affected rows. See [`docs/claude/dml.md`](docs/claude/dml.md).
 
 ### Cursors
-Session-scoped T-SQL cursors: `DECLARE <name> [INSENSITIVE][SCROLL] CURSOR [LOCAL|GLOBAL][FORWARD_ONLY|SCROLL][STATIC|KEYSET|DYNAMIC|FAST_FORWARD][READ_ONLY|…] FOR <select> [FOR {READ ONLY|UPDATE [OF cols]}]`, `OPEN`, `FETCH NEXT|PRIOR|FIRST|LAST|ABSOLUTE n|RELATIVE n [FROM] <cursor> [INTO @v,…]`, `CLOSE`, `DEALLOCATE`, plus `@@FETCH_STATUS` / `@@CURSOR_ROWS` / `CURSOR_STATUS` and positioned `WHERE CURRENT OF` UPDATE/DELETE. Effective sensitivity resolves at DECLARE: non-updatable query (not a single base table with a PK/UNIQUE) → STATIC; STATIC frozen-snapshot, KEYSET frozen-membership with live value re-reads (deleted member → `@@FETCH_STATUS=-2`), DYNAMIC fully live (skips deletes, sees inserts, `@@CURSOR_ROWS=-1`). Position tracked by the logical unique key (not a RID — UPDATE relocates rows). Errors Msg 16905/16915/16916/16917/16924/16925/16929/16931 probe-confirmed verbatim. Cursor variables (`DECLARE @c CURSOR`) raise `NotSupportedException`. See [`docs/claude/cursors.md`](docs/claude/cursors.md).
+Session-scoped T-SQL cursors: `DECLARE <name> [INSENSITIVE][SCROLL] CURSOR [LOCAL|GLOBAL][FORWARD_ONLY|SCROLL][STATIC|KEYSET|DYNAMIC|FAST_FORWARD][READ_ONLY|…] FOR <select> [FOR {READ ONLY|UPDATE [OF cols]}]`, `OPEN`, `FETCH NEXT|PRIOR|FIRST|LAST|ABSOLUTE n|RELATIVE n [FROM] <cursor> [INTO @v,…]`, `CLOSE`, `DEALLOCATE`, plus `@@FETCH_STATUS` / `@@CURSOR_ROWS` / `CURSOR_STATUS` and positioned `WHERE CURRENT OF` UPDATE/DELETE. Effective sensitivity resolves at DECLARE: non-updatable query (not a single base table) → STATIC; STATIC frozen-snapshot, KEYSET frozen-membership with live value re-reads (deleted-or-key-changed member → `@@FETCH_STATUS=-2`), DYNAMIC fully live (skips deletes, sees inserts, `@@CURSOR_ROWS=-1`). Position tracked by the row's stable `(page, slot)` heap address (preserved through UPDATEs via `Heap.UpdateAt`'s in-place / forwarding-pointer machinery); KEYSET additionally tracks the unique-key tuple when the base table has a PK/UNIQUE, so a unique-column UPDATE produces -2. Errors Msg 16905/16915/16916/16917/16924/16925/16929/16931 probe-confirmed verbatim. Cursor variables (`DECLARE @c CURSOR`) raise `NotSupportedException`. See [`docs/claude/cursors.md`](docs/claude/cursors.md).
 
 ### EF Core adapter coverage
 `UseSqlServerSimulator(...)` covers seven SqlParameter-downcast pairs: `DateOnly→date`, `DateTime→date`, `DateTime→smalldatetime`, `TimeOnly→time(N)`, `TimeSpan→time(N)`, `decimal→money`, `decimal→smallmoney`. Without the adapter those mappings throw at SaveChanges. MAX-string family flows through plain `UseSqlServer`.
@@ -213,8 +213,8 @@ Per-feature deep-dives live under `docs/claude/`. Each entry below is a trigger:
 - `decimal` / `numeric`: backed by .NET `decimal`. Values requiring more than 28 significant digits aren't modeled (declarations up through `decimal(38, *)` accepted so storage byte-width matches).
 - `float` text formatting: .NET `G15`/`G7` rather than SQL Server's `1e+015`-style scientific.
 - Auto-generated constraint names: PK / UNIQUE shape `PK__<table8>__<16hex>` / `UQ__<table8>__<16hex>` (16-hex 64-bit FNV-1a); CK / FK / DF shape `CK__<table8>__[<col8>__]<8hex>` (8-hex 32-bit FNV-1a). Both are deterministic across runs, distinct from SQL Server's object-id-derived hex (so won't byte-match).
-- **DELETE / UPDATE leak page space**: deleted (or relocated) row payload bytes stay in their original page until process exit; only the slot is tombstoned. Slot directory entries also never reused.
-- **DELETE / UPDATE leak LOB chains**: orphaned LOB chains stay in `Heap.LobPages`. Other rows reference LOB pages by stable index, so list compaction would corrupt them.
+- **DELETE / forwarding UPDATE leak page space**: deleted row payload bytes stay in their original page until process exit; only the slot is tombstoned. Slot directory entries also never reused. An UPDATE whose new payload exceeds the slot's existing extent forwards (single-level pointer matching SQL Server's heap-update behavior) and similarly leaves the original slot's payload bytes dead. An UPDATE that fits in place reuses the slot's bytes and adds no leak.
+- **UPDATE leaks LOB chains**: a UPDATE-replaced row's LOB chains stay in `Heap.LobPages`; the row encoder allocates fresh chains for the new payload regardless of whether the UPDATE rewrites in place or forwards. DELETE leaks similarly. Other rows reference LOB pages by stable index, so list compaction would corrupt them.
 - **`GetBytes`/`GetChars` materialize, don't stream**: each call decodes the full column value via `RowDecoder` and slices into the caller's buffer. Behavior matches per-call observation; the streaming-memory guarantee doesn't.
 - **`SELECT INTO` string `+` reads as nullable**: real SQL Server projects `cs + 'x'` (both NOT NULL) as NOT NULL; the simulator can't statically distinguish string-concat from integer-add at projection-schema time (the dispatch happens runtime on operand types), so all `Add` results read as nullable. Conservative; no test reliance on string-`+`-non-null.
 - **`SELECT INTO` from a CTE drops identity + nullability**: CTE bindings synthesize their wrapper `HeapColumn` entries with `nullable: true` and no identity, so the analyzer treats CTE sources as derived plans. Real SQL Server preserves both through simple single-source CTEs. Fix requires propagating column metadata through CTE bindings.
@@ -230,4 +230,4 @@ Per-feature deep-dives live under `docs/claude/`. Each entry below is a trigger:
 - **`STRING_SPLIT(..., ..., cast(@v as int))` — wrapped variable accepted**: the simulator's `enable_ordinal` const-only gate rejects bare `@v` (Msg 8748, matching real) but a Cast / Parenthesized wrapper around the variable slips past the gate. Real SQL Server rejects all variable-bearing shapes regardless of wrapping. No real-world emission hits this — the const-only restriction means `@v` ergonomics are weak in any form.
 - **`OPENJSON` `WITH ... AS JSON`** on `nvarchar(max)` raises `NotSupportedException` even though real SQL Server accepts the column form there (sub-tree extraction not modeled). Non-`nvarchar(max)` columns with `AS JSON` raise **Msg 13618**, matching real.
 - **`FOR SYSTEM_TIME` qualified-name format**: real SQL Server pads temp-table names in Msg 13544 with their internal allocation suffix (`#x____...___…000000000148`); the simulator emits the bare `tempdb.dbo.#x` form. Same Msg number / framing, less verbose name.
-- **Cursor edges**: cursors over a **JOIN / derived table / view** are forced to STATIC (read-only snapshot, `@@CURSOR_ROWS`=count); real SQL Server makes them DYNAMIC (`@@CURSOR_ROWS`=-1, sees mid-loop changes, `WHERE CURRENT OF` works) — the simulator only takes the updatable KEYSET/DYNAMIC path for a *direct single base table with a PK/UNIQUE* (set-op/UNION cursors are forced STATIC on the real server too, so those match). `@@CURSOR_ROWS` is `-1` throughout for DYNAMIC (real may report a transient positive count pre-fetch); a keyset `-2` (deleted-member) fetch leaves the `INTO` variables unchanged rather than zeroing/NULLing them; `FETCH` without `INTO` omits the trailing `ROWSTAT` column. See [`docs/claude/cursors.md`](docs/claude/cursors.md).
+- **Cursor edges**: cursors over a **JOIN / derived table / view** are forced to STATIC (read-only snapshot, `@@CURSOR_ROWS`=count); real SQL Server makes them DYNAMIC (`@@CURSOR_ROWS`=-1, sees mid-loop changes, `WHERE CURRENT OF` works) — the simulator only takes the updatable KEYSET/DYNAMIC path for a *direct single base table* (set-op/UNION cursors are forced STATIC on the real server too, so those match). `@@CURSOR_ROWS` is `-1` throughout for DYNAMIC (real may report a transient positive count pre-fetch); a keyset `-2` (deleted-member) fetch leaves the `INTO` variables unchanged rather than zeroing/NULLing them; `FETCH` without `INTO` omits the trailing `ROWSTAT` column. See [`docs/claude/cursors.md`](docs/claude/cursors.md).

SqlServerSimulator.Tests/CursorTests.cs

@@ -556,6 +556,69 @@ select @log
             """));
     }
 
+    private const string NoKeySeed =
+        "create table h (id int not null, name varchar(20) not null); " +
+        "insert h values (1,'a'),(2,'b'),(3,'c');";
+
+    [TestMethod]
+    public void KeysetCursor_NoUniqueKeyHeap_SeesUpdatedValues()
+        => AreEqual("a;NEW;c;", new Simulation().ExecuteScalar(NoKeySeed + """
+            declare @id int, @name varchar(20), @log varchar(200) = '';
+            declare c cursor keyset for select id, name from h order by id;
+            open c; fetch next from c into @id, @name;
+            update h set name = 'NEW' where id = 2;        -- non-key UPDATE on no-key heap
+            while @@fetch_status = 0
+            begin
+              set @log = @log + @name + ';';
+              fetch next from c into @id, @name;
+            end
+            close c; deallocate c;
+            select @log
+            """));
+
+    [TestMethod]
+    public void WhereCurrentOf_NoUniqueKeyHeap_UpdatesPositionedRow()
+        => AreEqual("POS", new Simulation().ExecuteScalar(NoKeySeed + """
+            declare @id int;
+            declare c cursor for select id from h order by id;
+            open c;
+            fetch next from c into @id;        -- id 1
+            fetch next from c into @id;        -- id 2 (positioned)
+            update h set name = 'POS' where current of c;
+            close c; deallocate c;
+            select name from h where id = 2
+            """));
+
+    [TestMethod]
+    public void WhereCurrentOf_NoUniqueKeyHeap_DeletesPositionedRow()
+        => AreEqual(2, ExecuteScalar<int>(NoKeySeed + """
+            declare @id int;
+            declare c cursor for select id from h order by id;
+            open c;
+            fetch next from c into @id;
+            fetch next from c into @id;
+            delete h where current of c;
+            close c; deallocate c;
+            select count(*) from h
+            """));
+
+    /// <summary>
+    /// Force the UPDATE to forward (new name longer than old) — the cursor
+    /// must still find the row by its stable address on re-fetch.
+    /// </summary>
+    [TestMethod]
+    public void ForwardedUpdate_PreservesRowAddressForCursorRefetch()
+        => AreEqual("AAA", new Simulation().ExecuteScalar(NoKeySeed + """
+            declare @id int, @name varchar(20);
+            declare c cursor keyset for select id, name from h order by id;
+            open c;
+            fetch next from c into @id, @name;        -- id 1
+            update h set name = replicate('A', 20) where id = 1;   -- grows the row → forward
+            update h set name = 'AAA' where id = 1;               -- second update on the same (now-forwarded) row
+            fetch first from c into @id, @name;
+            select @name
+            """));
+
     [TestMethod]
     public void FetchWithoutInto_YieldsSingleRowResultSet()
     {

SqlServerSimulator/Cursor.cs

@@ -32,18 +32,19 @@ internal enum FetchDirection { Next, Prior, First, Last, Absolute, Relative }
 /// <summary>
 /// A session-scoped T-SQL cursor (declared with <c>DECLARE … CURSOR FOR
 /// &lt;select&gt;</c>). Lives in <see cref="SimulatedDbConnection.Cursors"/>.
-/// Position is tracked by the base table's unique-key tuple (not a physical
-/// RID, since the simulator's UPDATE relocates rows), so KEYSET re-reads and
-/// positioned <c>WHERE CURRENT OF</c> DML survive value updates.
+/// Position is tracked by the base row's stable <c>(page, slot)</c> address,
+/// which <see cref="Heap.UpdateAt"/> preserves through value updates by
+/// rewriting in place (or installing a forwarding pointer) — so KEYSET
+/// membership tracking and positioned <c>WHERE CURRENT OF</c> DML work
+/// without requiring the base table to have a unique key.
 /// </summary>
 internal sealed class Cursor(
     string name,
     Selection selection,
     CursorSensitivity sensitivity,
     bool scrollable,
     bool readOnly,
-    HeapTable? baseTable,
-    int[]? keyStorageOrdinals)
+    HeapTable? baseTable)
 {
     public readonly string Name = name;
     public readonly Selection Selection = selection;
@@ -54,9 +55,6 @@ internal sealed class Cursor(
     /// <summary>The single base table, non-null for KEYSET / DYNAMIC / positioned DML.</summary>
     public readonly HeapTable? BaseTable = baseTable;
 
-    /// <summary>Storage ordinals of the base table's unique key; non-null with <see cref="BaseTable"/>.</summary>
-    public readonly int[]? KeyStorageOrdinals = keyStorageOrdinals;
-
     public bool IsOpen;
 
     /// <summary>
@@ -69,17 +67,21 @@ internal sealed class Cursor(
         ? -1
         : this.Sensitivity == CursorSensitivity.Dynamic
             ? 1
-            : (this.staticRows?.Count ?? this.keysetKeys?.Count ?? 0) > 0 ? 1 : 0;
+            : (this.staticRows?.Count ?? this.keysetIdentities?.Count ?? 0) > 0 ? 1 : 0;
 
-    /// <summary>Unique-key tuple of the row the cursor is positioned on, or
-    /// null when not on a live row (before first FETCH, past the end, or on a
+    /// <summary>Stable <c>(page, slot)</c> address of the row the cursor is positioned
+    /// on, or null when not on a live row (before first FETCH, past the end, or on a
     /// keyset hole). Read by positioned <c>WHERE CURRENT OF</c> DML.</summary>
-    public SqlValue[]? CurrentKey;
+    public (int Page, int Slot)? CurrentRid;
 
     // STATIC: frozen projected values, walked by index.
     private List<SqlValue[]>? staticRows;
-    // KEYSET: ordered snapshot of unique-key tuples (membership frozen at OPEN).
-    private List<SqlValue[]>? keysetKeys;
+    // KEYSET: ordered snapshot of row identities (membership frozen at OPEN).
+    // UniqueKey carries the tuple when the base table has a PK/UNIQUE — KEYSET
+    // membership tracks by that, matching SQL Server's keyset-is-identified-by-
+    // the-unique-index behavior. UniqueKey null falls back to Rid (no-unique-
+    // key heap path; simulator extension).
+    private List<(SqlValue[]? UniqueKey, (int Page, int Slot) Rid)>? keysetIdentities;
     // Position for indexed (STATIC / KEYSET): -1 before-first, == count after-last.
     private int position;
 
@@ -103,9 +105,9 @@ public void Open(BatchContext batch)
                 batch.Connection.LastCursorRows = this.staticRows.Count;
                 break;
             case CursorSensitivity.Keyset:
-                this.keysetKeys = [.. this.Selection.EnumerateForCursor(batch).Select(r => r.UniqueKey)];
+                this.keysetIdentities = [.. this.Selection.EnumerateForCursor(batch).Select(r => (r.UniqueKey, r.Rid))];
                 this.position = -1;
-                batch.Connection.LastCursorRows = this.keysetKeys.Count;
+                batch.Connection.LastCursorRows = this.keysetIdentities.Count;
                 break;
             default: // Dynamic
                 this.dynamicLast = null;
@@ -115,7 +117,7 @@ public void Open(BatchContext batch)
                 break;
         }
 
-        this.CurrentKey = null;
+        this.CurrentRid = null;
         this.IsOpen = true;
     }
 
@@ -127,9 +129,9 @@ public void Close()
         if (!this.IsOpen)
             throw SimulatedSqlException.CursorNotOpen(state: 1);
         this.staticRows = null;
-        this.keysetKeys = null;
+        this.keysetIdentities = null;
         this.dynamicLast = null;
-        this.CurrentKey = null;
+        this.CurrentRid = null;
         this.IsOpen = false;
     }
 
@@ -171,33 +173,38 @@ private void EnsureDirectionAllowed(FetchDirection direction)
         var count = this.staticRows!.Count;
         if (!this.TryMoveIndex(direction, offset, count))
         {
-            this.CurrentKey = null;
+            this.CurrentRid = null;
             return (-1, null);
         }
-        // STATIC is read-only; CurrentKey stays null (WHERE CURRENT OF rejected upstream).
+        // STATIC is read-only; CurrentRid stays null (WHERE CURRENT OF rejected upstream).
         return (0, this.staticRows[this.position]);
     }
 
     private (int, SqlValue[]?) FetchKeyset(BatchContext batch, FetchDirection direction, long offset)
     {
-        var count = this.keysetKeys!.Count;
+        var count = this.keysetIdentities!.Count;
         if (!this.TryMoveIndex(direction, offset, count))
         {
-            this.CurrentKey = null;
+            this.CurrentRid = null;
             return (-1, null);
         }
 
-        var key = this.keysetKeys[this.position];
+        var (key, rid) = this.keysetIdentities[this.position];
         foreach (var row in this.Selection.EnumerateForCursor(batch))
         {
-            if (Selection.CompareKeyTuples(row.UniqueKey, key) == 0)
+            var match = key is not null
+                ? row.UniqueKey is not null && Selection.CompareKeyTuples(row.UniqueKey, key) == 0
+                : row.Rid.Equals(rid);
+            if (match)
             {
-                this.CurrentKey = key;
+                this.CurrentRid = row.Rid;
                 return (0, row.Values);
             }
         }
-        // Member deleted out from under the keyset: status -2, no current row.
-        this.CurrentKey = null;
+        // Member deleted out from under the keyset (or its unique-key columns
+        // changed, making the row no longer findable by the snapshotted key):
+        // status -2, no current row.
+        this.CurrentRid = null;
         return (-2, null);
     }
 
@@ -245,14 +252,14 @@ private bool TryMoveIndex(FetchDirection direction, long offset, int count)
 
         if (target is null)
         {
-            this.CurrentKey = null;
+            this.CurrentRid = null;
             return (-1, null);
         }
 
         this.dynamicLast = target;
         this.dynamicBeforeFirst = false;
         this.dynamicAfterLast = false;
-        this.CurrentKey = target.UniqueKey;
+        this.CurrentRid = target.Rid;
         return (0, target.Values);
     }