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

[D1] feat: add missing d1 meta object properties #14422

Open
wants to merge 1 commit into
base: production
Choose a base branch
from

Conversation

hrueger
Copy link

@hrueger hrueger commented May 6, 2024

Hi,
this adds docs about the missing properties to the meta object returned by stmt.all() and db.batch() d1 methods.
I'm not sure what last_row_id is exactly, can anyone help with that?

I think the information added here has quite some value - I did make a pretty weird pr to another project because I did not know how to get the number of changed rows excluding indices (since there were no docs on that). Turns out - it's pretty simple 馃槄

Thanks for looking into this!

changes: number, // the number of data rows (excluding extra indices) changed by this query
changed_db: boolean, // whether the db was changed by this query
size_after: number, // the db size after executing this query
last_row_id: number // ToDo
Copy link
Contributor

Choose a reason for hiding this comment

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

@joshthoward user facing description for this field?

Copy link
Contributor

Choose a reason for hiding this comment

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

meta has type Record<string, unknown>. We shouldn't strongly type this in the docs.

Copy link
Author

Choose a reason for hiding this comment

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

Hm. Is that expected to change frequently or what is the reason for it to be Record<string, unknown>? Big projects like prisma use it...

Copy link
Contributor

Choose a reason for hiding this comment

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

@hrueger we wouldn't change existing fields from the response without gating the change behind a compatibility date. I want to keep our documentation and Typescript types in sync.

Copy link
Author

Choose a reason for hiding this comment

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

That all makes perfect sense. Can I make a PR to update the types first, and after that this PR can be considered?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Updating the types is still a breaking change.

Copy link
Author

Choose a reason for hiding this comment

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

OK. What do you suggest?

@Maddy-Cloudflare Maddy-Cloudflare changed the title feat: add missing d1 meta object properties [D1] feat: add missing d1 meta object properties May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product:d1 D1: https://developers.cloudflare.com/d1/ size/xs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants