sonar: fix remaining mechanical issues (batch 5)

src/SharpCoreDB/DataStructures/HashIndex.cs

@@ -337,7 +337,7 @@ internal void RemoveBatchKeys(object?[] keys, long[] positions)
  {
  foreach (var kvp in deferred.Where(static kvp => kvp.Value is not null))
  {
- CompactPositionList(kvp.Key, kvp.Value!);
+ CompactPositionList(kvp.Key, kvp.Value);
  }
  }
  }

src/SharpCoreDB/DataStructures/Table.CRUD.cs

@@ -1572,15 +1572,11 @@ private void UpdateSingleRow(Dictionary<string, object> row, IStorageEngine engi
  : null;
 
  // Snapshot old values of hash-indexed columns for key-only removal.
- Dictionary<string, object>? oldHashKeys = null;
- foreach (var kvp in this.hashIndexes)
- {
- if (row.TryGetValue(kvp.Key, out var oldVal))
- {
- oldHashKeys ??= new Dictionary<string, object>();
- oldHashKeys[kvp.Key] = oldVal;
- }
- }
+ Dictionary<string, object>? oldHashKeys = this.hashIndexes.Count == 0
+ ? null
+ : this.hashIndexes.Keys
+ .Where(row.ContainsKey)
+ .ToDictionary(key => key, key => row[key]);
 
  // Apply updates to the row
  foreach (var update in updates)
@@ -3410,6 +3406,54 @@ _btreeManager is null &&
  }
  }
 
+ /// <summary>
+ /// Returns true when the file is physically PK-ordered (strictly ascending keys, no tombstone or
+ /// zero-length anomalies) from offset 0 up to <paramref name="firstSearch"/>. This is the
+ /// pre-pass guard for the sequential PK batch scan: only a proven-ordered file lets the
+ /// forward-only main pass skip the rows before the first target — an unordered file could
+ /// otherwise place a later target before the first target's position.
+ /// </summary>
+ private bool IsFilePkOrderedUpTo(byte[] wholeFile, string pkCol, long firstSearch)
+ {
+ var pkWantedPre = new[] { this.PrimaryKeyIndex };
+ long walk = 0;
+ long prev = long.MinValue;
+ while (walk + 4 <= wholeFile.Length && walk < firstSearch)
+ {
+ int len = BinaryPrimitives.ReadInt32LittleEndian(wholeFile.AsSpan((int)walk, 4));
+ if (len > 0 && walk + 4 + len <= wholeFile.Length)
+ {
+ var r = DeserializeDeleteKeyRow(wholeFile.AsSpan((int)walk + 4, len), pkWantedPre);
+ if (r != null && r.TryGetValue(pkCol, out var v) && v is not null && v is not DBNull)
+ {
+ long pk = Convert.ToInt64(v, CultureInfo.InvariantCulture);
+ if (pk < prev)
+ {
+ return false; // physically unordered file -> per-row resolution
+ }
+
+ prev = pk;
+ }
+
+ walk += 4 + len;
+ }
+ else
+ {
+ if (len == 0)
+ {
+ return false;
+ }
+
+ // Tombstone marker: the negative value already encodes the whole slot span
+ // (4-byte prefix + payload), so skipping by |len| lands exactly on the next
+ // record's prefix.
+ walk += Math.Abs(len);
+ }
+ }
+
+ return true;
+ }
+
  /// <summary>
  /// Sequential ascending-INTEGER-PK batch resolution for the legacy (variable-length, plaintext,
  /// Columnar) DELETE path. When every condition is a strictly-ascending literal on an INTEGER PK
