fix(adi): the branch descent compares only the bytes of the seek key - #162
Merged
FiveTechSoft merged 1 commit intoAug 5, 2026
Merged
Conversation
AdiIndex::compare_keys_ compares min(a, b, key_total_len_) bytes, so the dense-leaf scan honours a seek key shorter than the index key -- the prefix rule the CDX side already follows. The char-key BRANCH descent did not: it memcmp'd key_total_len_ bytes against the separator while the folded search key held only what the caller supplied, reading past the end of that buffer whenever the separator's own prefix equalled the search prefix. Honesty about the impact: I could not turn this into a wrong answer. The bytes following a std::string compare low, which selects the same child the prefix rule wants, and that held both inside the small-buffer optimisation and with a key long enough to force a heap allocation. So this is an out-of-bounds read removed and the rule stated once instead of twice, not a reproduced defect. The tests are worth more than the fix here: the ADT / ADI side of the partial-seek rule had no coverage at all, while the CDX side has had it since the seek and scope fixes. Both cases sweep every key rather than probing one, because a single wrong branch decision shows up as one miss in 1500 and any single probe is likely to land mid-page and pass regardless. The tree has to be multi-level for the branch descent to run at all, hence the row counts. Test: abi_adi_prefix_seek_multilevel_test -- 6000 rows on a 16-byte key seeking a 10-byte prefix (the shape of the ERP's `SEEK cCodigoCon + cDocumeTra` over a con+doc+seq tag), and 3000 rows on a 36-byte key seeking a 16-byte prefix so the folded key lands on the heap instead of inside std::string's inline buffer. The key is a single wide character field, not a compound expression, so a plain v1 ADI tag suffices. (cherry picked from commit ae4994a)
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
2 participants
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.
Descending a multi-level ADI bag compared the full index key length even
when the caller supplied a shorter (partial) key. Past the end of the search
string the comparison read whatever followed it in memory, so a partial seek
whose prefix equalled a separator's could take the wrong child and miss a key
that exists.
The dense leaf already applies the right rule — compare only the bytes the
caller gave — so this makes the branch descent agree with it:
Found on a Harbour/FiveWin ERP doing
dbSeek(partial_code)over an ADT companywhose article bag is deep enough to have a branch level; the same seek over
DBFCDX found the row.
Tests
New
abi_adi_prefix_seek_multilevel_test: builds a bag with enough keys to growa branch level and seeks with partial keys of several lengths, including one
whose prefix collides with the separator.
Full suite green on this branch (1292 cases).