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

[feature request] add a generator for getAllKeys? #1

Closed
huan opened this issue Aug 13, 2019 · 7 comments
Closed

[feature request] add a generator for getAllKeys? #1

huan opened this issue Aug 13, 2019 · 7 comments

Comments

@huan
Copy link
Contributor

huan commented Aug 13, 2019

Hello,

I really love your idea about pure javascript, zero dependencies, and leveldb performance.

I want to switch my project to his library, however, I found that it leaks a generator function for getAllKeys (and getAll and range as well).

To demonstrate in code, I need a code snip work like the following:

for await (const key of snapDb.getAllKeys()) {
  console.info(key)
}

Which should be defined in the class SnapDb like the following:

  public async * keys (): AsyncIterableIterator<K> {
       const it = reverse ? this._index.end() : this._index.begin()
        try {
            if (!this._index.length()) {
                complete();
                return;
            }

            while (it.valid()) {
                yield it.key();
                if (reverse) {
                    it.prev();
                } else {
                    it.next();
                }
            }
            complete();
        } catch (e) {
            throw (e)
        }
  }

Could you please consider adding the generator interface to the library? Because it would be really convenient for the end-users.

I would love to add this feature by creating a PR if you like.

@only-cliches
Copy link
Owner

only-cliches commented Aug 14, 2019

Hello Huan! The exact api you proposed here isn't possible as far as I know, my knowledge of AsyncIterableIterator is pretty new and there might be some tricks I'm not aware of.

In any case, I was able to put something together that might work for you.

I haven't pushed this to NPM yet, wanted to make sure this fits your use case well enough.

There are four new methods:

  • getAllKeysIt(reverse?: boolean)
  • getAllIt(reverse?: boolean)
  • rangeIt(lower: K, higher: K, reverse?: boolean)
  • offsetIt(offset: number, limit: number, reverse?: boolean)

All of them return a Promise which resolves to an AsyncIterableIterator. So usage would be like this:

const it = await db.getAllKeysIt();
for await (const key of it) {
  console.info(key)
}

Let me know if this works! You can grab the current master and compile it to try it out.

only-cliches pushed a commit that referenced this issue Aug 14, 2019
@huan
Copy link
Contributor Author

huan commented Aug 14, 2019

Hi Scott!

It's so great that your response as fast as light!

I'll check it out and get back to you for the result, thank you very much for the quick improvements!

@huan
Copy link
Contributor Author

huan commented Aug 14, 2019

BTW: Could we also consider to add search options for the getAllKeysIt()? Because it would be really convenient for the end-users.

In leveldb, we can use the following options with the iterator:

export interface IteratorOptions {
  gt?      : any,
  gte?     : any,
  lt?      : any,
  lte?     : any,
  reverse? : boolean,
  limit?   : number,

  prefix?  : any,
}

Here's more documentation from level README: https://github.com/Level/level#dbcreatereadstreamoptions

@huan
Copy link
Contributor Author

huan commented Aug 14, 2019

The iterator code seems works, however, my unit tests had not been passed yet. Please see #3

only-cliches pushed a commit that referenced this issue Aug 14, 2019
@only-cliches
Copy link
Owner

There's a new method called .query and .queryIt.

The arguments are:

export interface QueryArgs<K> {
    gt?: K;
    gte?: K;
    lt?: K;
    lte?: K;
    offset?: number;
    limit?: number;
    values?: boolean;
    reverse?: boolean;
}

@only-cliches
Copy link
Owner

1.1.2 is now live on NPM, this issue has been resolved. Feel free to comment if you find anything further!

@huan
Copy link
Contributor Author

huan commented Aug 15, 2019

My library had switched to SnapDB and all unit tests had been passed, published to NPM now!

Thank you so much, Scott!

See: https://github.com/huan/flash-store/#v016-may-2019---snapdb-as-backend

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

No branches or pull requests

2 participants