You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@seata.apache.org by "Vanillaxi (via GitHub)" <gi...@apache.org> on 2026/09/19 09:07:46 UTC
[PR] fix: correct legacy undo log builder behavior [incubator-seata-go]
Vanillaxi opened a new pull request, #1188:
URL: https://github.com/apache/incubator-seata-go/pull/1188
<!-- Thanks for sending a pull request!
-->
<!-- Please make sure you have read and understood the contributing guidelines -->
- [ ] I have registered the PR [changes](https://github.com/apache/incubator-seata-go/tree/master/changes).
**What this PR does**:
Fix basic correctness issues in the legacy builders under `pkg/datasource/sql/undo/builder`:
- Fix slice index out-of-range panics when converting `NamedValues` in DELETE, multi-update, and multi-delete builders.
- Build `lockKey` for DELETE and multi-delete from the primary keys in the already constructed `RecordImage`.
- Add the missing `WHERE` between the table name and primary key conditions in the multi-update after-image SQL.
**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #1187
**Special notes for your reviewer**:
**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
```
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix: correct legacy undo log builder behavior [incubator-seata-go]
Posted by "Ethan-Xingyue (via GitHub)" <gi...@apache.org>.
Ethan-Xingyue commented on PR #1188:
URL: https://github.com/apache/incubator-seata-go/pull/1188#issuecomment-5748201630
Verified locally: the new tests fail on `master` and pass with this patch; build, vet, gofmt and `go test -race` are clean. The three fixes look correct to me.
Before merging:
1. `BasicUndoLogBuilder.buildLockKey(rows driver.Rows, ...)` has no callers after this change (staticcheck `U1000`). Please remove it, since it was the root cause of the empty lock key. Optionally rename `buildLockKey2` back to `buildLockKey`.
2. `TestMySQLDeleteUndoLogBuilder_BeforeImage` still asserts the index-out-of-range panic as expected behavior and only passes now because `Conn` is nil. Please update or drop it.
3. The new test file keeps repro leftovers: the "Avoid the independent NamedValues bug" comment (fixed in this PR), the `t.Logf` calls, and the parser "control" check. Consider folding the cases into the existing `<source>_test.go` files with `testify/assert`, like the rest of the package.
4. Nit: for the empty before-image case, return `types.NewEmptyRecordImage(&metaData, execCtx.ParseContext.SQLType)` to match `exec/at/multi_update_excutor.go`.
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix: correct legacy undo log builder behavior [incubator-seata-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1188:
URL: https://github.com/apache/incubator-seata-go/pull/1188#issuecomment-5740694119
## [Codecov](https://app.codecov.io/gh/apache/incubator-seata-go/pull/1188?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 61.47%. Comparing base ([`5980741`](https://app.codecov.io/gh/apache/incubator-seata-go/commit/598074146bc4142dd33f29d5d2a2049c90ba0f6f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`797a8e7`](https://app.codecov.io/gh/apache/incubator-seata-go/commit/797a8e76cdca2454ed56953c574c3896a3e44c56?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #1188 +/- ##
==========================================
+ Coverage 61.11% 61.47% +0.36%
==========================================
Files 309 309
Lines 22626 22626
==========================================
+ Hits 13827 13909 +82
+ Misses 7518 7421 -97
- Partials 1281 1296 +15
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/incubator-seata-go/pull/1188?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix: correct legacy undo log builder behavior [incubator-seata-go]
Posted by "Vanillaxi (via GitHub)" <gi...@apache.org>.
Vanillaxi commented on PR #1188:
URL: https://github.com/apache/incubator-seata-go/pull/1188#issuecomment-5749296934
> Verified locally: the new tests fail on `master` and pass with this patch; build, vet, gofmt and `go test -race` are clean. The three fixes look correct to me.
>
> Before merging:
>
> 1. `BasicUndoLogBuilder.buildLockKey(rows driver.Rows, ...)` has no callers after this change (staticcheck `U1000`). Please remove it, since it was the root cause of the empty lock key. Optionally rename `buildLockKey2` back to `buildLockKey`.
> 2. `TestMySQLDeleteUndoLogBuilder_BeforeImage` still asserts the index-out-of-range panic as expected behavior and only passes now because `Conn` is nil. Please update or drop it.
> 3. The new test file keeps repro leftovers: the "Avoid the independent NamedValues bug" comment (fixed in this PR), the `t.Logf` calls, and the parser "control" check. Consider folding the cases into the existing `<source>_test.go` files with `testify/assert`, like the rest of the package.
> 4. Nit: for the empty before-image case, return `types.NewEmptyRecordImage(&metaData, execCtx.ParseContext.SQLType)` to match `exec/at/multi_update_excutor.go`.
Thanks ,done.
同时还额外修复了multi-delete 原来直接按分号切字符串的行为，遇到以下情况会把合法SQL拆错：
```
DELETE FROM cart_item WHERE cart_id = 101;
DELETE FROM cart_item WHERE cart_id = 102;
```
旧实现 `strings.Split(query, ";")` 会切成三段：两条DELETE和一个空字符串
现在改为直接使用前面已解析好的 `ParseContext.MultiStmt` ，不再做分号切割
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org
Re: [PR] fix: correct legacy undo log builder behavior [incubator-seata-go]
Posted by "thunguo (via GitHub)" <gi...@apache.org>.
thunguo merged PR #1188:
URL: https://github.com/apache/incubator-seata-go/pull/1188
--
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: notifications-unsubscribe@seata.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@seata.apache.org
For additional commands, e-mail: notifications-help@seata.apache.org