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

Use default sql package functions for Scanner and Valuer #32

Merged
merged 2 commits into from
Aug 18, 2023

Conversation

RangelReale
Copy link
Contributor

This change allows any type that is already supported by database/sql to be used for sql Scan and Value functions.

It uses the unexported database/sql.convertAssign function via go:linkname, as the Go developers won't make it public, and copying it as suggested in the thread would be a very high burden given the size of it.

@moznion
Copy link
Owner

moznion commented Aug 16, 2023

Thank you for your contribution, @RangelReale!

It seems a great suggestion, but I'm afraid to use the private function convertAssign by picking up with go:linkname which comes from unsafe package. According to the golang/go#24258 thread, the committers said "I don't want the extra burden of maintaining convertAssign as a public API", I think this implies it has the potential to have the breaking change without notices, so the behavior of this function might not have the consistency across the runtime versions.

And in another issue golang/go#35697, a committer suggested "To date, I've classified this as "a little copying is fine", especially as the code doesn't change often. I think if we gathered sufficient real world examples of this copied in the wild, we should consider exporting it in some way". I suppose this would be a safer way to realize the feature. How about copying the function code instead of the hacky way? I'm feeling good at it. I think this might be gonna be a chance to reboot that feature proposal about the exposure of the API if we're going to face a massive change about that function in the future.

@RangelReale
Copy link
Contributor Author

Thank you for your contribution, @RangelReale!

It seems a great suggestion, but I'm afraid to use the private function convertAssign by picking up with go:linkname which comes from unsafe package. According to the golang/go#24258 thread, the committers said "I don't want the extra burden of maintaining convertAssign as a public API", I think this implies it has the potential to have the breaking change without notices, so the behavior of this function might not have the consistency across the runtime versions.

And in another issue golang/go#35697, a committer suggested "To date, I've classified this as "a little copying is fine", especially as the code doesn't change often. I think if we gathered sufficient real world examples of this copied in the wild, we should consider exporting it in some way". I suppose this would be a safer way to realize the feature. How about copying the function code instead of the hacky way? I'm feeling good at it. I think this might be gonna be a chance to reboot that feature proposal about the exposure of the API if we're going to face a massive change about that function in the future.

Done, if they suggest doing this way, then it is better to follow.

@moznion moznion merged commit c95f59c into moznion:main Aug 18, 2023
@moznion
Copy link
Owner

moznion commented Aug 18, 2023

Thank you so much, @RangelReale!

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