fix(adi): keep every tag when the bag grows past page 255 - #144
Merged
FiveTechSoft merged 1 commit intoJul 29, 2026
Merged
Conversation
A tag-directory entry stored the per-tag header page number in a single byte. The first tag always lands on page 3, so it survived; every later add_tag appends its pages at the end of a bag that is already megabytes long, so the real page (897, 5282, 13519, ...) was truncated to its low byte. On reopen the reader jumped to the wrong page, found no F-marker and dropped the tag silently. Reported by RusSoft, whose ERP indexes ARTICULO with 22 tags: the .adi held all 22 on disk (page 2 says count=22, the 22 per-tag headers are there) yet exposed exactly one order, so the browse asked to sort by a tag the engine claimed not to exist. The page is now written and read as u32 LE over the entry's first four bytes. Those bytes were unused zeros, so a legacy entry reads back exactly the value it did before. Bags already written with a truncated page stay broken -- the high bits are gone, not recoverable -- and need a reindex. Test: 4000 rows loaded BEFORE the extra tags are created, which pushes their headers past page 255. That is the only condition the old code failed under, which is why the existing two-tag test on a tiny table kept passing.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A
.adibag keeps every tag but the first when it grows past page 255.Reported by RusSoft, whose ERP indexes
ARTICULOwith 22 tags. After areindex the bag held all 22 on disk — page 2 says
count=22and the 22per-tag header pages are there — yet reopening it exposed exactly one
order. The browse then asked to sort by a tag the engine claimed not to
exist, and the application fell back to scanning 34,595 records.
Verified by reading the tag directory of the real file:
Root cause
The tag-directory entry stored the per-tag header PAGE NUMBER in a single
byte:
The first tag always lands on page 3, so it survived. Every later
add_tagappends its pages at the end of a bag that is already megabyteslong, so the real page was truncated to its low byte. On reopen
scan_tagdirjumped to the wrong page, found no F-marker there anddropped the tag without an error.
Fix
The page is written and read as
u32 LEover the entry's first fourbytes. Those bytes were unused zeros in the old layout, so a legacy entry
reads back exactly the value it did before — existing bags stay readable.
Bags already written with a truncated page cannot be recovered (the high
bits are simply gone) and need a reindex; the commit message says so.
Test
tests/unit/abi_adi_tagdir_wide_page_test.cpploads 4,000 rows beforecreating the extra tags, which pushes their headers past page 255. That is
the only condition the old code failed under — which is why the existing
two-tag test on a tiny table kept passing. Verified on the generated bag
that the scenario really exercises it:
The test asserts the tags are listed, opened, and navigable — it
activates each one by handle and checks that the tag ordering by a
descending-valued column starts on the last-appended row. It avoids tag
ordinals and tag-name lookup on purpose, so it does not depend on
directory ordering (prepend vs append) or on name-based tag identity.
Validation
Built on MSVC 2022 x64 against
main@a253ff0:mainvsmain+ this fixFailures were compared by NAME on both builds, not by count — several of
the pre-existing ones (triggers,
openads_serverd) are flaky.