github-actions[bot] commented on PR #1140: URL: https://github.com/apache/incubator-seata-go/pull/1140#issuecomment-5291144503
> Bot detected the issue body's language is not English, translate it automatically. Thank you for reviewing, the following points are the design intentions pointed out by the corresponding review 1. In fact, the aggregation path returns the last result to the user, because the underlying driver uses` go-sql-driver/mysql v1.6.0 `, which must be upgraded to 1.8.0 to get a sum RowsAffected in the case of an aggregation.If you want to upgrade, feel that you can do it in the follow-up pr.So here are the last results returned by both paths, I will add a comment 2. I haven't thought of a better plan to let the sql registered with `update/delete hook` go the aggregation path, I first let him go the sequential execution and then supplement the log 3. This` HooksForSQLType `is not only used to judge the length, the sequential sub-executor will also call `` `go childHooks: = hooksForSQLType (parseCtx.SQLType)//multi_sequential_executor.go: 148 ```In addition, in the aggregation judgment, only those with hooks will be copied, and only the first statement with hooks will be copied, and only one slice copy will be wasted at most. 4. `rejectATPreparedMultiSQL' fails open Here is to reject only multi-SQL that can be confirmed, and the execution phase will still be parsed normally.(I'll add a comment) 5. `execSequential` re-parses each statement not only to check if the executor type has changed, but mainly to reset the index of the sub-statement parameter placeholder to prevent out-of-bounds/incorrect parameters: When multi-sentence global parsing, placeholder ordinals are globally contiguous, for example: ``` Update t_user set name =? Where id =?; Delete from t_user where user_id =?; ``` The sequence number of this parameter in the overall AST is: `Update: 0,1` `delete: 2` But when executed sequentially, the code is to split the parameter into its own slice of the sub-statement and reset the `driver.NamedValue.Ordinal`: ```Update args: [name, id] Delete args: [user_id] ``` This` TestReparseStatementResetsParameterOrders (214) `test in` multi_sequential_executor_test.go `has been validated. I'll add a comment next. 6. I intend to implement this in the follow-up pr 7. Point 8 is unintentional, I will adjust -- 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]
