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

parser: implement drop view parse #67

Merged
merged 2 commits into from
Dec 7, 2018
Merged

parser: implement drop view parse #67

merged 2 commits into from
Dec 7, 2018

Conversation

AndrewDi
Copy link
Contributor

@AndrewDi AndrewDi commented Dec 5, 2018

Alter parser to support drop view function, and reuse DropTableStmt struct.

@AndrewDi
Copy link
Contributor Author

AndrewDi commented Dec 5, 2018

@zz-jason @XuHuaiyu PTAL

@AndrewDi AndrewDi mentioned this pull request Dec 6, 2018
Closed
19 tasks
XuHuaiyu
XuHuaiyu reviewed Dec 7, 2018
View reviewed changes
Copy link
Contributor

@XuHuaiyu XuHuaiyu left a comment

Choose a reason for hiding this comment

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

LGTM

@XuHuaiyu XuHuaiyu changed the title parser:implement drop view parse parser: implement drop view parse Dec 7, 2018
zz-jason
zz-jason approved these changes Dec 7, 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 added the status/LGT2 LGT2 label Dec 7, 2018
@XuHuaiyu XuHuaiyu merged commit 639797c into pingcap:master Dec 7, 2018
@tiancaiamao
Copy link
Collaborator

This PR does not run this step https://github.com/pingcap/parser/blob/master/README.md#step-2-make-your-parser-changes-take-effect-in-tidb-and-run-ci, and update parser will make TiDB broke now.

Would you like to take a look? @XuHuaiyu @zz-jason

pingcap/tidb#8624 is blocked by it.

tiancaiamao added a commit to tiancaiamao/parser that referenced this pull request Dec 8, 2018
@tiancaiamao
reset pingcap#67 to fix CI
006feb4
@AndrewDi
Copy link
Contributor Author

AndrewDi commented Dec 9, 2018

@tiancaiamao It's unreasonable that DROP grammar effect with JOIN grammar.

@AndrewDi
Copy link
Contributor Author

AndrewDi commented Dec 9, 2018

@tiancaiamao I update go mod on top of TiDB master repo pingcap/tidb#8571, and explain test passed, you may have a look at it.

CGO_ENABLED=0 GO111MODULE=on go build   -ldflags '-X "github.com/pingcap/parser/mysql.TiDBReleaseVersion=v2.1.0-rc.3-298-g835a9a5ba" -X "github.com/pingcap/tidb/util/printer.TiDBBuildTS=2018-12-09 01:59:56" -X "github.com/pingcap/tidb/util/printer.TiDBGitHash=835a9a5ba22028bd954d6f314066d7f0de36796a" -X "github.com/pingcap/tidb/util/printer.TiDBGitBranch=pull/8571" -X "github.com/pingcap/tidb/util/printer.GoVersion=go version go1.11.2 linux/amd64" ' -o bin/tidb-server tidb-server/main.go
skip building tidb-server, using existing binary: ../../bin/tidb-server
skip building importer, using existing binary: 
building explain-test binary: ./explain_test
start tidb-server, log file: ./explain-test.out
tidb-server(PID: 4791) started
run all explain test cases

Great, All tests passed
explaintest end

@tiancaiamao
Copy link
Collaborator

@AndrewDi In pingcap/tidb#8571
A test case is removed:
https://github.com/pingcap/tidb/pull/8571/files#diff-9bd3d43d440bb72c7bbc5c509505a553L995

If you run the master branch, this test case will fail like that:

----------------------------------------------------------------------
PANIC: logical_plan_test.go:880: testPlanSuite.TestColumnPruning

... Panic: interface conversion: *core.DDL is not core.LogicalPlan: missing method Children (PC=0x45B7FA)

/media/genius/OS/project/go/src/runtime/panic.go:513
  in gopanic
/media/genius/OS/project/go/src/runtime/iface.go:85
  in getitab
/media/genius/OS/project/go/src/runtime/iface.go:522
  in assertI2I
logical_plan_test.go:1010
  in testPlanSuite.TestColumnPruning
/media/genius/OS/project/go/src/reflect/value.go:308
  in Value.Call
/media/genius/OS/project/pkg/mod/github.com/pingcap/check@v0.0.0-20171206051426-1c287c953996/check.go:798
  in suiteRunner.forkTest.func1
/media/genius/OS/project/pkg/mod/github.com/pingcap/check@v0.0.0-20171206051426-1c287c953996/check.go:692
  in suiteRunner.forkCall.func1
/media/genius/OS/project/go/src/runtime/asm_amd64.s:1333
  in goexit
OOPS: 0 passed, 1 PANICKED
--- FAIL: TestT (0.00s)
FAIL
exit status 1
FAIL	github.com/pingcap/tidb/planner/core	0.010s

@winoros winoros mentioned this pull request Dec 10, 2018
Merged
@winoros
Copy link
Member

winoros commented Dec 10, 2018

Please label the pr with DNM next time, if it has corresponding planner change in TiDB.

@XuHuaiyu
Copy link
Contributor

Sorry for that, I did not notice it.

@AndrewDi AndrewDi deleted the implement_drop_view_parse branch January 16, 2019 12:45
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
@AndrewDi @XuHuaiyu
parser: implement drop view parse (pingcap#67)
30aca86
lyonzhi pushed a commit to lyonzhi/parser that referenced this pull request Apr 25, 2024
@AndrewDi @XuHuaiyu
parser: implement drop view parse (pingcap#67)
1d20fc5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XuHuaiyu XuHuaiyu XuHuaiyu left review comments

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

Assignees
No one assigned
Labels
contribution status/LGT2 LGT2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AndrewDi @tiancaiamao @winoros @XuHuaiyu @zz-jason @you06