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

Async Iterators for Database#each? #3

Open
MatthewMerrill opened this Issue Mar 15, 2019 · 0 comments

Comments

Projects
None yet
1 participant
@MatthewMerrill
Copy link

MatthewMerrill commented Mar 15, 2019

It might be handy to have a version of Database#each that returns an Async Iterator. This would remove the requirement of the callback function provided to Database#each. The if condition that checks that there is a function argument provided at the end of the arguments could be changed to handle the AsyncIterator case, or maybe a new method like eachIterated could be added.

Here's how the code might change:

// before
let stopReading = false;
await this.db.each('SELECT * FROM my_table', (row) => {
    if (someCondition(row)) {
        stopReading = true;
    }
    else if (!stopReading) {
         console.log(row);
    }
});
// after
let iterable = this.db.each('SELECT * FROM my_table');
for await (let row of iterable) {
    if (someCondition(row)) {
        break;
    }
    console.log(row);
}

Pros:

  • It removes the callback artifact of the old style.
  • Some might prefer the for await of loop syntax.

Cons:

  • The behind-the-scenes callback will be called by node-sqlite3 until node-sqlite3 has found its last row -- regardless of whether the user breaks the loop early. This might be misleading to a developer who thinks the database will stop doing work when they break. Currently users will have to make a pretty conscious decision to ignore callback results, this would hide that.
  • Async Iterators are not simple to code, especially while trying to support older versions of node.
  • If a buffer is needed to store row results that have been found but not consumed yet, this buffer might grow quite large in size if the user breaks the iterator after the first element, and might hold onto this large chunk of memory for a while.
  • Even if a way to stop Database#each operations early is added to the node-sqlite3 library, we might not be able to stop that operation because we cannot know to stop processing -- We cannot distinguish between "user is just taking a while before asking for the next item" and "user broke out of the loop".

I'm happy to attempt and submit a PR, but depending on how much code it requires and considering how the for await of could mislead users/eat memory it might make sense not to provide this method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.