@@ -3481,42 +3525,9 @@ private bool TryResolvePkBatchSequentially(
  // target's position. Only then may the main pass skip those leading rows — an unordered
  // file could otherwise place a later target *before* the first target's position, which a
  // forward-only scan would never see. Any disorder falls back to the per-row path.
+ if (!IsFilePkOrderedUpTo(wholeFile, pkCol, firstSearch.Value))
  {
- var pkWantedPre = new[] { this.PrimaryKeyIndex };
- long walk = 0;
- long prev = long.MinValue;
- while (walk + 4 <= wholeFile.Length && walk < firstSearch.Value)
- {
- int len = BinaryPrimitives.ReadInt32LittleEndian(wholeFile.AsSpan((int)walk, 4));
- if (len > 0 && walk + 4 + len <= wholeFile.Length)
- {
- var r = DeserializeDeleteKeyRow(wholeFile.AsSpan((int)walk + 4, len), pkWantedPre);
- if (r != null && r.TryGetValue(pkCol, out var v) && v is not null && v is not DBNull)
- {
- long pk = Convert.ToInt64(v, CultureInfo.InvariantCulture);
- if (pk < prev)
- {
- return false; // physically unordered file -> per-row resolution
- }
-
- prev = pk;
- }
-
- walk += 4 + len;
- }
- else
- {
- if (len == 0)
- {
- return false;
- }
-
- // Tombstone marker: the negative value already encodes the whole slot span
- // (4-byte prefix + payload), so skipping by |len| lands exactly on the next
- // record's prefix.
- walk += Math.Abs(len);
- }
- }
+ return false;
  }
 
  var remaining = new HashSet<long>(targets); // set-based matching keeps unordered files correct

src/SharpCoreDB/Database/Execution/Database.Batch.cs

@@ -1121,8 +1121,9 @@ private bool TryParseDeleteForBatch(string sql, out string tableName, out string
  {
  if (tables.TryGetValue(tableName, out var tbl) && tbl is DataStructures.Table concreteDelete)
  {
- // Canonical batches go through the structured path (no WHERE rebuild/re-parse);
- // any non-canonical statement forces the string form for the whole table.
+ // Canonical batches go through the structured path (no WHERE rebuild or re-parse).
+ // NOSONAR:S125 - prose description: any non-canonical statement forces the
+ // string form for the whole table, not commented-out code.
  bool allCanonical = true;
  foreach (var (_, column, _) in deletes)
  {

src/SharpCoreDB/Services/Storage.Append.cs

@@ -53,8 +53,8 @@ public partial class Storage
  // append because OverwriteRecordAt refused to write inside a transaction.
  private readonly ConcurrentDictionary<string, Dictionary<long, byte[]>> bufferedOverwrites = new(StringComparer.Ordinal);
 
- // ✅ Commit-time tombstones: physical offsets of records deleted inside the current
- // transaction. The marker is NOT written at delete time (a rollback must keep the row);
+ // ✅ Commit-time tombstones: physical offsets of records deleted inside the current transaction.
+ // The marker is NOT written at delete time — a rollback must keep the row. NOSONAR:S125 (prose, not dead code)
  // ApplyBufferedTombstones writes the in-place negative-prefix markers when the transaction
  // commits, after the buffered appends are on disk. Rollback discards the buffer.
  private readonly Dictionary<string, List<long>> bufferedTombstones = new(StringComparer.Ordinal);

tests/benchmarks/SharpCoreDB.Benchmarks.Comparative/Program.cs

@@ -32,7 +32,7 @@ private Program() { } // Static utility class - prevent instantiation.
  const string BannerTop = "╔══════════════════════════════════════════════════════════╗";
  const string BannerBottom = "╚══════════════════════════════════════════════════════════╝";
  const string ResultsDirName = "results";
- const string BenchDbPassword = "bench123";
+ const string BenchDbPassword = "bench123"; // NOSONAR:S2068 - throwaway local benchmark credential, not a real secret
  const string EmailColumn = "email";
  const string ScoreColumn = "score";
  const string NameParam = "@name";
@@ -441,8 +441,8 @@ private static DatabaseConfig BuildPkDefaultVariantConfig(
  },
  "noadaptive" => new DatabaseConfig { StorageEngineType = engineType, EnableAdaptiveWalBatching = false },
  "hsinsert" => new DatabaseConfig { StorageEngineType = engineType, HighSpeedInsertMode = true },
- // "plain" == the tuned harness config with NoEncryptMode=true (BuildConfig default);
- // "tuned" == the same knob set but NoEncryptMode=false (isolates that flag).
+ // "plain" and "tuned" both select the tuned harness config built by BuildConfig; the
+ // only difference is that "tuned" turns the NoEncryptMode flag off (isolating it).
  "plain" => BuildConfig(engineType, fixedWidth: true),
  "tuned" => BuildConfig(engineType, fixedWidth: true, noEncrypt: false),
  _ => new DatabaseConfig { StorageEngineType = engineType },