Repository navigation
*: update tidb and parser - #12674
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates import paths in dm/checker/checker.go and dm/loader/lightning.go to reference relocated packages under github.com/pingcap/tidb/lightning/pkg and github.com/pingcap/tidb/pkg/lightning/importdef. Additionally, it upgrades the Go version to 1.25.10 and updates various third-party dependencies in go.mod and go.sum. All review comments were filtered out as they commented on version numbers, so there is no additional feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/retest |
| @@ -1,10 +1,10 @@ | |||
| module github.com/pingcap/tiflow | |||
|
|
|||
| go 1.25.8 | |||
OliverS929
left a comment
There was a problem hiding this comment.
pingcap/tidb#67778 changes schemacmp so charset/collation comparison is no longer strict singleton matching: utf8/utf8mb3 -> utf8mb4 and latin1 -> utf8mb4 are treated as compatible upgrades, while unrelated charset/collation families remain incompatible.
My current understanding is that this is explainable for DM optimistic shard DDL, since DM also relies on schemacmp semantics here. The direction looks reasonable to me, but could @D3Hunter and @GMHDBJD help confirm whether this matches the intended DM behavior?
A follow-up could also consider covering the classic DM optimistic shard DDL path with a small charset/collation case, so this behavior is pinned on the DM side as well.
|
Since this PR also changes engine-related files, I think @D3Hunter's approval is needed before merging. |
@alastori PTAL. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3AceShowHand, GMHDBJD The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
@OliverS929 Thanks for the context. Non-blocking PM note from the customer-impact angle. The direction makes sense for rolling charset upgrades in sharded DM tasks. A common customer scenario is one shard moving from utf8/utf8mb3 to utf8mb4 before the others; treating that as a compatible widening avoids a false optimistic DDL conflict. The main caveat I want to make explicit is latin1. I think we should frame latin1 -> utf8mb4 as schema compatibility, not as a data conversion guarantee. For example, a latin1 value like Café may be stored as bytes 43 61 66 E9, while the same value in utf8mb4 is 43 61 66 C3 A9. If DM treats the schema change as compatible, can we confirm how full-load and incremental paths handle existing latin1 bytes after the downstream schema is widened? I want to make sure we are not accidentally changing equality, uniqueness, sorting, or query behavior for users with non-ASCII latin1 data. For test coverage, I would prioritize the customer-facing boundaries that should remain incompatible:
A lower-priority follow-up is table-default-only changes, for example ALTER TABLE ... DEFAULT CHARACTER SET utf8mb4, because changing the table default does not necessarily convert existing explicit columns. |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
@wk989898: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: close #12672
What is changed and how it works?
Check List
Tests
Questions
Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?
Release note