-
Notifications
You must be signed in to change notification settings - Fork 288
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
mounter(ticdc): fix mounter default value panic and data inconsistency #3930
mounter(ticdc): fix mounter default value panic and data inconsistency #3930
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. @@ Coverage Diff @@
## master #3930 +/- ##
================================================
- Coverage 57.0741% 55.1263% -1.9479%
================================================
Files 478 486 +8
Lines 56551 59848 +3297
================================================
+ Hits 32276 32992 +716
- Misses 20978 23506 +2528
- Partials 3297 3350 +53 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a unit test?
Sure, I will do it later. So I hold the pr. |
/run-dm-compatibility-test |
/run-verify |
/run-kafka-integration-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
/run-kafka-integration-test /tidb=pr/30773 |
/run-dm-compatibility-test |
if d.IsNull() { | ||
log.Error("meet unsupported column type", zap.String("column info", col.String())) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen in this scenario?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sink null to the downstream and cause data inconsistency for unsupported data type
/run-kafka-integration-test /tidb=pr/30773 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: ti-chi-bot <[email protected]>
In response to a cherrypick label: new pull request created: #4442. |
Signed-off-by: ti-chi-bot <[email protected]>
In response to a cherrypick label: new pull request created: #4443. |
Signed-off-by: ti-chi-bot <[email protected]>
In response to a cherrypick label: new pull request created: #4444. |
Signed-off-by: ti-chi-bot <[email protected]>
In response to a cherrypick label: new pull request created: #4445. |
Signed-off-by: ti-chi-bot <[email protected]>
In response to a cherrypick label: new pull request could not be created: failed to create pull request against pingcap/tiflow#release-5.3 from head ti-chi-bot:cherry-pick-3930-to-release-5.3: the GitHub API request returns a 403 error: {"message":"You have exceeded a secondary rate limit and have been temporarily blocked from content creation. Please retry your request again later.","documentation_url":"https://docs.github.com/rest/overview/resources-in-the-rest-api#secondary-rate-limits"} |
close #3929 #3918
What problem does this PR solve?
When meet "amend + add column datetime default '2020-10-10 10:10:10' (DDL)", TiCDC will
What is changed and how it works?
Check List
Tests
Start 2 transactions:
Code changes
Side effects
Related changes
Release note