Skip to content
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.

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

Fix limit prefix delete case #368

Merged
merged 8 commits into from
Apr 23, 2020
Merged

Conversation

cheme
Copy link
Collaborator

@cheme cheme commented Apr 4, 2020

My previous PR to delete by prefix contains an ordering error, this PR fixes that.

The test change in this PR shows the problematic case.

The pr make use of the existing iter by prefix iterator for the corner case in rocksdb, and change end_prefix api to be safest, so it breaks #365.
One of the corner case that may be tricky to implement in #365 is this one, basically iterating by prefix [255] with existing key [255, 255], [255, 255, 255] ... in the db.
Ok, I am wrong this should work without change (since #365 only change read opt to set upper bound).
So this only require changing the call to end_prefix in #365

@cheme cheme requested a review from ordian April 4, 2020 10:47
kvdb-rocksdb/src/lib.rs Outdated Show resolved Hide resolved
kvdb/src/lib.rs Show resolved Hide resolved
kvdb-rocksdb/src/lib.rs Outdated Show resolved Hide resolved
kvdb-rocksdb/src/lib.rs Outdated Show resolved Hide resolved
kvdb-rocksdb/src/lib.rs Outdated Show resolved Hide resolved
Co-Authored-By: Andronik Ordian <write@reusable.software>
kvdb-rocksdb/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM!

@ordian ordian merged commit 692aa9d into paritytech:master Apr 23, 2020
ordian added a commit that referenced this pull request Apr 23, 2020
* master:
  Fix limit prefix delete case (#368)
  Add arbitrary trait implementation (#378)
ordian added a commit that referenced this pull request May 5, 2020
* master: (56 commits)
  primitive-types: add no_std support for serde feature (#385)
  Add Rocksdb Secondary Instance Api (#384)
  kvdb-rocksdb: update rocksdb to 0.14 (#379)
  prepare releases for a few crates (#382)
  uint: fix UB in uint::from_big_endian (#381)
  Fix limit prefix delete case (#368)
  Add arbitrary trait implementation (#378)
  kvdb-rocksdb: optimize and rename iter_from_prefix  (#365)
  bump parity-util-mem (#376)
  parity-util-mem: fix for windows (#375)
  keccak-hash: fix bench and add one for range (#372)
  [parity-crypto] Release 0.6.1 (#373)
  keccak-hash: bump version to 0.5.1 (#371)
  keccak-hash: add keccak256_range and keccak512_range functions (#370)
  Allow pubkey recovery for all-zero messages (#369)
  Delete by prefix operator in kvdb (#360)
  kvdb: no overlay (#313)
  Ban duplicates of parity-uil-mem from being linked into the same program (#363)
  Use correct license ID (#362)
  Memtest example for Rocksdb (#349)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants