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

BUG: Different responses when running a D1 query in the cloud and the local mode #442

Closed
jiripospisil opened this issue Nov 22, 2022 · 0 comments · Fixed by #480
Closed
Assignees
Milestone

Comments

@jiripospisil
Copy link

What version of Wrangler are you using?

2.4.2

What operating system are you using?

Linux

Describe the Bug

Hello! Consider the following code:

export default {
  async fetch(
    request: Request,
    env: Env,
    ctx: ExecutionContext
  ): Promise<Response> {
    await env.db.exec(
      "create table if not exists foos (value text);"
    );

    const result = await env.db.prepare(
      "insert into foos (value) values (?)"
    ).bind("from param").run();

    return new Response(JSON.stringify(result));
  },
};

When ran in the cloud, the response looks like this:

{
    "meta": {
        "changes": null,
        "duration": 9.485032001510262,
        "internal_stats": null,
        "last_row_id": null,
        "served_by": "primary-78d95a04-af3b-430b-8bd5-0196cf0c6331.db3"
    },
    "results": [],
    "success": true
}

However then running in the local mode (npm start -- --local --persist), the response is different:

{
    "changes": 1,
    "duration": 15.273903001099825,
    "lastRowId": 17,
    "results": null,
    "served_by": "x-miniflare.db3",
    "success": true
}
@mrbbot mrbbot moved this to Backlog in workers-sdk Nov 28, 2022
@mrbbot mrbbot self-assigned this Nov 28, 2022
@lrapoport-cf lrapoport-cf moved this from Backlog to Selected for Development in workers-sdk Dec 2, 2022
@mrbbot mrbbot modified the milestones: 2.11.0, 2.12.0 Dec 22, 2022
mrbbot added a commit that referenced this issue Jan 23, 2023
Previously, Miniflare had its own implementations of `BetaDatabase`
and `Statement`. These were subtly different to the implementations
in the D1 Wrangler shim D1JS, causing behaviour mismatches.

This change switches the implementation to use the shim, with
Miniflare implementing the underlying D1 HTTP API instead. We'll
need to do this anyway when adding D1 support to Miniflare 3 using
`workerd`.

Specific changes:
- Throw when calling `D1PreparedStatement#run()` with statements that
  return data, closes #441
- Fix response envelope format, closes #442 and
  cloudflare/workers-sdk#2504
- Fix binding/return of `BLOB`-typed values, closes wrangler2#2527
- Fix `D1Database#raw()` return, closes cloudflare/workers-sdk#2238
  (already fixed in #474)
- Add support for `D1Database#dump()`
- Run `D1Database#{batch,exec}()` statements in implicit transaction
- Only run first statement when calling
  `D1PreparedStatement#{first,run,all,raw}()`
mrbbot added a commit that referenced this issue Jan 23, 2023
Previously, Miniflare had its own implementations of `BetaDatabase`
and `Statement`. These were subtly different to the implementations
in the D1 Wrangler shim D1JS, causing behaviour mismatches.

This change switches the implementation to use the shim, with
Miniflare implementing the underlying D1 HTTP API instead. We'll
need to do this anyway when adding D1 support to Miniflare 3 using
`workerd`.

Specific changes:
- Throw when calling `D1PreparedStatement#run()` with statements that
  return data, closes #441
- Fix response envelope format, closes #442 and
  cloudflare/workers-sdk#2504
- Fix binding/return of `BLOB`-typed values, closes wrangler2#2527
- Fix `D1Database#raw()` return, closes cloudflare/workers-sdk#2238
  (already fixed in #474)
- Add support for `D1Database#dump()`
- Run `D1Database#{batch,exec}()` statements in implicit transaction
- Only run first statement when calling
  `D1PreparedStatement#{first,run,all,raw}()`
@mrbbot mrbbot closed this as completed in 2d0fbc6 Jan 24, 2023
@github-project-automation github-project-automation bot moved this from Selected for Development to Done in workers-sdk Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants