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.

Sign up for GitHub

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

Jump to bottom

Add options to approximateSize & compactRange #85

Closed
1 of 7 tasks
vweevers opened this issue Oct 1, 2019 · 2 comments
Closed
1 of 7 tasks

Add options to approximateSize & compactRange #85

vweevers opened this issue Oct 1, 2019 · 2 comments
Labels
bug Something isn't working

Comments

@vweevers
Copy link
Member

vweevers commented Oct 1, 2019 •
edited
Loading

If you do:

const db = require('level')('db') // or similar
db.approximateSize('a', 'b', { valueEncoding: 'json' }, callback)

Then encoding-down correctly encodes the start (a) and end (b) arguments as JSON, but it also passes the options argument down to leveldown, which doesn't support that: it expects a callback at that index.

Add options to approximateSize() and compactRange() in:

  • leveldown
    • PR or commit: pending
    • Release: pending
  • rocksdb
    • PR or commit: pending
    • Release: pending
  • leveldown-hyper
@vweevers vweevers added the bug Something isn't working label Oct 1, 2019
@vweevers
Copy link
Member Author

vweevers commented Oct 6, 2019

Similarly, iterator.seek() and chained batch operations (Level/levelup#633) don't support encoding options. In general, all methods should start following the basic signature fn(..., options, callback).

@vweevers vweevers added this to Level Nov 21, 2021
@vweevers vweevers moved this to Todo in Level Nov 21, 2021
vweevers added a commit to Level/classic-level that referenced this issue Feb 19, 2022
@vweevers
Upgrade from abstract-leveldown to abstract-level
a87ca04 
On the C++ side:

- Replace asBuffer options with encoding options
- Refactor iterator_next to work for nextv(). We already had a
  iterator.ReadMany(size) method in C++, with a hardcoded size.
  Now size is taken from the JS argument to _nextv(size). The
  cache logic for next() is the same as before.
  Ref Level/community#70
  Ref Level/abstract-level#12
- Use std::vector<Entry> in iterator.cache_ instead of
  std::vector<std::string> so that the structure of the cache
  matches the desired result of nextv() in JS.

On the JS side:

- Use classes for ChainedBatch, Iterator, ClassicLevel
- Defer approximateSize() and compactRange()
- Encode arguments of approximateSize() and compactRange(). Ref
  Level/community#85
- Add promise support to additional methods
- Remove tests that were copied to abstract-level.

This is the most of it, a few more changes are needed in follow-up
commits.
vweevers added a commit to Level/classic-level that referenced this issue Feb 19, 2022
@vweevers
Breaking: upgrade from abstract-leveldown to abstract-level
b9ba3dd 
On the C++ side:

- Replace asBuffer options with encoding options
- Refactor iterator_next to work for nextv(). We already had a
  iterator.ReadMany(size) method in C++, with a hardcoded size.
  Now size is taken from the JS argument to _nextv(size). The
  cache logic for next() is the same as before.
  Ref Level/community#70
  Ref Level/abstract-level#12
- Use std::vector<Entry> in iterator.cache_ instead of
  std::vector<std::string> so that the structure of the cache
  matches the desired result of nextv() in JS.

On the JS side:

- Use classes for ChainedBatch, Iterator, ClassicLevel
- Defer approximateSize() and compactRange()
- Encode arguments of approximateSize() and compactRange(). Ref
  Level/community#85
- Add promise support to additional methods
- Remove tests that were copied to abstract-level.

This is the most of it, a few more changes are needed in follow-up
commits.
@vweevers
Copy link
Member Author

Done in classic-level (not on npm yet), including for seek(). I will also add it to rocks-level, the yet-to-be-written replacement for rocksdb. These fixes will not be backported to leveldown or rocksdb.

Repository owner moved this from Todo to Done in Level Feb 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
Level
Status: Done
Milestone
No milestone
Development

No branches or pull requests
1 participant
@vweevers