Improve prepared statement response
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Enhancement
### Observed differences with MySQL
Example client code:
```go
package main
import (
"database/sql"
_ "github.com/go-sql-driver/mysql"
)
func main() {
dsns := []string{
// "root:@tcp(127.0.0.1:3306)/test",
"root:@tcp(127.0.0.1:4000)/test",
}
for _, dsn := range dsns {
db, err := sql.Open("mysql", dsn)
if err != nil {
panic(err)
}
defer db.Close()
if _, err = db.Exec("DROP TABLE IF EXISTS t1"); err != nil {
panic(err)
}
if _, err = db.Exec("CREATE TABLE t1 (id binary(8) PRIMARY KEY, c1 varchar(255), c2 int)"); err != nil {
panic(err)
}
db.Exec("SET SESSION tidb_slow_log_threshold = 0")
sth, err := db.Prepare("INSERT INTO t1(id, c1) VALUES (?, ?)")
if err != nil {
panic(err)
}
_, err = sth.Exec([]byte{0x01}, "abc")
if err != nil {
panic(err)
}
sth.Close()
sth, err = db.Prepare("SELECT id, c1 FROM t1 WHERE id=? AND c1=?")
if err != nil {
panic(err)
}
_, err = sth.Exec([]byte{0x01}, "abc")
if err != nil {
panic(err)
}
sth.Close()
}
}
```
The `INSERT INTO t1(id, c1) VALUES (?, ?)` statements leads to this:
With MySQL 8.4 this happens:
Note that MySQL does this for the `id binary(8)` column:
- Sets the charset to 63 (binary)
- Sets the 0x0080 flag (binary)
### Possible improvement
With a few changes:
- https://github.com/dveeden/tidb/commit/e58603a0d4a25c31b5c340ddc4f28bc5d88b661a
This is the result for TiDB:
This now sets the binary flag and also sets the collation to 63.
### Why this matters
For prepared statements arguments are logged in `(*PlanCacheParamList).String()` which in turn eventually calls `DatumsToString()` which tries to format the arguments. This is used for the slowlog which eventually ends up at the TiDB Dashboard.
But the `datum` in `DatumsToString` doesn't have information about the collation, binary flag or other things.
The result is that binary data is quoted and printed as-is. This leads to garbled output.
See also:
- https://github.com/pingcap/tidb/issues/65383
- https://github.com/pingcap/tidb/pull/65384
### How we could fix this
The commit above gets more information about the markers in the query. This is send in the prepare response. However this is not yet available or used in the code that prints the slowlog. If we can fix this then the code that prints the slowlog could more easily figure out if something is a printable text argument or a binary argument that should be printed in hex.
Contributor guide
Assessment
This issue has not been assessed yet.