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

DDL: create table should not allow explicit empty name index #20617

Merged
merged 25 commits into from Nov 3, 2020

Conversation

watchpoints
Copy link
Contributor

What problem does this PR solve?

Issue Number: close #18149
create table not support create empty name index

after parser PR: pingcap/parser/pull/1059
then update go.mod.

Problem Summary:

What is changed and how it works?

change token IndexName from ident to item type
use a NullIndexName struct to represent a empty index to distinguish from a anonymous index

Related changes

  • pingcap/parser

Check List

Tests

  • Unit test
    go test -check.f TestValidator
    go test -check.f TestCreateTable

  • Manual test (add detailed scripts or steps below)
    In TiDB

mysql> CREATE TABLE t(x INT, KEY ``(x));
ERROR 1280 (42000): Incorrect index name '

In MySQL:

mysql> CREATE TABLE t1(x INT, KEY ``(x));
ERROR 1280 (42000): Incorrect index name ''

It has the same action with MySQL

Side effects

Release note

  • No release note.

@watchpoints watchpoints requested a review from a team as a code owner October 24, 2020 05:01
@watchpoints watchpoints requested review from XuHuaiyu and removed request for a team October 24, 2020 05:01
@ti-srebot ti-srebot added the first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. label Oct 24, 2020
@watchpoints
Copy link
Contributor Author

@ti-srebot /run-all-tests

@ti-srebot
Copy link
Contributor

/run-all-tests

@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Oct 24, 2020
@watchpoints watchpoints reopened this Oct 24, 2020
@watchpoints watchpoints reopened this Oct 24, 2020
@watchpoints watchpoints reopened this Oct 24, 2020
Copy link
Contributor

@xhebox xhebox left a comment

Choose a reason for hiding this comment

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

ddl_api.go changed its file permission from 644 to 755.

@bb7133 bb7133 changed the title DDL: createtable not support empty name index DDL: create table should not allow explicit empty name index Oct 25, 2020
@XuHuaiyu XuHuaiyu removed their request for review October 28, 2020 02:28
@watchpoints
Copy link
Contributor Author

watchpoints commented Oct 28, 2020

@tangenta , when runing make test int tidb with my parser,I get err:
tidb-server(PID: 129817) started

run all explain test cases
[2020/10/27 14:44:27.050 +08:00] [FATAL] [main.go:694] ["run test"] [test=select] [error="sql:sc;: run \"select !(1 + 2);\" at line 89 err Error 1105: Unknown Unary Op opcode.Op"] [errorVerbose="run \"select !(1 + 2);\" at line 89 err Error 1105: Unknown Unary Op opcode.Op\nmain.

(*tester).execute\n\t/root/code/src/github.com/watchpoints/tidb/cmd/explaintest/main.go:356\nmain.

but I run this sql , it is right.
select !(1 + 2);

@tangenta
Copy link
Contributor

tangenta commented Nov 1, 2020

It is recommended to run make check before committing :)

@ti-srebot
Copy link
Contributor

/run-make check

1 similar comment
@ti-srebot
Copy link
Contributor

/run-make check

@watchpoints
Copy link
Contributor Author

@ti-srebot /run-all-tests

@ti-srebot
Copy link
Contributor

/run-all-tests

@tangenta
Copy link
Contributor

tangenta commented Nov 2, 2020

[2020-11-01T15:38:52.701Z] + GOPATH=/home/jenkins/agent/workspace/tidb_ghpr_check/go
[2020-11-01T15:38:52.701Z] + make check
[2020-11-01T15:38:52.701Z] gofmt (simplify)
[2020-11-01T15:39:48.903Z] cd tools/check; \
[2020-11-01T15:39:48.903Z] GO111MODULE=on go build -o ../bin/errcheck github.com/kisielk/errcheck
[2020-11-01T15:39:48.903Z] errcheck
[2020-11-01T15:39:58.910Z] cd tools/check; \
[2020-11-01T15:39:58.910Z] GO111MODULE=on go build -o ../bin/unconvert github.com/mdempsky/unconvert
[2020-11-01T15:39:58.910Z] unconvert check
[2020-11-01T15:41:07.428Z] cd tools/check; \
[2020-11-01T15:41:07.428Z] GO111MODULE=on go build -o ../bin/revive github.com/mgechev/revive
[2020-11-01T15:41:14.053Z] linting
[2020-11-01T15:41:24.026Z] go mod tidy
[2020-11-01T15:41:24.026Z] ./tools/check/check-tidy.sh
[2020-11-01T15:41:42.083Z] Files go.sum and /tmp/go.sum.before differ
[2020-11-01T15:41:42.083Z] make: *** [tidy] Error 1
script returned exit code 2

@watchpoints
Copy link
Contributor Author

[2020-11-01T15:38:52.701Z] + GOPATH=/home/jenkins/agent/workspace/tidb_ghpr_check/go
[2020-11-01T15:38:52.701Z] + make check
[2020-11-01T15:38:52.701Z] gofmt (simplify)
[2020-11-01T15:39:48.903Z] cd tools/check; \
[2020-11-01T15:39:48.903Z] GO111MODULE=on go build -o ../bin/errcheck github.com/kisielk/errcheck
[2020-11-01T15:39:48.903Z] errcheck
[2020-11-01T15:39:58.910Z] cd tools/check; \
[2020-11-01T15:39:58.910Z] GO111MODULE=on go build -o ../bin/unconvert github.com/mdempsky/unconvert
[2020-11-01T15:39:58.910Z] unconvert check
[2020-11-01T15:41:07.428Z] cd tools/check; \
[2020-11-01T15:41:07.428Z] GO111MODULE=on go build -o ../bin/revive github.com/mgechev/revive
[2020-11-01T15:41:14.053Z] linting
[2020-11-01T15:41:24.026Z] go mod tidy
[2020-11-01T15:41:24.026Z] ./tools/check/check-tidy.sh
[2020-11-01T15:41:42.083Z] Files go.sum and /tmp/go.sum.before differ
[2020-11-01T15:41:42.083Z] make: *** [tidy] Error 1
script returned exit code 2

go mod tidy
first run make check is right

**second run make check is wrong ,i not know why ?

Files go.sum and /tmp/go.sum.before differ**

i update tidb,now is

go: github.com/pingcap/tidb/_tools/util/timeutil imports
	github.com/uber-go/atomic: github.com/uber-go/atomic@v1.7.0: parsing go.mod:
	module declares its path as: go.uber.org/atomic
	        but was required as: github.com/uber-go/atomic

Copy link
Contributor

@tangenta tangenta 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 Nov 3, 2020
Copy link
Member

@wjhuang2016 wjhuang2016 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 removed the status/LGT1 Indicates that a PR has LGTM 1. label Nov 3, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 3, 2020
@wjhuang2016 wjhuang2016 added the type/bug-fix This PR fixes a bug. label Nov 3, 2020
Copy link
Member

@bb7133 bb7133 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/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Nov 3, 2020
@bb7133
Copy link
Member

bb7133 commented Nov 3, 2020

/merge

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

Your auto merge job has been accepted, waiting for:

  • 20313
  • 20122
  • 20715
  • 20715

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 3b4ba35 into pingcap:master Nov 3, 2020
@ti-challenge-bot
Copy link

@watchpoints, Congratulations, you get 400 in this PR, and your total score is 400 in high-performance challenge program.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/expression first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. sig/execution SIG execution sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT3 The PR has already had 3 LGTM. type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TiDB can create empty name index.
7 participants