Skip to content

Compare index values at one method, across their representations - #1331

Merged
noha merged 1 commit into
mainfrom
index-values-compare-as-bytes
Oct 7, 2026
Merged

noha merged 1 commit into
mainfrom
index-values-compare-as-bytes

Conversation

@noha

@noha noha commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

A page read back from disk holds its values as byte arrays, a page written in this image and a journal entry hold SoilObjectIds, and the two are never equal. Four places compared them with #= and failed after a reopen:

  • SoilIndexItemsPage>>#itemIndexForKey:value: and SoilSkipListIterator>>#findPageFor:value: - #removeKey:value: went into its absent block for an entry that is there, and adding a pair the index held appended a second entry for it, in a running database and in a replay.
  • SoilAbstractBTreeDataPage>>#findItem:with: - the same miss, and the walk did not stop at the first page, so it read page 0 and failed on the stream position.
  • SoilRemoveKeyEntry>>#isForItem:index: - a reader older than a removal lost the key, on unique indexes too.

All of them, and SoilAddKeyEntry>>#isForItem:index:, now ask SoilIndexItemsPage class>>#value:matches:. Across representations the bytes decide; within one, #= does, because an object id without an index yet (a new object before commit) only matches itself - as bytes all of them would look the same and a second new value under a key would overwrite the first.

Carries the red test of duplicate-key-remove-after-reopen (559587cb) and adds tests for the re-add, the replay, the MVCC restore of a removal on unique and duplicate keys, and the BTree walk past the first page. The replay test is skipped on BTree: SoilBTreePage>>#cleanRemovedIn: removes a tombstone by key and can take the live duplicate next to it, which is a fault of its own.

A page read back from disk holds its values as byte arrays, a page written in
this image and a journal entry hold SoilObjectIds, and the two are never equal.
Four places compared them with #= and failed after a reopen:

- SoilIndexItemsPage>>#itemIndexForKey:value: and
  SoilSkipListIterator>>#findPageFor:value: - #removeKey:value: went into its
  absent block for an entry that is there, and adding a pair the index held
  appended a second entry for it, in a running database and in a replay.
- SoilAbstractBTreeDataPage>>#findItem:with: - the same miss, and the walk did
  not stop at the first page, so it read page 0 and failed on the stream position.
- SoilRemoveKeyEntry>>#isForItem:index: - a reader older than a removal lost the
  key, on unique indexes too.

All of them, and SoilAddKeyEntry>>#isForItem:index:, now ask
SoilIndexItemsPage class>>#value:matches:. Across representations the bytes
decide; within one, #= does, because an object id without an index yet (a new
object before commit) only matches itself - as bytes all of them would look the
same and a second new value under a key would overwrite the first.

Carries the red test of duplicate-key-remove-after-reopen (559587cb) and adds
tests for the re-add, the replay, the MVCC restore of a removal on unique and
duplicate keys, and the BTree walk past the first page. The replay test is
skipped on BTree: SoilBTreePage>>#cleanRemovedIn: removes a tombstone by key and
can take the live duplicate next to it, which is a fault of its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@noha
noha requested a review from MarcusDenker October 7, 2026 12:05
@noha
noha merged commit 5ff4910 into main Oct 7, 2026
5 checks passed
@noha
noha deleted the index-values-compare-as-bytes branch October 10, 2026 14:51
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.

1 participant