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

[Miniflare 2] Fix binding of ?N parameters in D1 prepared statements #544

Merged
merged 1 commit into from
Mar 23, 2023

Conversation

mrbbot
Copy link
Contributor

@mrbbot mrbbot commented Mar 22, 2023

better-sqlite3 expects parameters of the form ?1, ?2, ... to be bound as an object of the form { 1: params[0], 2: params[1], ...}. In #480, we accidentally removed the code that handled this case. This PR adds it back, and lifts out some common functionality into a #prepareAndBind() function. :)

Thanks @ruslantalpa for spotting the removed code.

Closes #504
Closes #526
Closes cloudflare/workers-sdk#2811
Closes cloudflare/workers-sdk#2887

`better-sqlite3` expects parameters of the form `?1, ?2, ...` to be
bound as an object of the form `{ 1: params[0], 2: params[1], ...}`.
In #480, we accidentally removed the code that handled this case.
This PR adds it back, and lifts out some common functionality into a
`#prepareAndBind()` function. :)

Thanks @ruslantalpa for spotting the removed code.

Closes #526
Closes cloudflare/workers-sdk#2811
Closes cloudflare/workers-sdk#2887
@mrbbot mrbbot requested a review from penalosa March 22, 2023 17:40
@changeset-bot
Copy link

changeset-bot bot commented Mar 22, 2023

⚠️ No Changeset found

Latest commit: fd6bb5f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mrbbot mrbbot changed the title [Miniflare 2] Fix binding of ?N parameters in D1 prepare statements [Miniflare 2] Fix binding of ?N parameters in D1 prepared statements Mar 22, 2023
@mrbbot mrbbot merged commit e49b9b3 into master Mar 23, 2023
@mrbbot mrbbot deleted the bcoll/fix-d1-numbered-parameters branch March 23, 2023 17:30
mrbbot added a commit that referenced this pull request Apr 26, 2023
Also brings forward the changes from #533 and #544

Closes DEVX-591
mrbbot added a commit that referenced this pull request Apr 28, 2023
Also brings forward the changes from #533 and #544

Closes DEVX-591
mrbbot added a commit that referenced this pull request May 9, 2023
* Re-implement D1 gateway using new storage system

Also brings forward the changes from #533 and #544

Closes DEVX-591

* fixup! Re-implement D1 gateway using new storage system
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants