pingcap / pingcap/tidb

Improve prepared statement response

Open
#65,448 0 comments 1 reaction 0 assignees View on GitHub
type/enhancement
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:

Image

With MySQL 8.4 this happens:

Image

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:

Image

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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.