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

Add builtin sublevels ✨ #8

Merged
merged 5 commits into from
Jan 2, 2022
Merged

Add builtin sublevels ✨ #8

merged 5 commits into from
Jan 2, 2022

Conversation

vweevers
Copy link
Member

I initially wanted to wait with this and instead fork subleveldown and keep that functionality in a separate module, but I figured that would require breaking changes later on. In addition, I've yet to benchmark abstract-level versus levelup and abstract-leveldown; including sublevels now saves me from having to benchmark twice.

Adds a db.sublevel(name[, options]) method that works the same as subleveldown except it returns an abstract-level database instead of a levelup database. Data-wise there's no change, so an abstract-level sublevel can read sublevels previously created with (and populated by) subleveldown.

Uses code and tests from subleveldown. Where possible, I converted functional tests (against a memdown db) to unit tests (against a mocked database). The abstract test suite also repeats itself, to run both on a regular database, and a sublevel of the database.

Comes with some new features compared to subleveldown:

  • db.batch(..) takes a sublevel option on operations, to atomically commit data to multiple sublevels
  • Sublevels support Uint8Array in addition to Buffer
  • AbstractLevel#_sublevel() can be overridden to add additional methods to sublevels.

@vweevers vweevers added the enhancement New feature or request label Dec 28, 2021
@@ -1,6 +1,6 @@
The MIT License (MIT)

Copyright © 2013 Rod Vagg and the contributors to abstract-level and abstract-leveldown.
Copyright © 2013 Rod Vagg and the contributors to abstract-level.
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the and abstract-leveldown because I'd also have to list encoding-down, levelup, deferred-leveldown and subleveldown. Not sure how meaningful that is.

@mafintosh I copied code from subleveldown to here. You're the copyright holder on subleveldown. Do you want your name in the LICENSE file of abstract-level (or in code comments)?

Copy link
Member

@juliangruber juliangruber left a comment

Choose a reason for hiding this comment

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

just quick initial feedback 👏

README.md Show resolved Hide resolved
abstract-chained-batch.js Outdated Show resolved Hide resolved
abstract-level.js Outdated Show resolved Hide resolved
@vweevers
Copy link
Member Author

Review of the following is welcome: function signatures of the public API, docs (README.md and UPGRADING.md), TypeScript types, and LICENSE (see above comment). The code itself is covered by tests (now including the new sublevel option for batches) but a high-level review on that would be welcome too (in terms of code complexity, plugin compatibility, extensibility, etc).

Adds a `db.sublevel(name[, options]`) method that works the same as
`subleveldown` except it returns an `abstract-level` database instead
of a `levelup` database. Data-wise there's no change, so an
`abstract-level` sublevel can read sublevels previously created with
(and populated by) `subleveldown`.

Uses code and tests from `subleveldown`. Where possible, I converted
functional tests (against a `memdown` db) to unit tests (against a
mocked database). The abstract test suite also repeats itself, to run
both on a regular database, and a sublevel of the database.

Comes with some new features compared to `subleveldown`:

- `db.batch(..)` takes a `sublevel` option on operations, to atomically
  commit data to multiple sublevels
- Sublevels support Uint8Array in addition to Buffer
- `AbstractLevel#_sublevel()` can be overridden to add additional
  methods to sublevels.
- Add links to modules
- Replace references to `leveldown` (etc) with `classic-level` (etc)
- Add more code examples
- Document lack of constructor callback (compared to `levelup`)
- Split encodings guide into consumers and implementors sections
- Describe idempotent open in more detail
- Describe passive open
- Document that `open()` options must be accepted by constructor too
- Describe `level@7` vs `level@8`.
@vweevers
Copy link
Member Author

vweevers commented Jan 2, 2022

Merging because I have some cleanup to do on the main branch (has some fixup commits I forgot to squash). Retroactive review is of course still welcome.

@vweevers vweevers merged commit 3c7fe9b into main Jan 2, 2022
@vweevers vweevers deleted the sublevel branch January 2, 2022 13:44
vweevers added a commit that referenced this pull request Jan 9, 2022
I'm squeezing this in for similar reasons as #8. I wondered whether
these additions would be breaking. The short answer is no. In
addition, adding this now means `level-read-stream` can make use of
it without conditional code paths based on `db.supports.*`.

Ref Level/abstract-leveldown#380

Adds key and value iterators: `db.keys()` and `db.values()`. As
a replacement for levelup's `db.create{Key,Value}Stream()`. Example:

```
for await (const key of db.keys({ gte: 'a' }) {
  console.log(key)
}
```

Adds two new methods to all three types of iterators: `nextv()` for
getting many items (entries, keys or values) in one call and `all()`
for getting all (remaining) items. These methods have functional
but suboptimal defaults: `all()` falls back to repeatedly calling
`nextv()` and that in turn falls back to `next()`. Example:

```
const values = await db.values({ limit: 10 }).all()
```

Adds a lot of new code, with unfortunately some duplicate code
because I wanted to avoid mixins and other forms of complexity,
which means key and value iterators use classes that are separate
from preexisting iterators. For example, a method like `_seek()`
must now be implemented on three classes: `AbstractIterator`,
`AbstractKeyIterator` and `AbstractValueIterator`. This (small?)
problem extends to implementations and their subclasses, if they
choose to override key and value iterators to improve performance.

On the flip side, the new methods are supported across the board:
on sublevels, with encodings, with deferred open, and fully
functional. This may demonstrate one of the major benefits of
`abstract-level` over `abstract-leveldown` paired with `levelup`.

Yet todo:

- [ ] Tests
- [ ] Replace use of `level-concat-iterator` in existing tests
- [ ] After that, try another approach with a `mode` property on
      iterators, that is one of entries, keys or values (moving
      logic to if-branches... I already don't like it but it may
      result in cleaner logic downstream).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants