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

util: fix issue #9532, quote database name before running SQL with it #9547

Merged
merged 2 commits into from Mar 5, 2019

Conversation

Projects
None yet
4 participants
@kennytm
Copy link
Member

kennytm commented Mar 4, 2019

What problem does this PR solve?

Fixed #9532.

What is changed and how it works?

Add quotation marks around the database name, to ensure it is already interpreted as an identifier even if it contains exotic characters or keywords.

Check List

Tests

  • Unit test

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch
    • Needs to backport to 2.1, so that Lightning 2.1.x can pick up this bug fix.

@kennytm kennytm added the type/bug-fix label Mar 4, 2019

@kennytm

This comment has been minimized.

Copy link
Member Author

kennytm commented Mar 4, 2019

/run-all-tests

@zz-jason
Copy link
Member

zz-jason left a comment

LGTM

need one more reviewer

@zz-jason zz-jason added the status/LGT1 label Mar 4, 2019

@kennytm

This comment has been minimized.

Copy link
Member Author

kennytm commented Mar 4, 2019

(The test failure in https://internal.pingcap.net/idc-jenkins/blue/organizations/jenkins/tidb_ghpr_unit_test/detail/tidb_ghpr_unit_test/1764/pipeline/

FAIL: show_test.go:205: testSuite2.TestShow3

show_test.go:209:
    tk.MustQuery("show create user 'test_show_create_user'@'%'").
        Check(testkit.Rows(`CREATE USER 'test_show_create_user'@'%' IDENTIFIED WITH 'mysql_native_password' AS '*81F5E21E35407D884A6CD4A731AEBFB6AF209E1B' REQUIRE NONE PASSWORD EXPIRE DEFAULT ACCOUNT UNLOCK`))
/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:212:
    tk.c.Assert(errors.ErrorStack(err), check.Equals, "", comment)
... obtained string = "" +
...     "[executor:1396]Operation SHOW CREATE USER failed for 'test_show_create_user'@'%'\n" +
...     "github.com/pingcap/errors.AddStack\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/errors@v0.11.0/errors.go:174\n" +
...     "github.com/pingcap/parser/terror.(*Error).GenWithStackByArgs\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/parser@v0.0.0-20190228070002-74e8cffabf28/terror/terror.go:233\n" +
...     "github.com/pingcap/tidb/executor.(*ShowExec).fetchShowCreateUser\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/show.go:911\n" +
...     "github.com/pingcap/tidb/executor.(*ShowExec).fetchAll\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/show.go:117\n" +
...     "github.com/pingcap/tidb/executor.(*ShowExec).Next\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/show.go:80\n" +
...     "github.com/pingcap/tidb/executor.(*recordSet).Next\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/adapter.go:104\n" +
...     "github.com/pingcap/tidb/session.GetRows4Test\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/session/tidb.go:264\n" +
...     "github.com/pingcap/tidb/util/testkit.(*TestKit).ResultSetToResult\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:211\n" +
...     "github.com/pingcap/tidb/util/testkit.(*TestKit).MustQuery\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:185\n" +
...     "github.com/pingcap/tidb/executor_test.(*testSuite2).TestShow3\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/src/github.com/pingcap/tidb/executor/show_test.go:209\n" +
...     "runtime.call32\n" +
...     "\t/usr/local/go/src/runtime/asm_amd64.s:522\n" +
...     "reflect.Value.call\n" +
...     "\t/usr/local/go/src/reflect/value.go:447\n" +
...     "reflect.Value.Call\n" +
...     "\t/usr/local/go/src/reflect/value.go:308\n" +
...     "github.com/pingcap/check.(*suiteRunner).forkTest.func1\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:836\n" +
...     "github.com/pingcap/check.(*suiteRunner).forkCall.func1\n" +
...     "\t/home/jenkins/workspace/tidb_ghpr_unit_test/go/pkg/mod/github.com/pingcap/check@v0.0.0-20190102082844-67f458068fc8/check.go:730\n" +
...     "runtime.goexit\n" +
...     "\t/usr/local/go/src/runtime/asm_amd64.s:1333"
... expected string = ""
... sql:show create user 'test_show_create_user'@'%', args:[]

and https://internal.pingcap.net/idc-jenkins/blue/organizations/jenkins/tidb_ghpr_integration_common_test/detail/tidb_ghpr_integration_common_test/1576/pipeline

FAIL: region_request_test.go:276: testRegionRequestSuite.TestNoReloadRegionForGrpcWhenCtxCanceled

region_request_test.go:280:
    c.Assert(err, IsNil)
... value *net.OpError = &net.OpError{Op:"listen", Net:"tcp", Source:net.Addr(nil), Addr:(*net.TCPAddr)(0xc03dee4060), Err:(*os.SyscallError)(0xc03dbb2120)} ("listen tcp 127.0.0.1:56341: bind: address already in use")

do not seem to be relevant.)

@kennytm

This comment has been minimized.

Copy link
Member Author

kennytm commented Mar 4, 2019

/run-integration-common-test

@eurekaka
Copy link
Contributor

eurekaka left a comment

LGTM

@kennytm kennytm added status/LGT2 and removed status/LGT1 labels Mar 5, 2019

@eurekaka

This comment has been minimized.

Copy link
Contributor

eurekaka commented Mar 5, 2019

/run-unit-test

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 5, 2019

Codecov Report

Merging #9547 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9547      +/-   ##
==========================================
+ Coverage   67.37%   67.38%   +0.01%     
==========================================
  Files         375      375              
  Lines       78778    78779       +1     
==========================================
+ Hits        53076    53088      +12     
+ Misses      20931    20923       -8     
+ Partials     4771     4768       -3
Impacted Files Coverage Δ
util/kvencoder/kv_encoder.go 70.17% <100%> (+0.26%) ⬆️
store/tikv/lock_resolver.go 41.7% <0%> (-0.95%) ⬇️
executor/distsql.go 72.18% <0%> (+0.45%) ⬆️
ddl/delete_range.go 79.36% <0%> (+4.23%) ⬆️
util/systimemon/systime_mon.go 100% <0%> (+20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d094a0f...df32ae7. Read the comment docs.

@eurekaka eurekaka merged commit 63b49c9 into pingcap:master Mar 5, 2019

5 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@kennytm kennytm deleted the kennytm:fix-9532 branch Mar 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.