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

feature: 添加审核项 columns_must_have_index, 指定列必须添加索引 #175

Merged
merged 4 commits into from
Mar 27, 2020
Merged

feature: 添加审核项 columns_must_have_index, 指定列必须添加索引 #175

merged 4 commits into from
Mar 27, 2020

Conversation

soul-F
Copy link
Contributor

@soul-F soul-F commented Mar 21, 2020

添加审核项:
// 如果表包含以下列,列必须有索引。可指定多个列,以逗号分隔.列类型可选. 格式: 列名 [列类型,可选],...
ColumnsMustHaveIndex string toml:"columns_must_have_index" json:"columns_must_have_index"

@hanchuanchuan
Copy link
Owner

修改一下测试用的配置 config/config.toml.example ,把默认的审核级别加上,使其和内存中的默认全局配置相一致。

er_columns_must_have_index = 1
er_columns_must_have_index_type_err = 1

@codecov
Copy link

codecov bot commented Mar 21, 2020

Codecov Report

Merging #175 into master will decrease coverage by 0.0761%.
The diff coverage is 0.0000%.

@@               Coverage Diff                @@
##             master       #175        +/-   ##
================================================
- Coverage   58.7335%   58.6573%   -0.0762%     
================================================
  Files           371        371                
  Lines         83506      83582        +76     
================================================
- Hits          49046      49027        -19     
- Misses        30195      30284        +89     
- Partials       4265       4271         +6     

@soul-F
Copy link
Contributor Author

soul-F commented Mar 26, 2020

不知道这个功能是否PR呢~

@@ -3964,6 +4019,11 @@ func (s *session) checkAlterTable(node *ast.AlterTableStmt, sql string) {
}
}

if s.Inc.ColumnsMustHaveIndex != "" {
Copy link
Owner

Choose a reason for hiding this comment

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

alter table 也需要审核吗?
通常来说如果需要的话,应该是在添加字段后再添加索引的。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我是这么想的,
alter table add column,不加索引的时候,会有提示。
ALTER TABLE t1 ADD COLUMN c2 int,add index idx_c2(c2);
加列同时加索引的时候,就是正常的。
顺便养成同学们DDL 写一起的习惯。

@@ -784,6 +784,30 @@ primary key(id)) comment 'test';`

config.GetGlobalConfig().Inc.MustHaveColumns = ""

// 如果表包含以下列,列必须有索引。
Copy link
Owner

Choose a reason for hiding this comment

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

建议补充以下类型的测试

  1. 列索引,如create table t1(id int,c2 int unique)create table t1(c1 int primary key)
  2. 多列索引,如 CREATE TABLE t1(id int, c1 int, c2 int, index idx_1 (c1,c2));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个稍后补充一下

@hanchuanchuan
Copy link
Owner

首先非常感谢你的PR!
报歉是我操作有点问题,之前提了两个建议一直没点提交,我还奇怪怎么没有回复我~

@hanchuanchuan hanchuanchuan merged commit ebdf131 into hanchuanchuan:master Mar 27, 2020
@hanchuanchuan hanchuanchuan changed the title ColumnsMustHaveIndex feature: 添加审核项 columns_must_have_index, 指定列必须添加索引 Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants