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

Refactor parser: Avoid some assertion for StmtNode #4705

Merged
merged 4 commits into from Oct 9, 2017

Conversation

Projects
None yet
3 participants
@shenli
Member

shenli commented Oct 1, 2017

Make code cleaner. No performance improvement.

Before:

➜ parser git:(master) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4589 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14310 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 32.910s
➜ parser git:(master) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4770 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14001 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 33.489s
➜ parser git:(master) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4679 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 ^[[A 1000000 14028 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 32.791s
➜ parser git:(master) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4628 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14434 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 33.395s

After:

➜ parser git:(shenli/parser) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4586 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14229 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 32.819s
➜ parser git:(shenli/parser) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4567 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14253 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 32.768s
➜ parser git:(shenli/parser) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4622 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14979 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 33.719s
➜ parser git:(shenli/parser) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4701 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14687 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 33.645s
➜ parser git:(shenli/parser) ✗ go test -run=XXX -bench=. -benchtime=10s
BenchmarkSysbenchSelect-4 3000000 4561 ns/op 1888 B/op 27 allocs/op
BenchmarkParse-4 1000000 14233 ns/op 5224 B/op 89 allocs/op
PASS
ok github.com/pingcap/tidb/parser 32.730s

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Oct 4, 2017

Member

/run-all-tests

Member

shenli commented Oct 4, 2017

/run-all-tests

@shenli shenli referenced this pull request Oct 4, 2017

Open

Avoid type assertion in parser.y #4709

0 of 2 tasks complete
@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Oct 9, 2017

Member

LGTM

Member

zz-jason commented Oct 9, 2017

LGTM

@zz-jason zz-jason added the status/LGT1 label Oct 9, 2017

Show outdated Hide outdated parser/parser.y
| SubSelect
{
// `(select 1)`; is a valid select statement
// TODO: This is used to fix issue #320. There may be a better solution.
$$ = $1.(*ast.SubqueryExpr).Query
$$ = $1.(*ast.SubqueryExpr).Query.(ast.StmtNode)

This comment has been minimized.

@XuHuaiyu

XuHuaiyu Oct 9, 2017

Contributor

why we add type assertion here?

@XuHuaiyu

XuHuaiyu Oct 9, 2017

Contributor

why we add type assertion here?

This comment has been minimized.

@shenli

shenli Oct 9, 2017

Member

$1.(*ast.SubqueryExpr).Query is a ResultSet, not StmtNode.

@shenli

shenli Oct 9, 2017

Member

$1.(*ast.SubqueryExpr).Query is a ResultSet, not StmtNode.

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli
Member

shenli commented Oct 9, 2017

@XuHuaiyu PTAL

@XuHuaiyu

This comment has been minimized.

Show comment
Hide comment
@XuHuaiyu

XuHuaiyu Oct 9, 2017

Contributor

circleci failed @shenli

Contributor

XuHuaiyu commented Oct 9, 2017

circleci failed @shenli

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Oct 9, 2017

Member

@XuHuaiyu I can not open CircleCI now. I merge master and rerun it.

Member

shenli commented Oct 9, 2017

@XuHuaiyu I can not open CircleCI now. I merge master and rerun it.

@XuHuaiyu

LGTM

@XuHuaiyu XuHuaiyu merged commit 0d0685a into master Oct 9, 2017

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 72.618%
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@XuHuaiyu XuHuaiyu deleted the shenli/parser branch Oct 9, 2017

mccxj added a commit to mccxj/tidb that referenced this pull request Oct 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment