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: Add lower limit of length of index column type #3864

Merged
merged 3 commits into from Jul 26, 2017
Merged

Conversation

jackysp
Copy link
Member

@jackysp jackysp commented Jul 24, 2017

Fix issue #3833 . @zimulala @hicqu PTAL

@@ -49,6 +49,10 @@ func buildIndexColumns(columns []*model.ColumnInfo, idxColNames []*ast.IndexColN
return nil, errKeyColumnDoesNotExits.Gen("column does not exist: %s", ic.Column.Name)
}

if col.Flen == 0 {
Copy link
Member

Choose a reason for hiding this comment

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

We should check this in plan/validator.go.

Copy link
Member Author

Choose a reason for hiding this comment

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

How to check table column type length in create index grammar?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is not a grammar check here. #3838 and #3835 will be fixed in plan/validator.go

Copy link
Member

Choose a reason for hiding this comment

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

You can check the Flen in this function:https://github.com/pingcap/tidb/blob/master/plan/validator.go#L259

Refer to this as an example.

Copy link
Member Author

Choose a reason for hiding this comment

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

A statement as create index idx on t (i) will not pass table column type, it is not the same case of create table statement.

@jackysp jackysp changed the title Add lower limit of length of index column type ddl: Add lower limit of length of index column type Jul 25, 2017
@shenli
Copy link
Member

shenli commented Jul 25, 2017

LGTM

@shenli
Copy link
Member

shenli commented Jul 25, 2017

@tiancaiamao @zimulala PTAL

@shenli shenli added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 25, 2017
@zimulala
Copy link
Contributor

LGTM

@zimulala zimulala added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 25, 2017
@zz-jason zz-jason merged commit 0806aac into master Jul 26, 2017
@zz-jason zz-jason deleted the yusp/index_char0 branch July 26, 2017 00:23
@jackysp jackysp added the sig/sql-infra SIG: SQL Infra label Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/sql-infra SIG: SQL Infra status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants