Skip to content

fix parsing prepare response #1057

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 17, 2025
Merged

fix parsing prepare response #1057

merged 2 commits into from
Oct 17, 2025

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Oct 14, 2025

https://dev.mysql.com/doc/dev/mysql-server/9.3.0/page_protocol_com_stmt_prepare.html

warnings is optional from server, & is preceded by a reserved byte

@serprex serprex force-pushed the fix-decoding-prepare-response branch from 89bc530 to 056c336 Compare October 14, 2025 20:14
@serprex
Copy link
Contributor Author

serprex commented Oct 14, 2025

lint error fixed in #1058

@lance6716
Copy link
Collaborator

Thanks. I'll take a look later

dveeden
dveeden approved these changes Oct 16, 2025
View reviewed changes
Copy link
Collaborator

@dveeden dveeden left a comment

Choose a reason for hiding this comment

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

Looks correct when compared with the docs. Haven't tested the code.

What about CLIENT_OPTIONAL_RESULTSET_METADATA? Should there be a comment? A check to ensure it isn't set? or fully implemented?

@serprex
Copy link
Contributor Author

serprex commented Oct 16, 2025

Right now we don't support CLIENT_OPTIONAL_RESULTSET_METADATA so it should never appear (since we don't advertise), may look into creating that as a separate PR

lance6716
lance6716 reviewed Oct 17, 2025
View reviewed changes
Copy link
Collaborator

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

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

lgtm

@serprex serprex force-pushed the fix-decoding-prepare-response branch from f691e29 to 0e0e4c7 Compare October 17, 2025 15:05
@lance6716
Copy link
Collaborator

I think we'd better to check a non-zero WarningsNum in unit test but I didn't find the doc for it 😂 And for protocol we can see it's correct so LGTM

@lance6716 lance6716 merged commit dec4081 into go-mysql-org:master Oct 17, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dveeden dveeden dveeden approved these changes

@lance6716 lance6716 lance6716 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@serprex @lance6716 @dveeden