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 alter table add column(col_name column_definition) #5054

Merged
merged 4 commits into from Nov 10, 2017

Conversation

Projects
None yet
4 participants
@zimulala
Member

zimulala commented Nov 9, 2017

zimulala and others added some commits Nov 9, 2017

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Nov 10, 2017

Member

LGTM

Member

shenli commented Nov 10, 2017

LGTM

@shenli

This comment has been minimized.

Show comment
Hide comment
@shenli

shenli Nov 10, 2017

Member

/run-all-tests

Member

shenli commented Nov 10, 2017

/run-all-tests

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala
Member

zimulala commented Nov 10, 2017

@@ -842,12 +842,11 @@ AlterTableSpec:
Position: $4.(*ast.ColumnPosition),
}
}
| "ADD" ColumnKeywordOpt '(' ColumnDef ColumnPosition ')'

This comment has been minimized.

@holys

holys Nov 10, 2017

Member

Why remove ColumnPosition ?

@holys

holys Nov 10, 2017

Member

Why remove ColumnPosition ?

This comment has been minimized.

@zimulala

zimulala Nov 10, 2017

Member

Because we need to be compatible with MySQL.

  | ADD [COLUMN] col_name column_definition
        [FIRST | AFTER col_name]
  | ADD [COLUMN] (col_name column_definition,...)
@zimulala

zimulala Nov 10, 2017

Member

Because we need to be compatible with MySQL.

  | ADD [COLUMN] col_name column_definition
        [FIRST | AFTER col_name]
  | ADD [COLUMN] (col_name column_definition,...)
@@ -1467,7 +1467,8 @@ func (s *testParserSuite) TestDDL(c *C) {
{"CREATE TABLE IF NOT EXISTS `general_log` (`event_time` timestamp(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6),`user_host` mediumtext NOT NULL,`thread_id` bigint(20) unsigned NOT NULL,`server_id` int(10) unsigned NOT NULL,`command_type` varchar(64) NOT NULL,`argument` mediumblob NOT NULL) ENGINE=CSV DEFAULT CHARSET=utf8 COMMENT='General log'", true},
// for alter table
{"ALTER TABLE t ADD COLUMN( a SMALLINT UNSIGNED )", true},
{"ALTER TABLE t ADD COLUMN (a SMALLINT UNSIGNED)", true},
{"ALTER TABLE t ADD COLUMN (a SMALLINT UNSIGNED FIRST)", false},

This comment has been minimized.

@holys

holys Nov 10, 2017

Member

It seems this DDL statement not valid for MySQL.

mysql> ALTER TABLE s1 ADD COLUMN (a SMALLINT UNSIGNED FIRST);
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FIRST)' at line 1

Server version: 5.7.18-log MySQL Community Server (GPL)

@holys

holys Nov 10, 2017

Member

It seems this DDL statement not valid for MySQL.

mysql> ALTER TABLE s1 ADD COLUMN (a SMALLINT UNSIGNED FIRST);
ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FIRST)' at line 1

Server version: 5.7.18-log MySQL Community Server (GPL)

This comment has been minimized.

@zimulala

zimulala Nov 10, 2017

Member

Yes, so the result is false.

@zimulala

zimulala Nov 10, 2017

Member

Yes, so the result is false.

This comment has been minimized.

@holys

holys Nov 10, 2017

Member

Oh, sorry for my carelessness.

@holys

holys Nov 10, 2017

Member

Oh, sorry for my carelessness.

@XuHuaiyu

LGTM

@XuHuaiyu XuHuaiyu added status/LGT2 and removed status/LGT1 labels Nov 10, 2017

@holys

This comment has been minimized.

Show comment
Hide comment
@holys

holys Nov 10, 2017

Member

LGTM

Member

holys commented Nov 10, 2017

LGTM

@zimulala zimulala merged commit dd5767a into pingcap:master Nov 10, 2017

4 checks passed

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

@zimulala zimulala deleted the zimulala:parser-add-column branch Nov 10, 2017

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

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