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: iter method to iterate with params #133

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

prasannavl
Copy link
Contributor

@prasannavl prasannavl commented Aug 4, 2024

Summary

  • Add an iter method that accepts arguments to allow iterating through results.
  • Currently, there is no way to iterate providing args to bind to.
  • Symbol[Iterator] works for ones without any args.
  • This PR exposes the method as iter that works both with and without bind values.

Why

Without this, there's no way to iterate on large results efficiently. The current all methods and other variants always have to return the full query, which is not an ideal way to scan through the DB.

Alternative approaches

  • The current Symbol[iterator] works for items without bind params, but any bind params will be reset on execution. Can just remove the resetting, but in that case, it breaks compatibility and older behavior, since it's the responsibility of the user of the prepared statement to ensure it's also reset and bound.

Copy link
Member

@DjDeveloperr DjDeveloperr left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@DjDeveloperr DjDeveloperr merged commit 35156ce into denodrivers:main Aug 6, 2024
2 of 5 checks passed
@pitekantrop
Copy link

This also breaks backwards compatibility as bound statements are no longer iterable. This worked before

const [row] = db.prepare(`SELECT sqlite_version() AS ver WHERE ver > ?`).bind(0)

But now it throws with:

error: Uncaught (in promise) Error: Statement already bound to values
    if (this.#bound) throw new Error("Statement already bound to values");
                           ^
    at Statement.#bindAll (https://jsr.io/@db/sqlite/0.12.0/src/statement.ts:376:28)
    at Statement.iter (https://jsr.io/@db/sqlite/0.12.0/src/statement.ts:703:18)
    at iter.next (<anonymous>)
    at file:///<snipped>.ts:4:8
    at eventLoopTick (ext:core/01_core.js:175:7)

@DjDeveloperr
Copy link
Member

DjDeveloperr commented Sep 17, 2024

Ohh, there's a little detail I missed here - it's always resetting binding values. I'll push a fix and make a new release. Thanks for reporting!

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

Successfully merging this pull request may close these issues.

3 participants