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

feat:(sql function) support last_insert_id with expr #1248

Merged

Conversation

xujihui1985
Copy link
Contributor

mysql support parameter for LAST_INSERT_ID(expr), it is useful in upsert query when we want to get the updated recored id
ref: https://dev.mysql.com/doc/refman/8.0/en/information-functions.html#function_last-insert-id

mysql support parameter for LAST_INSERT_ID(expr), it is useful in
upsert query when we want to get the updated recored id
ref: https://dev.mysql.com/doc/refman/8.0/en/information-functions.html#function_last-insert-id
Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

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

Hey @xujihui1985, thanks for sending us this contribution! 🙏

The code looks good; I just had one small suggestion for reusing some existing int conversion code.

I think the last thing we need it getting some test coverage. There are already some tests for last_update_id() in script_queries.go that should be pretty easy to extend.

sql/expression/function/queryinfo.go Outdated Show resolved Hide resolved
@fulghum fulghum self-assigned this Sep 9, 2022
1. use sql.Int64.Convert instead of duplicate toInt64
2. add test to improve converage
Copy link
Contributor

@fulghum fulghum left a comment

Choose a reason for hiding this comment

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

Hey @xujihui1985, thank you for this contribution! Awesome to have support for expressions passed to the last_insert_id function. 🎉 Thank you! 🙏

Your code changes look great. I found two very small edits to get the CI tests passing, so I went ahead and applied them for you. I'm approving and will merge this one into main.

@fulghum fulghum merged commit 3819ea2 into dolthub:main Sep 12, 2022
@xujihui1985
Copy link
Contributor Author

Hey @xujihui1985, thank you for this contribution! Awesome to have support for expressions passed to the last_insert_id function. 🎉 Thank you! 🙏

Your code changes look great. I found two very small edits to get the CI tests passing, so I went ahead and applied them for you. I'm approving and will merge this one into main.

Thanks, this my pleasure to have contribution to this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants