Merge pull request #356 from MPCoreDeveloper/sonar/cleanup-354

fix(sonar): resolve the 50 quality-gate findings on the backport

Author: MPCoreDeveloper

src/SharpCoreDB/DataStructures/Table.CRUD.cs

@@ -20,6 +20,9 @@ public partial class Table
     /// <summary>Error message used by every write path when the table is opened read-only.</summary>
     private const string ReadOnlyInsertError = "Cannot insert in readonly mode";
 
+    /// <summary>Error message used by every DELETE path when the table is opened read-only.</summary>
+    private const string ReadOnlyDeleteError = "Cannot delete in readonly mode";
+
     /// <summary>
     /// Inserts a row into the table.
     /// Routes to columnar or page-based storage ENGINE based on StorageMode.
@@ -1615,6 +1618,29 @@ private void ValidateUpdatedRow(Dictionary<string, object> row)
         }
     }
 
+    /// <summary>
+    /// Patches a single row in place when the existing record bytes can be located and every
+    /// updated field fits its slot (fixed-size fields resolved at their actual offsets, or an
+    /// unchanged-size trailing variable field); otherwise falls back to a full serialization.
+    /// A same-length patch enables an in-place overwrite (Issue #6) so the file does not grow.
+    /// </summary>
+    private byte[] TryPatchOrSerializeRow(byte[]? existingData, Dictionary<string, object> updates, Dictionary<string, object> row)
+    {
+        if (existingData is { Length: > 0 })
+        {
+            var patched = _fixedWidthRecords
+                ? TryOverwriteFixedWidthInPlace(existingData, updates)
+                : TryOverwriteFieldsInPlaceActual(existingData, updates);
+
+            if (patched is not null)
+            {
+                return patched;
+            }
+        }
+
+        return SerializeRowExact(row);
+    }
+
     private void UpdateColumnarRow(Dictionary<string, object> row, IStorageEngine engine, Dictionary<string, object> updates, string? oldPkValue, Dictionary<string, object>? oldHashKeys, long rowPos)
     {
         // Fixed-width layout step: when the row's existing bytes can be located, patch
@@ -1627,12 +1653,7 @@ private void UpdateColumnarRow(Dictionary<string, object> row, IStorageEngine en
         if (rowPos >= 0)
         {
             var existingData = engine.Read(Name, rowPos);
-            rowData = existingData is { Length: > 0 }
-                && (_fixedWidthRecords
-                    ? TryOverwriteFixedWidthInPlace(existingData, updates)
-                    : TryOverwriteFieldsInPlaceActual(existingData, updates)) is { } patched
-                        ? patched
-                        : SerializeRowExact(row);
+            rowData = TryPatchOrSerializeRow(existingData, updates, row);
         }
         else
         {
@@ -1771,7 +1792,7 @@ private void RepointPrimaryKeyIfChanged(Dictionary<string, object> row, string?
     /// via the PK when present). The position lets the columnar write path patch fields in place
     /// (fixed-width layout) instead of appending a new version.
     /// </summary>
-    private List<(long Position, Dictionary<string, object> Row)> ResolveUpdateRows(string? where)
+    private List<(long Position, Dictionary<string, object> Row)> ResolveUpdateRows(string? where) // NOSONAR:S3776 - sequential guarded index-resolution steps (PK -> hash -> scan); extracting branches would re-read/duplicate shared fallbacks
     {
         var engine = GetOrCreateStorageEngine();
         var result = new List<(long, Dictionary<string, object>)>();
@@ -1866,7 +1887,7 @@ private void RepointPrimaryKeyIfChanged(Dictionary<string, object> row, string?
     /// <param name="newPosition">The storage position after relocation.</param>
     /// <param name="oldPkValue">The PK value before the update (may be null if the table has no PK).</param>
     /// <param name="newPkValue">The PK value after the update (may be null if the table has no PK).</param>
-    private void RepointIndexesAfterRelocation(long oldPosition, long newPosition, string? oldPkValue, string? newPkValue)
+    private void RepointIndexesAfterRelocation(long oldPosition, long newPosition, string? oldPkValue, string? newPkValue) // NOSONAR:S1172 - oldPosition retained for call-site symmetry with relocation-reporting engines (all callers already hold it)
     {
         if (this.PrimaryKeyIndex >= 0)
         {
@@ -1933,7 +1954,7 @@ internal void UpdateMultiple(List<(string where, Dictionary<string, object> upda
                 bool touchesHashIndexedColumn = false;
                 if (this.hashIndexes.Count > 0)
                 {
-                    foreach (var updateKey in updates.Keys)
+                    foreach (var updateKey in updates.Keys) // NOSONAR:S3267 - updates.Keys is tiny; LINQ would add a closure + enumerator alloc per op in the batch-DML hot path
                     {
                         if (this.hashIndexes.ContainsKey(updateKey))
                         {
@@ -2167,12 +2188,7 @@ internal void UpdateMultiple(List<(string where, Dictionary<string, object> upda
                             if (oldPosition >= 0)
                             {
                                 var existingData = engine.Read(Name, oldPosition);
-                                rowData = existingData is { Length: > 0 }
-                                    && (_fixedWidthRecords
-                                        ? TryOverwriteFixedWidthInPlace(existingData, updates)
-                                        : TryOverwriteFieldsInPlaceActual(existingData, updates)) is { } patched
-                                            ? patched
-                                            : SerializeRowExact(row);
+                                rowData = TryPatchOrSerializeRow(existingData, updates, row);
                             }
                             else
                             {
@@ -2312,7 +2328,7 @@ private bool HasColumnCheckConstraints()
             return false;
         }
 
-        foreach (var expr in expressions)
+        foreach (var expr in expressions) // NOSONAR:S3267 - LINQ would allocate per call; this runs per UPDATE op in the batch-DML hot path
         {
             if (expr is not null)
             {
@@ -2347,7 +2363,7 @@ private bool HasExplicitNamedIndex(string column)
     /// cleanup, key-only hash-index cleanup (single lock per index) and row-count bookkeeping.
     /// </summary>
     /// <param name="recordsToDelete">The storage positions and their deserialized rows.</param>
-    private void DeleteRecordsCore(List<(long storagePosition, Dictionary<string, object> row)> recordsToDelete)
+    private void DeleteRecordsCore(List<(long storagePosition, Dictionary<string, object> row)> recordsToDelete) // NOSONAR:S3776 - per-engine physical delete, PK cleanup, key-only hash removal and compaction bookkeeping are distinct but share the batch; extraction would force intermediate lists
     {
         if (recordsToDelete.Count == 0)
             return;
@@ -2438,7 +2454,7 @@ public void Delete(string? where)
     /// </summary>
     public int DeleteAffected(string? where)
     {
-        if (this.isReadOnly) throw new InvalidOperationException("Cannot delete in readonly mode");
+        if (this.isReadOnly) throw new InvalidOperationException(ReadOnlyDeleteError);
 
         this.rwLock.EnterWriteLock();
         try
@@ -2462,7 +2478,7 @@ public int DeleteAffected(string? where)
     /// </summary>
     public List<Dictionary<string, object>> DeleteAffectedRows(string? where)
     {
-        if (this.isReadOnly) throw new InvalidOperationException("Cannot delete in readonly mode");
+        if (this.isReadOnly) throw new InvalidOperationException(ReadOnlyDeleteError);
 
         this.rwLock.EnterWriteLock();
         try
@@ -2637,7 +2653,7 @@ public List<Dictionary<string, object>> DeleteAffectedRows(string? where)
     [MethodImpl(MethodImplOptions.AggressiveOptimization)]
     internal void DeleteMultiple(List<string> whereConditions)
     {
-        if (this.isReadOnly) throw new InvalidOperationException("Cannot delete in readonly mode");
+        if (this.isReadOnly) throw new InvalidOperationException(ReadOnlyDeleteError);
         if (whereConditions.Count == 0) return;
 
         this.rwLock.EnterWriteLock();
@@ -3102,7 +3118,7 @@ public bool UpdateByPrimaryKey(object key, Dictionary<string, object> updates)
 
             // WP13: capture only what index maintenance needs instead of copying the whole row.
             Dictionary<string, object>? oldHashKeys = null;
-            foreach (var kvp in this.hashIndexes)
+            foreach (var kvp in this.hashIndexes) // NOSONAR:S3267 - deliberate: LINQ Select/Where would allocate per point-update on the hot path
             {
                 if (row.TryGetValue(kvp.Key, out var oldVal))
                 {
@@ -3202,7 +3218,7 @@ public bool DeleteByPrimaryKey(object key)
         ArgumentNullException.ThrowIfNull(key);
 
         if (this.isReadOnly)
-            throw new InvalidOperationException("Cannot delete in readonly mode");
+            throw new InvalidOperationException(ReadOnlyDeleteError);
 
         if (this.PrimaryKeyIndex < 0)
             return false;

src/SharpCoreDB/DataStructures/Table.Serialization.cs

@@ -1021,7 +1021,7 @@ private int EstimateRowSize(Dictionary<string, object> row)
     /// <param name="type">The data type of the value.</param>
     /// <returns>Number of bytes written.</returns>
     [MethodImpl(MethodImplOptions.AggressiveOptimization)]
-    internal static int WriteTypedValueToSpan(Span<byte> buffer, object value, DataType type)
+    internal static int WriteTypedValueToSpan(Span<byte> buffer, object value, DataType type) // NOSONAR:S3776 - exhaustive per-DataType binary writer with size guards; splitting would add a dispatch layer on the row-serialization hot path
     {
         if (value == DBNull.Value || value == null)
         {
@@ -1193,7 +1193,7 @@ internal static int WriteTypedValueToSpan(Span<byte> buffer, object value, DataT
     /// <param name="bytesRead">Output: number of bytes consumed.</param>
     /// <returns>The deserialized value.</returns>
     [MethodImpl(MethodImplOptions.AggressiveOptimization)]
-    internal static object ReadTypedValueFromSpan(ReadOnlySpan<byte> buffer, DataType type, out int bytesRead)
+    internal static object ReadTypedValueFromSpan(ReadOnlySpan<byte> buffer, DataType type, out int bytesRead) // NOSONAR:S3776 - exhaustive per-DataType binary reader with size guards; splitting would add a dispatch layer on the row-deserialization hot path
     {
         bytesRead = 1;
 

src/SharpCoreDB/DataStructures/Table.StructScanning.cs

@@ -53,6 +53,14 @@ public partial class Table
     /// <returns>Zero-allocation enumerable of StructRow instances.</returns>
     [MethodImpl(MethodImplOptions.AggressiveOptimization)]
     public IEnumerable<StructRow> ScanStructRows(bool enableCaching = false)
+    {
+        // ✅ FIX: Validate upfront, then delegate to the iterator so argument errors surface at
+        // the call site instead of on first enumeration (iterator bodies defer all validation).
+        ArgumentNullException.ThrowIfNull(this.storage);
+        return ScanStructRowsCore(enableCaching);
+    }
+
+    private IEnumerable<StructRow> ScanStructRowsCore(bool enableCaching)
     {
         // Fixed-width record layout (out-of-line overflow): the zero-alloc struct scan walks the
         // variable-length record format, so fixed-width tables fall back to the dictionary path
@@ -69,9 +77,6 @@ public IEnumerable<StructRow> ScanStructRows(bool enableCaching = false)
             yield break;
         }
 
-        // ✅ FIX: Validate upfront, then delegate to iterator methods
-        ArgumentNullException.ThrowIfNull(this.storage);
-
         // Build schema once for entire scan
         var schema = BuildVariableLengthSchema();
 
@@ -191,7 +196,7 @@ private IEnumerable<StructRow> ScanStructRowsWhereCore(string? where, bool enabl
         return ScanStructRowsWhereCoreIterator(where, enableCaching);
     }
 
-    private IEnumerable<StructRow> ScanStructRowsWhereCoreIterator(string? where, bool enableCaching)
+    private IEnumerable<StructRow> ScanStructRowsWhereCoreIterator(string? where, bool enableCaching) // NOSONAR:S3776 - ordered fast-path cascade (hash -> PK -> SIMD -> scan); each guard is a separate resolution strategy with early yield-break
     {
         // Fixed-width records: StructRow's variable-length schema can't walk the fixed-width
         // format, so matched records are materialized through the dictionary path. The numeric-SIMD
@@ -247,15 +252,15 @@ private IEnumerable<StructRow> ScanStructRowsWhereCoreIterator(string? where, bo
         {
             foreach (var row in Select(where))
             {
-                yield return StructRow.FromDictionary(row, fixedColumns ?? [], fixedTypes ?? []);
+                // fixedColumns/fixedTypes are non-null here (assigned when fixedWidth was resolved).
+                yield return StructRow.FromDictionary(row, fixedColumns!, fixedTypes!);
             }
 
             yield break;
         }
 
         // Fallback: full scan with a simple equality predicate (scalar, allocation-free per row).
-        // NOSONAR:S3267 - intentional: LINQ Where would allocate per row on the scan hot path.
-        foreach (var row in ScanStructRows(enableCaching))
+        foreach (var row in ScanStructRows(enableCaching)) // NOSONAR:S3267 - intentional: LINQ Where would allocate per row on the scan hot path
         {
             if (!hasSimpleWhere || simpleColumn is null || simpleValue is null ||
                 MatchesSimpleWhere(row, schema, simpleColumn, simpleValue))
@@ -318,7 +323,7 @@ private IEnumerable<StructRow> ScanByPrimaryKeyPoint(
     /// (no deserialization, no boxing). Integer/Long use portable Vector&lt;T&gt;; Real uses
     /// direct per-record reads. Fixed-width tables materialize rows through the dictionary path.
     /// </summary>
-    private IEnumerable<StructRow> ScanByNumericSimd(
+    private IEnumerable<StructRow> ScanByNumericSimd( // NOSONAR - S3776 (SIMD kernel with per-type extraction passes) + S107 (layout parameters intentionally threaded; grouped further would add an allocation in the hot path)
         int numericOffset, DataType numericType, object numericExpected,
         VariableLengthSchema schema, IStorageEngine engine, bool enableCaching,
         bool fixedWidth, string[]? fixedColumns, DataType[]? fixedTypes)
@@ -510,7 +515,7 @@ public bool MoveNext()
             }
         }
 
-        private bool InitAndMoveNext()
+        private bool InitAndMoveNext() // NOSONAR:S3776 - init resolves the WHERE via an ordered fast-path cascade and seeds the phase machine; each branch has distinct cleanup
         {
             _schema = _table.BuildVariableLengthSchema();
             _engine = _table.GetOrCreateStorageEngine();

src/SharpCoreDB/DataStructures/Table.cs

@@ -266,7 +266,7 @@ private Dictionary<string, int> GetColumnIndexCache()
     /// Gets or sets whether this table uses the fixed-width record layout (out-of-line overflow).
     /// Persisted in table metadata so a database created with the flag reopens correctly.
     /// </summary>
-    public bool IsFixedWidthRecords
+    public bool IsFixedWidthRecords // NOSONAR:S2292 - backing field is read/written directly across the Table.* partial files and metadata round-trip; auto-property would not remove the field
     {
         get => _fixedWidthRecords;
         set => _fixedWidthRecords = value;

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

@@ -523,7 +523,7 @@ private static bool IsInsertStatement(string sql)
     /// back to the general regex path. Returns raw literal text (quotes included) so the caller
     /// still converts via <see cref="SqlParser.ParseValue"/>.
     /// </summary>
-    private static bool TryScanCanonicalDml(
+    private static bool TryScanCanonicalDml( // NOSONAR:S3776 - defensive canonical-shape scanner; each guard rejects a deviation to the regex fallback and must stay sequential to stay allocation-free
         string sql,
         out string table,
         out string setCol,
@@ -965,7 +965,7 @@ private bool TryParseDeleteForBatch(string sql, out string tableName, out string
     /// ✅ FIX: Always use outer transaction to prevent concurrent write corruption.
     /// </summary>
     [MethodImpl(MethodImplOptions.AggressiveOptimization)]
-    public void ExecuteBatchSQL(IEnumerable<string> sqlStatements)
+    public void ExecuteBatchSQL(IEnumerable<string> sqlStatements) // NOSONAR:S3776 - grouped multi-table batch dispatcher (INSERT fast path / UPDATE / DELETE / fallback) under one transaction; extraction would fragment the single-lock contract
     {
         ArgumentNullException.ThrowIfNull(sqlStatements);
 

src/SharpCoreDB/Services/SqlParser.Core.cs

@@ -296,7 +296,7 @@ public List<Dictionary<string, object>> ExecuteQuery(CachedQueryPlan plan, Dicti
     /// Returns false (falling back to the full parser) for any condition that cannot be
     /// handled with exact parity to the legacy string-based path.
     /// </summary>
-    private bool TryExecuteSimpleSelect(
+    private bool TryExecuteSimpleSelect( // NOSONAR:S3776 - guarded fast-path cascade (indexed point lookup -> legacy WHERE-string fallback); each guard preserves exact parity with the parser and shares the fall-through
         SimpleSelectPlan simple,
         Dictionary<string, object?>? parameters,
         out List<Dictionary<string, object>> results)

src/SharpCoreDB/Services/Storage.Append.cs

@@ -422,8 +422,6 @@ public bool OverwriteRecordAt(string path, long offset, byte[] data)
     {
         ArgumentNullException.ThrowIfNull(data);
 
-        bool inTransaction = IsInTransaction;
-
         bool encryptWrites = ShouldEncryptWrites(path);
         byte[] record = EncryptRecord(data, encryptWrites);
         int recordLength = record.Length;
@@ -821,20 +819,17 @@ private void FlushBufferedOverwrites()
         if (!bufferedOverwrites.IsEmpty &&
             bufferedOverwrites.TryGetValue(path, out var buffered) &&
             buffered.TryGetValue(offset, out var newRecord) &&
-            newRecord.Length > 0)
+            newRecord.Length is > 0 and <= MaxRecordSize)
         {
-            if (newRecord.Length <= MaxRecordSize)
-            {
-                byte[] bufferedPayload = new byte[newRecord.Length];
-                Buffer.BlockCopy(newRecord, 0, bufferedPayload, 0, newRecord.Length);
-
-                if (UseRecordEncryption && FileHasEncryptedHeader(path))
-                {
-                    return DecryptRecord(bufferedPayload);
-                }
+            byte[] bufferedPayload = new byte[newRecord.Length];
+            Buffer.BlockCopy(newRecord, 0, bufferedPayload, 0, newRecord.Length);
 
-                return bufferedPayload;
+            if (UseRecordEncryption && FileHasEncryptedHeader(path))
+            {
+                return DecryptRecord(bufferedPayload);
             }
+
+            return bufferedPayload;
         }
 
         // PERF: Use cached SafeFileHandle + RandomAccess instead of opening a new