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

add execlastid implementation for mysql #13

Merged
merged 5 commits into from
Mar 3, 2024

Conversation

yshrsmz
Copy link
Contributor

@yshrsmz yshrsmz 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
Contributor Author

yshrsmz commented Feb 15, 2024

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
@@ -77,10 +83,10 @@ function createNodeGenerator(driver?: string): Driver {
return mysql2;
}
case "pg": {
return pg;
return pg as any;
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
Contributor Author

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
Contributor Author

Choose a reason for hiding this comment

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

fixed: 8d3fcaf (#13)

@yshrsmz yshrsmz requested a review from kyleconroy March 3, 2024 03:47
@yshrsmz
Copy link
Contributor Author

yshrsmz commented Mar 3, 2024

@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
Contributor Author

yshrsmz commented Mar 3, 2024

@kyleconroy rebase done.

@kyleconroy kyleconroy merged commit b79c5fa into sqlc-dev:main Mar 3, 2024
@yshrsmz yshrsmz deleted the execlastid branch March 3, 2024 04:34
Sign up for free to 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