-
Notifications
You must be signed in to change notification settings - Fork 1k
Store prepared statements field definitions #1073
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
base: master
Are you sure you want to change the base?
Conversation
647b192 to
8e13d8e
Compare
| data = data[0:4] | ||
| data = append(data, paramFieldData...) | ||
| if s.ParamFields != nil && i < len(s.ParamFields) { | ||
| data = append(data, s.ParamFields[i].Dump()...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems no need to parse it? We can only forward the raw value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rationale was that for a high-level library that users can use to inspect fields metadata, it makes sense that they don't have to parse the bytes themselves. But for a pure proxy use-case, yeah, the raw value would be better. Both work for me!
|
|
||
| // StmtFieldsProvider is an optional interface that prepared statement contexts can implement | ||
| // to provide field definitions for proxy passthrough scenarios. | ||
| type StmtFieldsProvider interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to verify why we need this interface? If the only reason is to cast st.Context, I think maybe we should move the common struct of client and server package to a common package, like utils or a new package, to avoid a runtime cast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you suggesting that we create something like
type StmtFields struct {
ParamFields []*Field
ClumnFields []*Field
}
and then use it in client.Stmt?
cc34b11 to
6ad3622
Compare
|
After learning your use case I came up with a new idea. Please check if it's suitable for you. First thing is because I want to avoid runtime cast, we can add a common package to store prepared statements. Like type PreparedStmt struct {
ID uint32
Params int
Columns int
Warnings int
...
}And let Second, type PreparedStmt struct {
...
RawParamFields [][]byte
paramFields []*mysql.Field
}
func (s *PreparedStmt) GetParamFields() []*mysql.Field {
if s.paramFields == nil and s.RawParamFields != nil {
// parse it and save to paramFields
}
return s.paramFields
}I didn't know your use case, please check if there will be concurrent calling and use This is just my preliminary idea, maybe underestimate the difficulty. So let me know if you see any issues. I can help fix them. |
Hey,
When a MySQL client uses prepared statements (i.e.
COM_STMT_PREPARE/COM_STMT_EXECUTE), the server sends column and parameter field definitions in the response, and clients rely on this metadata.Currently, go-mysql's client discards these field definitions after parsing, and only stores the counts. When used in a proxy scenario, the server would use the client and send the response back, meaning that the downstream client would end up with the current dummy field definitions, which cause issues (e.g., deserialization issue due to a varchar that's interpreted as a decimal.)
This PR enables proxy implementations to transparently pass through prepared statement metadata easily.