Copilot commented on code in PR #1140:
URL:
https://github.com/apache/incubator-seata-go/pull/1140#discussion_r3760594677
##########
pkg/datasource/sql/conn_at.go:
##########
@@ -51,7 +53,34 @@ type ATConn struct {
*Conn
}
+var errATPreparedMultiSQLUnsupported = errors.New("seata AT: prepared
multi-SQL is unsupported; use Exec or ExecContext")
+
+func rejectATPreparedMultiSQL(query string) error {
+ parseCtx, err := sqlparser.DoParser(query)
+ if err != nil || parseCtx == nil {
+ return nil
+ }
+
+ if len(parseCtx.MultiStmt) > 1 {
+ return errATPreparedMultiSQLUnsupported
+ }
+
+ return nil
+}
+
+func (c *ATConn) Prepare(query string) (driver.Stmt, error) {
+ if err := rejectATPreparedMultiSQL(query); err != nil {
+ return nil, err
+ }
+
+ return c.Conn.Prepare(query)
+}
Review Comment:
`rejectATPreparedMultiSQL` applies to all AT connections regardless of DB
type, but the PR description says PostgreSQL behavior is unchanged. If AT is
used with non-MySQL drivers, this new Prepare/PrepareContext rejection can
block previously working prepared multi-SQL (where the MySQL parser can parse
it). Consider gating the rejection to MySQL only (e.g., based on
`c.res.dbType`/`c.dbType`).
##########
pkg/datasource/sql/exec/at/multi_update_excutor.go:
##########
@@ -148,26 +152,40 @@ func (u *multiUpdateExecutor) afterImage(ctx
context.Context, beforeImages []*ty
return nil, errors.New("empty beforeImages")
}
beforeImage := beforeImages[0]
+ if beforeImage == nil {
+ return nil, errors.New("aggregate update before image is nil")
+ }
tableName :=
u.parserCtx.MultiStmt[0].UpdateStmt.TableRefs.TableRefs.Left.(*ast.TableSource).Source.(*ast.TableName).Name.O
metaData, err :=
datasource.GetTableCache(types.DBTypeMySQL).GetTableMeta(ctx,
u.execContext.DBName, tableName)
if err != nil {
return nil, err
}
+ // No row matched the aggregate UPDATE predicates.
+ //
+ // Do not generate an after-image SELECT with an empty primary-key list.
+ // Return one empty image so that before/after image counts remain
equal.
+ if len(beforeImage.Rows) == 0 {
+ return []*types.RecordImage{types.NewEmptyRecordImage(metaData,
u.parserCtx.SQLType)}, nil
+ }
+
// use
selectSQL, selectArgs := u.buildAfterImageSQL(beforeImage, *metaData)
rows, err = u.rowsPrepare(ctx, selectSQL, selectArgs)
+ if err != nil {
+ return nil, err
+ }
defer func() {
Review Comment:
`afterImage` assigns to the package-level `rows` variable (`rows, err =
...`). This introduces shared mutable state across executions and can cause
data races or incorrect row handles when multiple `multiUpdateExecutor`
instances run concurrently. Use a function-local `rows` variable instead.
--
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]
