Fix prepared statement #734
Conversation
When there are many args and maxAllowedPacket is not enough, writeExecutePacket() attempted to use STMT_LONG_DATA even for 0byte string. But writeCommandLongData() doesn't support 0byte data. So it caused to send malfold packet. This commit loosen threshold for using STMT_LONG_DATA.
packets.go
Outdated
| longDataSize := mc.maxAllowedPacket / stmt.paramCount | ||
| if longDataSize < 16 { | ||
| longDataSize = 16 | ||
| } |
There was a problem hiding this comment.
Maybe, 16byte is too short?
|
Can we have a test for this? It seems like you were able to confirm the bug report somehow. |
|
I confirmed 475ca26 fails without this patch, |
packets.go
Outdated
| @@ -912,6 +912,12 @@ func (stmt *mysqlStmt) writeExecutePacket(args []driver.Value) error { | |||
| const minPktLen = 4 + 1 + 4 + 1 + 4 | |||
| mc := stmt.mc | |||
|
|
|||
| // Determine threshould dynamically to avoid packet size shortage as possible. | |||
There was a problem hiding this comment.
s/threshould/threshold/
What do you mean by the "as possible"? When possible? As much as possible?
Are there still cases where the driver sends too short packets?
There was a problem hiding this comment.
What do you mean by the "as possible"? When possible? As much as possible?
Sorry, I'm not English speaker. I meant it's OK for most realistic use cases,
although there can be edge cases.
Are there still cases where the driver sends too short packets?
No, always send more than 64 bytes.
"shortage" meant there is chance which stmt_exec packet is bigger than
max_allowed_packet.
For example, user can set very small max_allowed_packet. In such case,
packet can be bigger than max_allowed_packet even when all data is sent
with STMT_LONG_DATA.
There was a problem hiding this comment.
I'm not good English writer. Could you rewrite this comment?
There was a problem hiding this comment.
just remove the "as possible"
* Fix prepared statement When there are many args and maxAllowedPacket is not enough, writeExecutePacket() attempted to use STMT_LONG_DATA even for 0byte string. But writeCommandLongData() doesn't support 0byte data. So it caused to send malfold packet. This commit loosen threshold for using STMT_LONG_DATA. * Change minimum size of LONG_DATA to 64byte * Add test which reproduce issue 730 * TestPreparedManyCols test only numParams = 65535 case * s/as possible//
When there are many args and maxAllowedPacket is not enough,
writeExecutePacket() attempted to use STMT_LONG_DATA even for
0byte string.
But writeCommandLongData() doesn't support 0byte data. So it
caused to send malfold packet.
This commit loosen threshold for using STMT_LONG_DATA.
fixes #730
Checklist