Skip to content

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
@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