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

zero-value of mysql.Result was expected by server.WriteValue, breaks in v1.11.0 #1005

Closed
stgarrity opened this issue Mar 3, 2025 · 3 comments · Fixed by #1006
Closed

zero-value of mysql.Result was expected by server.WriteValue, breaks in v1.11.0 #1005

stgarrity opened this issue Mar 3, 2025 · 3 comments · Fixed by #1006

Comments

@stgarrity
Copy link
Contributor

stgarrity commented Mar 3, 2025

d02e79a introduced new constructors for mysql.Result that always initializes Result.Resultset to be non-nil, but this breaks the logic in server.WriteValue that differentiates between a resultset response and an OK response. This in turn breaks responses to things like INSERT statements.

This is missed in tests because server/server_test.go creates Results without the constructors, and with nil Resultsets

I have a fix https://github.com/userclouds/go-mysql/tree/stg/server_missed_ok that I'm happy to PR if it makes sense and I'm not mis-understanding the intent here.

Thanks!

@stgarrity stgarrity changed the title zero-value of mysql.Result was expected by server.WriteValue, breaks in v.1.11.0 zero-value of mysql.Result was expected by server.WriteValue, breaks in v1.11.0 Mar 3, 2025
@lance6716
Copy link
Collaborator

Thanks for reporting, we have this fix PR #983 please check if it's correct

@stgarrity
Copy link
Contributor Author

yep, looks right to me! I think worth fixing the tests in server/server_test.go as well, happy to open a PR if you're OK with it.

@lance6716
Copy link
Collaborator

I think unit test can also be improved. Welcome to open PR

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 a pull request may close this issue.

2 participants