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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BUG: D1 Result inconsistency with docs, live, and wrangler dev #2504

Closed
demosjarco opened this issue Jan 5, 2023 · 4 comments
Closed
Labels
d1 Relating to D1 miniflare Relating to Miniflare

Comments

@demosjarco
Copy link

demosjarco commented Jan 5, 2023

Which Cloudflare product(s) does this pertain to?

D1, Wrangler

What version of Wrangler are you using?

2.6.2

What operating system are you using?

Windows 11 21H1

Describe the Bug

The return object (https://developers.cloudflare.com/d1/platform/client-api/#return-object) docs say there is supposed to be a meta object with the stats in it.
Running locally (wrangler dev --local --persist with v2.6.2):

[
  {
    results: [ [Object], [Object], [Object] ],
    duration: 0.32249999791383743,
    lastRowId: null,
    changes: null,
    success: true,
    served_by: 'x-miniflare.db3'
  }
]

Running live (pushed to cf):

[
  {
    results: [ [Object], [Object], [Object], [Object] ],
    success: true,
    meta: {
      duration: 0.08682096004486084,
      last_row_id: null,
      changes: null,
      served_by: 'primary-<redacted>.db3',
      internal_stats: null
    }
  }
]
@demosjarco demosjarco added the bug Something that isn't working label Jan 5, 2023
@demosjarco
Copy link
Author

For now I made this filler inline if statement. But I assume the docs' way is the way it should be?

const internalTiming = 'meta' in batchList[0] ? batchList[0].meta.duration : batchList[0].duration

@rozenmd rozenmd added d1 Relating to D1 and removed bug Something that isn't working labels Jan 9, 2023
@JoshVazq
Copy link

JoshVazq commented Jan 11, 2023

Besides this In D1 "live" after a successful insert not getting back "changes" flag or "last_row_id" value.
@rozenmd is this expected?

success: true,
  meta: {
    duration: 32.86404299736023,
    last_row_id: null,
    changes: null,
    served_by: 'primary-<redacted>.db3',
    internal_stats: null
  }

@rozenmd
Copy link
Contributor

rozenmd commented Jan 23, 2023

There's some inconsistency between miniflare's local emulation of D1 and live D1 @demosjarco - you can assume the D1 docs are the expected way it should work.

@rozenmd rozenmd added the miniflare Relating to Miniflare label Jan 23, 2023
mrbbot added a commit to cloudflare/miniflare 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 to cloudflare/miniflare 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 to cloudflare/miniflare that referenced this issue Jan 24, 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
  cloudflare/workers-sdk#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
Copy link
Contributor

mrbbot commented Feb 14, 2023

Hey! 馃憢 This should be resolved by #2717, which will be released shortly! 馃檪

@mrbbot mrbbot closed this as completed Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
d1 Relating to D1 miniflare Relating to Miniflare
Projects
None yet
Development

No branches or pull requests

4 participants