Enable equality-seek acceleration for snapshot/RCSI reads on tables with live version chains by materializing seek candidates through the version store instead of declining table-wide to a full scan.

Author: RyanLamansky

SqlServerSimulator.Tests.Internal/IndexSeekTests.cs

@@ -578,12 +578,13 @@ public void RcsiRead_NoVersions_Seeks()
     }
 
     [TestMethod]
-    public void RcsiRead_WithLiveVersionChain_Declines()
+    public void RcsiRead_WithLiveVersionChain_Seeks()
     {
-        // An open writer leaves a version chain on the table, so an RCSI
-        // reader's visible version can diverge from the live heap row — the
-        // seek declines back to the full scan, and the reader still sees the
-        // pre-write committed value.
+        // An open writer leaves a version chain on the table. The RCSI reader
+        // still seeks (it no longer declines table-wide on any version chain),
+        // materializing each candidate through the version store: it sees the
+        // pre-write committed value of the row the writer touched and the
+        // unaffected value of another, both via the seek.
         var sim = new Simulation();
         using var reader = sim.CreateDbConnection();
         reader.Open();
@@ -602,21 +603,84 @@ public void RcsiRead_WithLiveVersionChain_Declines()
         IndexSeekDiagnostics.Sink = [];
         try
         {
-            using var command = reader.CreateCommand();
-            command.CommandText = "select val from t where id = 2";
-            using var r = command.ExecuteReader();
-            var rows = new List<object?>();
-            while (r.Read())
-                rows.Add(r.GetValue(0));
-            Contains("Scan(t)", IndexSeekDiagnostics.Sink);
-            DoesNotContain("Seek(t)", IndexSeekDiagnostics.Sink);
-            HasCount(1, rows);
-            AreEqual(50, rows[0]);
+            AreEqual(50, ReadVal(reader, "select val from t where id = 2"));
+            Contains("Seek(t)", IndexSeekDiagnostics.Sink);
+            DoesNotContain("Scan(t)", IndexSeekDiagnostics.Sink);
+
+            // The touched row reads its last-committed value (5), not the
+            // writer's uncommitted 999 — resolved through the version store on
+            // the seek path.
+            AreEqual(5, ReadVal(reader, "select val from t where id = 1"));
+        }
+        finally
+        {
+            IndexSeekDiagnostics.Sink = null;
+        }
+    }
+
+    [TestMethod]
+    public void SnapshotRead_AfterCommittedKeyChange_SeekStaysCorrect()
+    {
+        // The false-negative case the table-wide decline used to guard against:
+        // a committed UPDATE moves a row's PRIMARY KEY (2 -> 99) after a SNAPSHOT
+        // reader pins its snapshot. The reader must still find the row by its
+        // old key (the live heap no longer has id = 2, so the bucket misses it —
+        // the version-chain sweep is what recovers it) and must NOT see it under
+        // the new key (the snapshot predates that commit). Both stay seeks.
+        var sim = new Simulation();
+        using var reader = sim.CreateDbConnection();
+        reader.Open();
+        using (var setup = reader.CreateCommand())
+        {
+            setup.CommandText =
+                $"alter database simulated set allow_snapshot_isolation on; {TableT}";
+            _ = setup.ExecuteNonQuery();
+        }
+
+        // Pin the snapshot before the writer commits.
+        Exec(reader, "set transaction isolation level snapshot");
+        Exec(reader, "begin tran");
+        _ = ReadVal(reader, "select count(*) from t");
+
+        using (var writer = sim.CreateDbConnection())
+        {
+            writer.Open();
+            Exec(writer, "update t set id = 99 where id = 2");
+        }
+
+        IndexSeekDiagnostics.Sink = [];
+        try
+        {
+            // Old key still resolves to the snapshot-visible row (val 50).
+            AreEqual(50, ReadVal(reader, "select val from t where id = 2"));
+            // New key is invisible to this snapshot — found in the live bucket
+            // but the resolved version carries the old key, so the residual
+            // WHERE drops it.
+            IsNull(ReadVal(reader, "select val from t where id = 99"));
+            Contains("Seek(t)", IndexSeekDiagnostics.Sink);
+            DoesNotContain("Scan(t)", IndexSeekDiagnostics.Sink);
         }
         finally
         {
             IndexSeekDiagnostics.Sink = null;
         }
+
+        Exec(reader, "commit");
+    }
+
+    private static void Exec(SimulatedDbConnection c, string sql)
+    {
+        using var cmd = c.CreateCommand();
+        cmd.CommandText = sql;
+        _ = cmd.ExecuteNonQuery();
+    }
+
+    private static object? ReadVal(SimulatedDbConnection c, string sql)
+    {
+        using var cmd = c.CreateCommand();
+        cmd.CommandText = sql;
+        using var r = cmd.ExecuteReader();
+        return r.Read() ? (r.IsDBNull(0) ? null : r.GetValue(0)) : null;
     }
 
     [TestMethod]

SqlServerSimulator/Parser/Selection.Execution.IndexSeek.cs

@@ -63,22 +63,17 @@ private static FromSource[] MaybeApplyIndexSeek(
             return sources;
         }
 
-        // A snapshot / RCSI reader's visible version can carry a different key
-        // than the live heap row, so a current-heap index could miss it — but
-        // only once the table actually has version chains. With an empty version
-        // store every row is implicitly committed at Xmin 0 (visible to every
-        // snapshot) and the live heap IS the visible version, so the seek is
-        // safe; this is the common case for read-mostly / bacpac-loaded data,
-        // where declining would force a full scan on every point lookup.
-        // ResolveSnapshotXidForRead is called unconditionally for its side
-        // effects (pins the statement/tx snapshot xid, registers the active
-        // snapshot reader) — the row-touch path relies on that bookkeeping
-        // whether the read seeks or scans.
-        if (batch.ResolveSnapshotXidForRead(table) is not null && !table.RowVersions.IsEmpty)
-        {
-            IndexSeekDiagnostics.Sink?.Add($"Scan({table.Name})");
-            return sources;
-        }
+        // A snapshot / RCSI reader sees the version visible at its snapshot, not
+        // necessarily the live heap row. ResolveSnapshotXidForRead returns that
+        // snapshot xid (and pins the statement / tx snapshot as a side effect the
+        // row-touch path relies on whether the read seeks or scans). When it's
+        // non-null the seek still runs, but materializes each candidate through
+        // the version store and additionally sweeps the rows carrying a version
+        // chain — those are the only rows whose snapshot-visible key can differ
+        // from their live key, so a live-key-only seek could miss them. With an
+        // empty version store the sweep is empty and every candidate resolves to
+        // its live bytes. See MaterializeSnapshotCandidates.
+        var snapshotXid = batch.ResolveSnapshotXidForRead(table);
 
         var conjuncts = new List<BooleanExpression>();
         foreach (var excluder in excluders)
