Skip to content

sonar: fix mechanical and new-code issues (batch 1)#396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MPCoreDeveloper merged 1 commit into from
Sep 4, 2026
Merged

sonar: fix mechanical and new-code issues (batch 1) #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 13 additions & 18 deletions src/SharpCoreDB/DataStructures/OverflowArena.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -121,34 +121,29 @@ public long Write(byte[] payload)
private bool TryReuseFreeBlock(byte[] payload, out long offset)
{
offset = 0;
if (!_freeByLength.TryGetValue(payload.Length, out var offsets))
if (!_freeByLength.TryGetValue(payload.Length, out var offsets) || offsets.Count == 0)
{
return false;
}

while (offsets.Count > 0)
{
offset = offsets[^1];
offsets.RemoveAt(offsets.Count - 1);
offset = offsets[^1];
offsets.RemoveAt(offsets.Count - 1);

if (_storage.OverwriteRecordAt(_filePath, offset, payload))
if (_storage.OverwriteRecordAt(_filePath, offset, payload))
{
if (offsets.Count == 0)
{
if (offsets.Count == 0)
{
_freeByLength.Remove(payload.Length);
}

_cache[offset] = payload;
_blockReuses++;
return true;
_freeByLength.Remove(payload.Length);
}

// In-place overwrite refused (e.g. transaction active): keep the block free for a
// later write and try the next candidate; if none succeeds we fall back to append.
offsets.Add(offset);
break;
_cache[offset] = payload;
_blockReuses++;
return true;
}

// In-place overwrite refused (e.g. transaction active): keep the block free for a later
// write and fall back to appending.
offsets.Add(offset);
offset = 0;
return false;
}
Expand Down
27 changes: 12 additions & 15 deletions src/SharpCoreDB/DataStructures/Table.CRUD.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2272,8 +2272,8 @@ oldHashValues is not null &&
var newPkVal = row[this.Columns[this.PrimaryKeyIndex]]?.ToString() ?? string.Empty;
if (!string.Equals(newPkVal, oldPkValue?.ToString(), StringComparison.Ordinal))
{
if (!string.IsNullOrEmpty(oldPkValue?.ToString()))
this.Index.Delete(oldPkValue!.ToString()!);
if (oldPkValue?.ToString() is { Length: > 0 } oldPkString)
this.Index.Delete(oldPkString);
if (!string.IsNullOrEmpty(newPkVal))
this.Index.Insert(newPkVal, oldPosition);
}
Expand DownExpand Up@@ -2692,12 +2692,9 @@ private bool HasExplicitNamedIndex(string column)
// Transactional delete: buffer the physical offsets so the in-place marker is
// applied at COMMIT (rollback discards the buffer). Durable in O(delete) — the
// flush-time full-file rewrite is no longer needed for transactional deletes.
foreach (var position in positions)
foreach (var position in positions.Where(static position => position >= 0))
{
if (position >= 0)
{
this.storage.BufferTombstoneForCommit(DataFile, position);
}
this.storage.BufferTombstoneForCommit(DataFile, position);
}
}
else
Expand DownExpand Up@@ -2755,10 +2752,10 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
List<long>? remainingPositions = null;
if (!this.storage.AreRecordsEncrypted(DataFile))
{
remainingPositions = ScanLegacyPlaintextRemainingKeys(DataFile, pkCol, deletedKeys);
remainingPositions = ScanLegacyPlaintextRemainingKeys(DataFile, deletedKeys);
}

remainingPositions ??= ScanLegacyRemainingKeysViaStorage(DataFile, pkCol, deletedKeys);
remainingPositions ??= ScanLegacyRemainingKeysViaStorage(DataFile, deletedKeys);
if (remainingPositions is { Count: > 0 })
{
TombstoneDeletedPositions(remainingPositions.ToArray());
Expand All@@ -2772,7 +2769,7 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
/// <see langword="null"/> when the raw layout could not be parsed safely (the caller then falls
/// back to the storage-layer scan, which understands per-record encryption).
/// </summary>
private List<long>? ScanLegacyPlaintextRemainingKeys(string dataFile, string pkCol, HashSet<string> deletedKeys)
private List<long>? ScanLegacyPlaintextRemainingKeys(string dataFile, HashSet<string> deletedKeys)
{
var matches = new List<long>();
try
Expand DownExpand Up@@ -2822,7 +2819,7 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
break;
}

if (TryReadPrimaryKeyFromLegacyRecord(recordData, pkCol, out var pkStr) && deletedKeys.Contains(pkStr))
if (TryReadPrimaryKeyFromLegacyRecord(recordData, out var pkStr) && deletedKeys.Contains(pkStr))
{
matches.Add(position);
}
Expand All@@ -2843,12 +2840,12 @@ private void TombstoneRemainingVersionsOfDeletedKeys(List<(long storagePosition,
/// plaintext raw scan is unavailable): iterates the records through
/// <c>storage.ReadAllRecords</c>, which decrypts payloads and already skips tombstone markers.
/// </summary>
private List<long> ScanLegacyRemainingKeysViaStorage(string dataFile, string pkCol, HashSet<string> deletedKeys)
private List<long> ScanLegacyRemainingKeysViaStorage(string dataFile, HashSet<string> deletedKeys)
{
var matches = new List<long>();
foreach (var (recordOffset, recordData) in this.storage!.ReadAllRecords(dataFile))
foreach (var (recordOffset, recordData) in this.storage.ReadAllRecords(dataFile))
{
if (TryReadPrimaryKeyFromLegacyRecord(recordData, pkCol, out var pkStr) && deletedKeys.Contains(pkStr))
if (TryReadPrimaryKeyFromLegacyRecord(recordData, out var pkStr) && deletedKeys.Contains(pkStr))
{
matches.Add(recordOffset);
}
Expand All@@ -2861,7 +2858,7 @@ private List<long> ScanLegacyRemainingKeysViaStorage(string dataFile, string pkC
/// Walks a legacy variable-length record and returns the value of the primary-key column
/// (the same layout walk used by the reopen index rebuild).
/// </summary>
private bool TryReadPrimaryKeyFromLegacyRecord(byte[] recordData, string pkCol, out string? pkValue)
private bool TryReadPrimaryKeyFromLegacyRecord(byte[] recordData, out string? pkValue)
{
pkValue = null;
try
Expand Down
2 changes: 1 addition & 1 deletion src/SharpCoreDB/Database/Core/Database.Core.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -35,7 +35,7 @@ public partial class Database : IDatabase, IDisposable, IAsyncDisposable
// Diagnostics: number of DELETE statements routed through the canonical structured batch path
// (proves the scanner/batch fast paths are engaged; 0 means everything fell back to regex).
private static long _canonicalDeleteStatements;
public long CanonicalDeleteStatementsParsed => Interlocked.Read(ref _canonicalDeleteStatements);
public static long CanonicalDeleteStatementsParsed => Interlocked.Read(ref _canonicalDeleteStatements);

private readonly IServiceProvider _serviceProvider;
private readonly IStorage storage;
Expand Down
4 changes: 2 additions & 2 deletions tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -299,12 +299,12 @@ public void CanonicalBatchDelete_EngagesStructuredPath()
InsertDocs(db, 1, 50);
db.Flush();

var before = ((SharpCoreDB.Database)db).CanonicalDeleteStatementsParsed;
var before = SharpCoreDB.Database.CanonicalDeleteStatementsParsed;
db.ExecuteBatchSQL(BuildDeletes(1, 10));
db.Flush();

Assert.True(
((SharpCoreDB.Database)db).CanonicalDeleteStatementsParsed >= before + 10,
SharpCoreDB.Database.CanonicalDeleteStatementsParsed >= before + 10,
"canonical DELETE statements must flow through the structured batch path");
Assert.Equal(40, db.ExecuteQuery("SELECT id FROM docs").Count);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/SharpCoreDB.Tests/FixedWidthRecordLayoutTests.cs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -58,7 +58,7 @@ public void RoundTrip_AllColumnTypes_PointAndFullScan()
Assert.Single(row);
Assert.Equal("beta", row[0]["name"]);
Assert.Equal(2.5, Convert.ToDouble(row[0]["score"]));
Assert.Equal(false, Convert.ToBoolean(row[0]["flag"]));
Assert.False(Convert.ToBoolean(row[0]["flag"]));

var all = db.ExecuteQuery("SELECT * FROM t ORDER BY id");
Assert.Equal(2, all.Count);
Expand Down
Loading