Skip to content

Include observed WAL page_numbers when calculating SQLite3 page_count#30

Merged
Schamper merged 8 commits intofox-it:mainfrom
JSCU-CNI:sqlite-page-count-wal
Feb 23, 2026
Merged

Include observed WAL page_numbers when calculating SQLite3 page_count#30
Schamper merged 8 commits intofox-it:mainfrom
JSCU-CNI:sqlite-page-count-wal

Conversation

@JSCU-CNI
Copy link
Contributor

This PR fixes the SQLite3 page_count when dealing with WAL files. The current implementation does not seem to account for uncommited pages. @PimSanders Interested to hear your thoughts on this, does this approach seem correct to you?

@Schamper
Copy link
Member

Curious about @PimSanders opinion too. In the mean time, you know the question I'm about to ask: do you have a reproducible test case?

@JSCU-CNI
Copy link
Contributor Author

Added test in d97238b.

@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (960b819) to head (9eb4697).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
dissect/database/sqlite3/sqlite3.py 0.00% 4 Missing ⚠️
dissect/database/sqlite3/wal.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main     #30   +/-   ##
=====================================
  Coverage   0.00%   0.00%           
=====================================
  Files        151     151           
  Lines       4161    4163    +2     
=====================================
- Misses      4161    4163    +2     
Flag Coverage Δ
unittests 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 13, 2026

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing JSCU-CNI:sqlite-page-count-wal (64f63b6) with main (6149d6f)1

Open in CodSpeed

Footnotes

  1. No successful run was found on main (798cf10) during the generation of this report, so 6149d6f was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@PimSanders
Copy link
Contributor

I did run into this issue once before while working on the WAL implementation, couldn't reproduce it once I had time to look into it. I think this is the correct approach, as there isn't really any other way of determining the highest page number that I am aware of. The only suggestion I have is that the current test adds 3 other database files, would it be possible to use the script in _tools to generate a new test.sqlite-wal? That keeps the _data folder a bit cleaner.

JSCU-CNI and others added 3 commits February 20, 2026 11:58
@JSCU-CNI
Copy link
Contributor Author

The only suggestion I have is that the current test adds 3 other database files, would it be possible to use the script in _tools to generate a new test.sqlite-wal? That keeps the _data folder a bit cleaner.

Removed the unnecessary shm test file for this regression test. At the moment I do not have the time to integrate this test case in existing test files.

@Schamper Schamper merged commit 5422893 into fox-it:main Feb 23, 2026
16 of 19 checks passed
@JSCU-CNI JSCU-CNI deleted the sqlite-page-count-wal branch February 23, 2026 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants