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

parser: Fix the bug of `alter table table_options, other_alter_specification` #4931

Merged
merged 7 commits into from Oct 31, 2017

Conversation

Projects
None yet
7 participants
@zimulala
Member

zimulala commented Oct 27, 2017

Fix #4927

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala
Member

zimulala commented Oct 27, 2017

@zz-jason

This comment has been minimized.

Show comment
Hide comment
@zz-jason

zz-jason Oct 27, 2017

Member

LGTM

Member

zz-jason commented Oct 27, 2017

LGTM

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Oct 27, 2017

Member

LGTM

Member

shenli commented Oct 27, 2017

LGTM

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Oct 27, 2017

Member

/run-all-tests

Member

shenli commented Oct 27, 2017

/run-all-tests

@coocood

This comment has been minimized.

Show comment
Hide comment
@coocood

coocood Oct 27, 2017

Member

@zimulala
Seems we need to define a new TableOptList that is separated only by space for alter table statement. Then restore the lowerThanComma precedence.

The following is copied from the MySQL parser.

create_table_options_space_separated:
          create_table_option
        | create_table_option create_table_options_space_separated
Member

coocood commented Oct 27, 2017

@zimulala
Seems we need to define a new TableOptList that is separated only by space for alter table statement. Then restore the lowerThanComma precedence.

The following is copied from the MySQL parser.

create_table_options_space_separated:
          create_table_option
        | create_table_option create_table_options_space_separated
@winkyao

This comment has been minimized.

Show comment
Hide comment
@winkyao

winkyao Oct 30, 2017

Member

agree with @coocood

Member

winkyao commented Oct 30, 2017

agree with @coocood

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Oct 31, 2017

Member

@coocood @winkyao
Alter table statement also can separate by comma. I tried on MySQL.
PTAL

Member

zimulala commented Oct 31, 2017

@coocood @winkyao
Alter table statement also can separate by comma. I tried on MySQL.
PTAL

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Oct 31, 2017

Member

/run-all-tests

Member

zimulala commented Oct 31, 2017

/run-all-tests

{"ALTER TABLE t ADD COLUMN a SMALLINT, ENGINE = '', default COLLATE = utf8_general_ci", true},
{"ALTER TABLE t ENGINE = '', COMMENT='', default COLLATE = utf8_general_ci", true},
{"ALTER TABLE t ENGINE = '', ADD COLUMN a SMALLINT", true},
{"ALTER TABLE t default COLLATE = utf8_general_ci, ENGINE = '', ADD COLUMN a SMALLINT", true},

This comment has been minimized.

@holys

holys Oct 31, 2017

Member

How about adding a case
(engine value in back-quote)

ALTER TABLE `foo`.`bar` ENGINE=``   

The above one appears in aliyun rds frequently, and can't parsed by TiDB parser.

@holys

holys Oct 31, 2017

Member

How about adding a case
(engine value in back-quote)

ALTER TABLE `foo`.`bar` ENGINE=``   

The above one appears in aliyun rds frequently, and can't parsed by TiDB parser.

This comment has been minimized.

@zimulala

zimulala Oct 31, 2017

Member

Done.

@zimulala
@coocood

LGTM

coocood and others added some commits Oct 31, 2017

@hanfei1991 hanfei1991 merged commit ecf0710 into pingcap:master Oct 31, 2017

3 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tidb/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@zimulala zimulala deleted the zimulala:fix-issue4927 branch Oct 31, 2017

dbjoa added a commit to cloud-pi/tidb that referenced this pull request Nov 6, 2017

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