@@ -102,7 +97,7 @@ private static FromSource[] MaybeApplyIndexSeek(
         }
 
         if (equalities.Count != 0
-            && TrySeekByLongestPrefix(source, table, plan, batch, outerResolver, equalities, out var seekRows, out var width))
+            && TrySeekByLongestPrefix(source, table, plan, batch, snapshotXid, outerResolver, equalities, out var seekRows, out var width))
         {
             IndexSeekDiagnostics.Sink?.Add($"Seek({table.Name})");
             IndexSeekDiagnostics.Sink?.Add($"SeekWidth({table.Name},{width})");
@@ -234,6 +229,7 @@ private static bool TrySeekByLongestPrefix(
         HeapTable table,
         DataLockPlan plan,
         BatchContext batch,
+        long? snapshotXid,
         Func<MultiPartName, SqlValue>? outerResolver,
         Dictionary<int, Expression[]> equalities,
         out IEnumerable<byte[]> seekRows,
@@ -299,7 +295,9 @@ private static bool TrySeekByLongestPrefix(
                 candidates.AddRange(bucket);
         }
 
-        seekRows = MaterializeWithLockChecks(table, batch, plan, candidates);
+        seekRows = snapshotXid is { } sx
+            ? MaterializeSnapshotCandidates(table, batch, sx, candidates)
+            : MaterializeWithLockChecks(table, batch, plan, candidates);
         width = bestLen;
         return true;
 
@@ -441,6 +439,56 @@ private static IEnumerable<byte[]> MaterializeWithLockChecks(
         }
     }
 
+    // Snapshot / RCSI candidate materialization. Mirrors the snapshot branch of
+    // BatchContext.WrapWithRowConflictChecks, but over the seeked candidate set
+    // rather than the whole heap. Two sources, deduplicated by slot:
+    //   1. Bucket candidates — live rows whose CURRENT key matched the probe.
+    //      Each resolves to the version visible at the snapshot (live bytes when
+    //      the row carries no chain), or drops out when not visible.
+    //   2. Version-chain sweep — every slot in RowVersions. These are the only
+    //      rows whose snapshot-visible key can differ from their live key (so a
+    //      live-key-only bucket lookup could miss them) plus tombstoned slots
+    //      whose pre-delete version a pre-delete snapshot still sees. Bounded by
+    //      |RowVersions|, which a read-mostly RCSI workload keeps small (the GC
+    //      trims versions no open snapshot needs). The whole-table scan this
+    //      replaces already walks RowVersions in its own second pass, so the
+    //      seek is never more expensive than the scan it supplants.
+    // The matched equality conjuncts stay in the residual WHERE, so any candidate
+    // whose resolved version doesn't actually match the probe is filtered there.
+    private static IEnumerable<byte[]> MaterializeSnapshotCandidates(
+        HeapTable table, BatchContext batch, long snapshotXid, IReadOnlyList<(int Page, int Slot)> bucketCandidates)
+    {
+        var tx = batch.Connection.CurrentTransaction;
+        var seen = new HashSet<(int, int)>();
+
+        foreach (var (page, slot) in bucketCandidates)
+        {
+            if (!seen.Add((page, slot)))
+                continue;
+            if (table.Heap.ReadSlotBytes(page, slot) is { } live
+                && Storage.VersionStore.ResolveVisibleVersion(table, (page, slot), live, snapshotXid, tx) is { } resolved)
+            {
+                yield return resolved;
+            }
+        }
+
+        foreach (var kv in table.RowVersions)
+        {
+            var page = kv.Key.PageIndex;
+            var slot = kv.Key.SlotIndex;
+            if (!seen.Add((page, slot)))
+                continue;
+
+            var resolved = table.Heap.IsSlotTombstoned(page, slot)
+                ? Storage.VersionStore.ResolveTombstonedSlotForSnapshot(kv.Value, snapshotXid, tx)
+                : table.Heap.ReadSlotBytes(page, slot) is { } live
+                    ? Storage.VersionStore.ResolveVisibleVersion(table, (page, slot), live, snapshotXid, tx)
+                    : null;
+            if (resolved is { } bytes)
+                yield return bytes;
+        }
+    }
+
     // A value side that is constant across the source's rows and free of side
     // effects, so evaluating it once for the seek and again in the residual WHERE
     // is harmless. Pure conversion wrappers (CAST / CONVERT / parens) are peeled

docs/claude/indexes.md

@@ -27,12 +27,20 @@ A prefix is bounded at the first key column that either has no equality conjunct
 
 This is the path that collapses a **correlated `EXISTS` / `IN` / scalar subquery** (and an unindexed-inner `APPLY`) from O(outer × inner) toward linear: the inner re-executes per outer row, but a per-table cache (keyed on the `Heap`, built lazily on first seek) persists across those calls. Measured: a correlated `EXISTS` over a 4000 × 16000-row pair dropped from ~1480 ms to ~5 ms, on par with live SQL Server.
 
-The cache is a **read-only acceleration structure, not a maintained index**: buckets are tagged with the heap's `MutationGeneration` (bumped by every `Heap.Insert` / `DeleteAt`) and rebuilt when it moves, so the heap stays the single source of truth and the write path — including its row locking and lock-escalation accounting — is completely untouched. The seek isn't a free pass around concurrency control: it routes each seeked candidate row through the same `BatchContext.TouchRowForRead` lock / conflict pipeline the full scan uses, so it acquires locks on only the rows it touches (matching a real index seek's footprint), and it **declines** — falling back to the full scan — for:
+The cache is a **read-only acceleration structure, not a maintained index**: buckets are tagged with the heap's `MutationGeneration` (bumped by every `Heap.Insert` / `DeleteAt`) and rebuilt when it moves, so the heap stays the single source of truth and the write path — including its row locking and lock-escalation accounting — is completely untouched. The seek isn't a free pass around concurrency control: under a lock-based plan it routes each seeked candidate row through the same `BatchContext.TouchRowForRead` lock / conflict pipeline the full scan uses, so it acquires locks on only the rows it touches (matching a real index seek's footprint). It **declines** — falling back to the full scan — for:
 
-- **snapshot / RCSI reads on a table that carries a version chain** (`HeapTable.RowVersions` non-empty), where the version visible to the reader can carry a different key value than the live heap row (a current-heap index would miss it). When the version store is empty — the common read-mostly / bacpac-loaded case — every row is implicitly committed at Xmin 0 (visible to every snapshot) and the live heap *is* the visible version, so the seek is allowed; otherwise an RCSI point lookup would full-scan on every read. `ResolveSnapshotXidForRead` is still called unconditionally for its snapshot-pinning side effects;
 - **tx-scoped row-lock plans** (`REPEATABLE READ` / `SERIALIZABLE` / `UPDLOCK` / `HOLDLOCK` …), where the scan deliberately locks every row it reads to end of transaction;
 - non-base-table sources (derived tables, table variables, `FOR SYSTEM_TIME`), a WHERE with no equality conjunct on any index's leading column, range predicates (`>` / `<` / `BETWEEN`), and `NULL` / non-resolvable-collation value sides.
 
+### Snapshot / RCSI seeks (version-aware materialization)
+
+A SNAPSHOT / RCSI reader sees the version visible at its snapshot, which can carry a *different key* than the live heap row — so a live-key-only bucket lookup could miss a row whose key was changed out from under the snapshot. Rather than declining table-wide whenever the version store is non-empty (which under a busy RCSI workload meant nearly every point lookup full-scanned — a measured ~90–235× regression from a single live version chain), the seek runs and materializes through the version store (`MaterializeSnapshotCandidates`):
+
+- **Bucket candidates** (live rows whose *current* key matched the probe) each resolve to their snapshot-visible version via `VersionStore.ResolveVisibleVersion`, dropping out when not visible.
+- **A version-chain sweep** over `HeapTable.RowVersions` adds every slot carrying a chain — those are exactly the rows whose snapshot-visible key can differ from their live key, plus tombstoned slots a pre-delete snapshot still sees (`ResolveTombstonedSlotForSnapshot`). Deduplicated by slot against the bucket candidates.
+
+The matched equality conjuncts stay in the residual WHERE, so any candidate whose *resolved* version doesn't actually match the probe (e.g. a live-bucket row whose snapshot-visible version carries the old key) is filtered there — no false positives, and the sweep eliminates false negatives. The extra cost is O(|`RowVersions`|), which a read-mostly RCSI workload keeps small (the version-store GC trims versions no open snapshot needs); the whole-table scan this replaces *already* walks `RowVersions` in its own second pass, so the version-aware seek is never more expensive than the scan it supplants. `ResolveSnapshotXidForRead` is called unconditionally regardless of whether the read seeks or scans, for its snapshot-pinning side effects.
+
 All three single-table projectors — non-aggregate (`ProjectSqlRows`), aggregate (`BuildAggregateProjectionRows`), and window (`ProjectWindowedRows`) — narrow a single-base-table source through the seek, so `SELECT COUNT(*) … WHERE indexedcol = x` and `SELECT … OVER (…) FROM t WHERE indexedcol = x` both seek like their non-aggregate counterpart (without this the window projector silently full-scanned even when its WHERE was perfectly sargable, the regression that made running-total-per-parent EF queries scan the table). They also push single-source WHERE equality predicates onto the **leftmost** source of a multi-source FROM before the join (`NarrowLeftmostJoinSource` — the leftmost is always preserved, so this never changes outer-join semantics), which shrinks the join's driving rowset. The INNER / LEFT equi-join operator then **seeks the inner side per outer row** when that rowset is small and the inner is indexed on the join key — see [`joins.md`](joins.md). Not modeled: range seeks, ORDER BY elimination.
 
 ## Storage