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.

Sign up for GitHub

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

Jump to bottom

planner,types: refactor the PR #8619 by using parser/types/FieldType.Clone() #8641

Merged
merged 3 commits into from
Dec 12, 2018
Merged

planner,types: refactor the PR #8619 by using parser/types/FieldType.Clone() #8641

merged 3 commits into from
Dec 12, 2018

Conversation

dbjoa
Copy link
Contributor

@dbjoa dbjoa commented Dec 11, 2018 •
edited by c4pt0r
Loading

What problem does this PR solve?

Refactor the merged PR #8619

What is changed and how it works?

  • Replace types.CloneFieldType()" by introducing the member function FieldType.Clone()inparser/types``

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change
  • Has interface methods change

Side effects

  • None

Related changes

  • parser/types/field_type.go

This change is Reviewable

@dbjoa
Copy link
Contributor Author

dbjoa commented Dec 11, 2018

/run-all-tests

@zz-jason zz-jason added contribution This PR is from a community contributor. sig/planner SIG: Planner labels Dec 11, 2018
zz-jason
zz-jason reviewed Dec 11, 2018
View reviewed changes
go.mod Outdated
@@ -85,3 +85,5 @@ require (
sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4
sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67
)

replace github.com/pingcap/parser => github.com/cloud-pi/parser v0.0.0-20181211095502-1e4340922693
Copy link
Member

Choose a reason for hiding this comment

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

irrelevant change?

@dbjoa dbjoa mentioned this pull request Dec 11, 2018
Merged
@zhouqiang-cl
Copy link
Contributor

/rebuild

@zhouqiang-cl
Copy link
Contributor

/run-all-tests

@dbjoa
Copy link
Contributor Author

dbjoa commented Dec 11, 2018

/run-common-test

@dbjoa
Copy link
Contributor Author

dbjoa commented Dec 11, 2018 •
edited
Loading

@zhouqiang-cl Could you please rebuild the PR?

winoros
winoros reviewed Dec 11, 2018
View reviewed changes
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

LGTM
please waiting for pr in parser merged.

eurekaka
eurekaka previously approved these changes Dec 11, 2018
View reviewed changes
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM

@eurekaka eurekaka added the status/LGT2 Indicates that a PR has LGTM 2. label Dec 11, 2018
@winoros winoros dismissed a stale review December 11, 2018 07:07

CI controlled it.

@ngaut
Copy link
Member

ngaut commented Dec 11, 2018

/run-all-tests

@ngaut
Copy link
Member

ngaut commented Dec 11, 2018

Please fix CI

@zhouqiang-cl
Copy link
Contributor

/rebuild

@zhouqiang-cl
Copy link
Contributor

zhouqiang-cl commented Dec 11, 2018 •
edited
Loading

It seems build failed
@dbjoa

GOPATH=/home/jenkins/workspace/tidb_ghpr_build/go CGO_ENABLED=1 GO111MODULE=on go build -race -ldflags '-X "github.com/pingcap/parser/mysql.TiDBReleaseVersion=v2.1.0-rc.3-308-g9a9165a" -X "github.com/pingcap/tidb/util/printer.TiDBBuildTS=2018-12-11 08:51:21" -X "github.com/pingcap/tidb/util/printer.TiDBGitHash=9a9165a85db56390e5db257e82cbb45b28d78f41" -X "github.com/pingcap/tidb/util/printer.TiDBGitBranch=HEAD" -X "github.com/pingcap/tidb/util/printer.GoVersion=go version go1.11.2 linux/amd64" ' -o bin/tidb-server tidb-server/main.go

go: finding github.com/cloud-pi/parser v0.0.0-20181211095502-1e4340922693

go: downloading github.com/cloud-pi/parser v0.0.0-20181211095502-1e4340922693

# github.com/pingcap/tidb/ddl

ddl/ddl_api.go:1157:8: undefined: ast.AlterTableTruncatePartition

ddl/ddl_api.go:1499:15: undefined: model.ActionTruncateTablePartition

ddl/ddl_worker.go:288:137: undefined: model.ActionTruncateTablePartition

ddl/ddl_worker.go:475:7: undefined: model.ActionTruncateTablePartition

ddl/delete_range.go:246:39: undefined: model.ActionTruncateTablePartition

make: *** [server] Error 2

@zhouqiang-cl
Copy link
Contributor

/rebuild

@zhouqiang-cl
Copy link
Contributor

/run-all-tests

@crazycs520
Copy link
Contributor

/run-common-test -tidb-test=pr/692

@zhouqiang-cl
Copy link
Contributor

/run-all-tests

@crazycs520
Copy link
Contributor

/run-common-test -tidb-test=pr/693

3 similar comments
@crazycs520
Copy link
Contributor

/run-common-test -tidb-test=pr/693

@crazycs520
Copy link
Contributor

/run-common-test -tidb-test=pr/693

@crazycs520
Copy link
Contributor

/run-common-test -tidb-test=pr/693

@crazycs520
Copy link
Contributor

/run-integration-common-test -tidb-test=pr/693

@pingcap pingcap deleted a comment from zhouqiang-cl Dec 12, 2018
@pingcap pingcap deleted a comment from zhouqiang-cl Dec 12, 2018
@pingcap pingcap deleted a comment from zhouqiang-cl Dec 12, 2018
@pingcap pingcap deleted a comment from zhouqiang-cl Dec 12, 2018
@pingcap pingcap deleted a comment from dbjoa Dec 12, 2018
@zhouqiang-cl
Copy link
Contributor

/run-integration-ddl-test -tidb-test=pr/695

zz-jason
zz-jason reviewed Dec 12, 2018
View reviewed changes
Copy link
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

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

LGTM

@zz-jason zz-jason merged commit 64cd9b2 into pingcap:master Dec 12, 2018
iamzhoug37 pushed a commit to iamzhoug37/tidb that referenced this pull request Dec 13, 2018
@dbjoa @iamzhoug37
planner,types: refactor the PR pingcap#8619 by using parser/types/Fie…
f23e306 
…ldType.Clone() (pingcap#8641)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@winoros winoros winoros left review comments

@zz-jason zz-jason zz-jason approved these changes

@eurekaka eurekaka eurekaka left review comments

Assignees
No one assigned
Labels
contribution This PR is from a community contributor. sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@dbjoa @zhouqiang-cl @ngaut @crazycs520 @zz-jason @eurekaka @winoros