Skip to content
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

sink: support maxwell data format #869

Merged
merged 23 commits into from
Sep 3, 2020
Merged

Conversation

shldreams
Copy link
Contributor

What problem does this PR solve?

What is changed and how it works?

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Code changes

  • Compatible with maxwell output

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation
  • Need to update the tidb-cdc

Release note

@CLAassistant
Copy link

CLAassistant commented Aug 21, 2020

CLA assistant check
All committers have signed the CLA.

@zier-one zier-one changed the title Feture maxwell sink: support maxwell data format Aug 27, 2020
@zier-one
Copy link
Contributor

zier-one commented Sep 1, 2020

/run-all-tests

@zier-one zier-one added status/ptal Could you please take a look? component/sink Sink component. labels Sep 1, 2020
io "io"
math "math"
math_bits "math/bits"

proto "github.com/golang/protobuf/proto"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

recover this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry ,I don't know what's you mean,this pr has nothing to do with proto package.

Copy link
Contributor

@zier-one zier-one Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, undo the changes to this file.

Ts: e.CommitTs,
Database: e.Table.Schema,
Table: e.Table.Table,
Type: "update",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Type: "update",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return "table-alter"
}
switch ddlType {
case 3:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to use constant variable from parser lib instead of the const number, such as model2.ActionCreateTable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

func columnToMaxwellType(columnType byte) (string, error) {
switch columnType {
// tinyint,smallint,mediumint,int
case 1, 2, 3, 9:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, is it better to use mysql.TypeTiny

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@zier-one
Copy link
Contributor

zier-one commented Sep 2, 2020

/run-all-tests

Copy link
Contributor

@zier-one zier-one left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 3, 2020
Copy link
Contributor

@amyangfei amyangfei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 3, 2020
@amyangfei
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 3, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@codecov-commenter
Copy link

Codecov Report

Merging #869 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master       #869   +/-   ##
===========================================
  Coverage   33.2192%   33.2192%           
===========================================
  Files            99         99           
  Lines         11683      11683           
===========================================
  Hits           3881       3881           
  Misses         7416       7416           
  Partials        386        386           

@ti-srebot ti-srebot merged commit 56fdcfc into pingcap:master Sep 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/sink Sink component. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. status/ptal Could you please take a look?
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants