yangjj-iso opened a new issue, #1143: URL: https://github.com/apache/incubator-seata-go/issues/1143
### ✅ Verification Checklist - [x] 🔍 I have searched the [existing issues](https://github.com/apache/incubator-seata-go/issues) and confirmed this is not a duplicate - [x] 🛠️ I am willing to try to fix this bug myself. ### 🚀 Go Version 1.26.1 ### 📦 Seata-go Version master (local checkout), commit `9992f1b` ### 💾 Operating System 🪟 Windows ### 📝 Bug Description `MysqlXAConn.Recover` in `pkg/datasource/sql/xa/mysql_xa_connection.go` reads the xid out of the `XA RECOVER` result set with a type assertion to `string`: ```go gtridAndbqual, ok := dest[3].(string) if !ok { return nil, errors.New("the protocol of XA RECOVER statement is error") } ``` The driver never puts a `string` there. `go-sql-driver/mysql` v1.6.0, the version this repo pins, reads a text-protocol result set with `readLengthEncodedString`: ```go func readLengthEncodedString(b []byte) ([]byte, bool, int, error) ``` and assigns the result straight into `dest[i]` (`packets.go:770`). Every column of a text-protocol row therefore arrives as `[]byte`, whatever the column type is — the driver's own code says as much eleven lines later, where it asserts `dest[i].([]byte)` to parse a datetime (`packets.go:781`). So the assertion fails on the first row and `Recover` answers `the protocol of XA RECOVER statement is error` for any real connection, whatever the server returned. Two things have hidden this: - the unit test's mock hands back a Go `string`, so the assertion succeeds there and the test passes; - `Recover` has no production callers yet, only tests, so nothing exercises it against a driver. That second point also means this is a latent bug rather than one users are hitting today. It is worth fixing because a recovery scan is the first thing that will call it. The PostgreSQL sibling in the same package already gets this right, and its shape is exactly what the MySQL one is missing: ```go switch v := dest[0].(type) { case string: xids = append(xids, v) case []byte: xids = append(xids, string(v)) default: return nil, errors.New("the protocol of postgres prepared transaction query is error") } ``` There is a second defect in the same function: the result set is never closed. `PostgresXAConn.Recover` does `defer rows.Close()`; the MySQL one does not, so a recovery scan strands the connection it borrowed. `sqlclosecheck` is enabled in `.golangci.yml` but cannot catch it, because this is a `driver.Rows` rather than a `*sql.Rows`. The loop also carries a leftover debug print — `fmt.Printf("gtr: %v", gtridAndbqual)`, no newline, straight to stdout — once per recovered branch. ### 🔄 Steps to Reproduce Hand `Recover` the value shape the driver produces, rather than a string. Against current master: ```go rows := &mysqlMockRows{data: [][]interface{}{ {[]byte("1"), []byte("3"), []byte("0"), []byte("xid")}, {[]byte("1"), []byte("11"), []byte("0"), []byte("another_xid")}, }} mockConn.EXPECT().QueryContext(gomock.Any(), "XA RECOVER", gomock.Any()).AnyTimes().Return(rows, nil) c := &MysqlXAConn{Conn: mockConn} got, err := c.Recover(context.Background(), TMStartRScan|TMEndRScan) ``` ``` Recover() error = the protocol of XA RECOVER statement is error, want nil --- FAIL: TestMysqlXAConn_RecoverDriverValues ``` The existing `TestMysqlXAConn_Recover` passes alongside it, because its rows hold `"xid"` rather than `[]byte("xid")`. ### ✅ Expected Behavior `Recover` returns the xids that `XA RECOVER` listed, and releases the result set before it returns. ### ❌ Actual Behavior It returns `the protocol of XA RECOVER statement is error` on the first row, and leaves the result set open. ### 💡 Possible Solution Accept `[]byte` as well as `string` for the xid column, the way `PostgresXAConn.Recover` already does, and `defer res.Close()` after the query. I have the change and a regression test that fails on master ready to send. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
