Skip to content

Conversation

@yshrsmz
Copy link
Contributor

@yshrsmzyshrsmz commented Feb 15, 2024

resolve#12

Added MySQL implementation of :execlastid annotation

Needs some guide for other dialects if I have to cover all dialects to merge this.

@yshrsmz
Copy link
ContributorAuthor

According to the node-pg issue, node-pg does not return last insert id, so can we just throw error in this case?

brianc/node-postgres#1269

src/app.ts Outdated
}
case"pg": {
returnpg;
returnpgasany;
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of marking this as any, add a execlastidDecl method to the pg and postgres drivers that raise an error.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

thanks, fixed!: 2398fb0 (#13)

Comment on lines 181 to 185
factory.createImportSpecifier(
false,
undefined,
factory.createIdentifier("ResultSetHeader")
),
Copy link
Contributor

Choose a reason for hiding this comment

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

We should only import this if there is a execlastid query for the given file.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@yshrsmzyshrsmz requested a review from kyleconroyMarch 3, 2024 03:47
@yshrsmz
Copy link
ContributorAuthor

@kyleconroy Hi, thanks for the review. I have addressed your change requests. Please take a look 🙏

@kyleconroy
Copy link
Contributor

I just pushed some changes to fix the build, can you rebase on main?

@yshrsmz
Copy link
ContributorAuthor

@kyleconroy rebase done.

@kyleconroykyleconroy merged commit b79c5fa into sqlc-dev:mainMar 3, 2024
@yshrsmzyshrsmz deleted the execlastid branch March 3, 2024 04:34
Sign up for freeto 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.

support :execlastid query annotation

2 participants

@yshrsmz@kyleconroy