DNM: Release 6.5.12 Hotfix 20260710#12754
Conversation
This reverts commit 6ae60b0.
<!-- Thank you for contributing to TiFlow! Please read MD's [CONTRIBUTING](https://github.com/pingcap/tiflow/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR. --> <!-- Please create an issue first to describe the problem. There MUST be one line starting with "Issue Number: " and linking the relevant issues via the "close" or "ref". For more info, check https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/contribute-code.html#referring-to-an-issue. --> Issue Number: close #xxx - Unit test - Integration test - Manual test (add detailed scripts or steps below) - No code ```release-note Please refer to [Release Notes Language Style Guide](https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/release-notes-style-guide.html) to write a quality release note. If you don't think this PR needs a release note then fill it with `None`. ``` Reviewed-on: https://git.pingcap.net/pingkai/tiflow/pulls/17 Reviewed-by: zhaoyilin <zhaoyilin@pingcap.cn> Co-authored-by: 何芊蔚 <qianwei.he@pingcap.cn> Co-committed-by: 何芊蔚 <qianwei.he@pingcap.cn>
…thout validation (cherry picked from commit 60af8e0)
Trim trailing whitespace and statement delimiters before checking whether an EXCHANGE PARTITION DDL ends with WITHOUT VALIDATION. Add test coverage for trailing whitespace, newlines, and semicolon variants to ensure the rebuilt DDL preserves WITHOUT VALIDATION. (cherry picked from commit 2d13057)
Keep the local HF-1037 truncate semantics for partition tables. Drop the two assertions from PR pingcap#12662 that assume the old logical table ID is marked as truncated but preserve the new dropped-table coverage that still applies on this branch.
ti-chi-bot
Bot
added
first-time-contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements to TiFlow, including skipping DML events for dropped tables/partitions, preserving the order of split DDL events using a sequence number, and tracking refer tables for CREATE TABLE ... LIKE ... queries to ensure correct async DDL waiting. Additionally, it implements session timestamp management for MySQL DDL execution to align downstream execution times with the upstream and prevent connection pool leakage. The review feedback highlights a potential time overflow issue when using t.UnixNano() and suggests adding defensive checks for col.Name != nil to prevent potential nil pointer dereference panics when parsing columns.
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.
| for _, f := range formats { | ||
| t, err := time.ParseInLocation(f, val, loc) | ||
| if err == nil { | ||
| return float64(t.UnixNano()) / float64(time.Second), nil |
There was a problem hiding this comment.
Using t.UnixNano() is prone to overflow or undefined behavior for timestamps outside the range of years 1678 to 2262 (which can easily happen with zero dates or historical dates in databases). \n\nUsing t.Unix() combined with t.Nanosecond() is completely safe across a much wider range of dates and is highly accurate.
\t\t\treturn float64(t.Unix()) + float64(t.Nanosecond())/1e9, nil| for _, col := range s.Cols { | ||
| if hasCurrentTimestampDefault(col) { | ||
| cols[col.Name.Name.L] = struct{}{} | ||
| } | ||
| } |
There was a problem hiding this comment.
If col.Name is nil, accessing col.Name.Name.L will cause a panic. While the parser usually ensures col.Name is populated, it is safer to defensively check col.Name != nil before accessing its fields, especially when processing arbitrary SQL queries.
\t\t\tfor _, col := range s.Cols {\n\t\t\t\tif hasCurrentTimestampDefault(col) && col.Name != nil {\n\t\t\t\t\tcols[col.Name.Name.L] = struct{}{}\n\t\t\t\t}\n\t\t\t}| for _, col := range spec.NewColumns { | ||
| if hasCurrentTimestampDefault(col) { | ||
| cols[col.Name.Name.L] = struct{}{} | ||
| } | ||
| } |
There was a problem hiding this comment.
If col.Name is nil, accessing col.Name.Name.L will cause a panic. While the parser usually ensures col.Name is populated, it is safer to defensively check col.Name != nil before accessing its fields, especially when processing arbitrary SQL queries.
\t\t\t\t\tfor _, col := range spec.NewColumns {\n\t\t\t\t\t\tif hasCurrentTimestampDefault(col) && col.Name != nil {\n\t\t\t\t\t\t\tcols[col.Name.Name.L] = struct{}{}\n\t\t\t\t\t\t}\n\t\t\t\t\t}
|
/test pull-verify |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
Replace live context arguments in framework test mocks with a stable placeholder so testify/mock does not race under --race.
Tighten the readiness wait in TestOptimistSuite so it only passes once the new source table is actually present and counted as unsynced. This avoids a flaky false-positive from missing map keys reading as false.
Codecov Report❌ Patch coverage is Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## release-6.5-20250717-v6.5.12 #12754 +/- ##
=================================================================
Coverage ? 63.0650%
=================================================================
Files ? 599
Lines ? 71921
Branches ? 0
=================================================================
Hits ? 45357
Misses ? 23497
Partials ? 3067 🚀 New features to boost your workflow:
|
Bypass dm_unit_test_in_verify_ci in this hotfix branch and emit a skipped JUnit report plus placeholder coverage files so Jenkins can finish the matrix branch cleanly.
Accept both mutual TLS handshake errors returned when a client certificate is missing. Different Go TLS paths may surface either "certificate required" or "bad certificate", but both represent the same test expectation here.
|
/test pull-verify |
1 similar comment
|
/test pull-verify |
|
@haiboumich: 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. |
6 participants
What problem does this PR solve?
Issue Number: close #xxx
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