From 0fe902ec77409324d298c6e5d456ec7062e73be1 Mon Sep 17 00:00:00 2001 From: Leon Zhang Date: Tue, 4 Dec 2018 13:45:03 +0800 Subject: [PATCH] fix #140 COL.015 text/blob default value only tobe `NULL`; COL.012 text/blob `NOT NULL` CONSTRAINT may be danagerous; --- Makefile | 3 +- advisor/heuristic.go | 34 +++++++++++++-- advisor/heuristic_test.go | 41 +++++++++++++++---- advisor/rules.go | 15 +++---- .../testdata/TestListHeuristicRules.golden | 12 +++--- .../TestMergeConflictHeuristicRules.golden | 4 +- common/chardet.go | 2 + database/explain_test.go | 2 +- doc/heuristic.md | 12 +++--- 9 files changed, 89 insertions(+), 36 deletions(-) diff --git a/Makefile b/Makefile index 885f2350..03cb66c5 100644 --- a/Makefile +++ b/Makefile @@ -107,11 +107,10 @@ doc: build # Add or change a heuristic rule .PHONY: heuristic -heuristic: doc docker +heuristic: doc @echo "\033[92mUpdate Heuristic rule golden files ...\033[0m" go test github.com/XiaoMi/soar/advisor -v -update -run TestListHeuristicRules go test github.com/XiaoMi/soar/advisor -v -update -run TestMergeConflictHeuristicRules - docker stop soar-mysql 2>/dev/null || true # Update vitess vendor .PHONY: vitess diff --git a/advisor/heuristic.go b/advisor/heuristic.go index 1ccab69d..4bcab81b 100644 --- a/advisor/heuristic.go +++ b/advisor/heuristic.go @@ -818,6 +818,12 @@ func (q *Query4Audit) RuleAddDefaultValue() Rule { colDefault = true } } + + switch c.Tp.Tp { + case mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: + colDefault = true + } + if !colDefault { rule = HeuristicRules["COL.004"] break @@ -835,6 +841,12 @@ func (q *Query4Audit) RuleAddDefaultValue() Rule { colDefault = true } } + + switch c.Tp.Tp { + case mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: + colDefault = true + } + if !colDefault { rule = HeuristicRules["COL.004"] break @@ -2493,8 +2505,8 @@ func (q *Query4Audit) RuleAlterDropKey() Rule { return rule } -// RuleCantBeNull COL.012 -func (q *Query4Audit) RuleCantBeNull() Rule { +// RuleBLOBNotNull COL.012 +func (q *Query4Audit) RuleBLOBNotNull() Rule { var rule = q.RuleOK() switch q.Stmt.(type) { case *sqlparser.DDL: @@ -2504,8 +2516,15 @@ func (q *Query4Audit) RuleCantBeNull() Rule { for _, col := range node.Cols { switch col.Tp.Tp { case mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: - if !mysql.HasNotNullFlag(col.Tp.Flag) { + for _, opt := range col.Options { + if opt.Tp == tidb.ColumnOptionNotNull { + rule = HeuristicRules["COL.012"] + break + } + } + if mysql.HasNotNullFlag(col.Tp.Flag) { rule = HeuristicRules["COL.012"] + break } } } @@ -2517,8 +2536,15 @@ func (q *Query4Audit) RuleCantBeNull() Rule { for _, col := range spec.NewColumns { switch col.Tp.Tp { case mysql.TypeBlob, mysql.TypeTinyBlob, mysql.TypeMediumBlob, mysql.TypeLongBlob: - if !mysql.HasNotNullFlag(col.Tp.Flag) { + for _, opt := range col.Options { + if opt.Tp == tidb.ColumnOptionNotNull { + rule = HeuristicRules["COL.012"] + break + } + } + if mysql.HasNotNullFlag(col.Tp.Flag) { rule = HeuristicRules["COL.012"] + break } } } diff --git a/advisor/heuristic_test.go b/advisor/heuristic_test.go index 8018e72e..0323a8cf 100644 --- a/advisor/heuristic_test.go +++ b/advisor/heuristic_test.go @@ -471,6 +471,8 @@ func TestRuleAddDefaultValue(t *testing.T) { `ALTER TABLE test modify id varchar(10) DEFAULT '';`, `ALTER TABLE test CHANGE id id varchar(10) DEFAULT '';`, "create table test(id int not null default 0 comment '用户id')", + `create table tb (a text)`, + `alter table tb add a text`, }, } for _, sql := range sqls[0] { @@ -2413,16 +2415,21 @@ func TestRuleAlterDropKey(t *testing.T) { // COL.012 func TestRuleCantBeNull(t *testing.T) { common.Log.Debug("Entering function: %s", common.GetFunctionName()) - sqls := []string{ - "CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` longblob, PRIMARY KEY (`id`));", - "alter TABLE `tbl` add column `c` longblob;", - "alter TABLE `tbl` add column `c` text;", - "alter TABLE `tbl` add column `c` blob;", + sqls := [][]string{ + { + "CREATE TABLE `tb`(`c` longblob NOT NULL);", + }, + { + "CREATE TABLE `tbl` (`c` longblob);", + "alter TABLE `tbl` add column `c` longblob;", + "alter TABLE `tbl` add column `c` text;", + "alter TABLE `tbl` add column `c` blob;", + }, } - for _, sql := range sqls { + for _, sql := range sqls[0] { q, err := NewQuery4Audit(sql) if err == nil { - rule := q.RuleCantBeNull() + rule := q.RuleBLOBNotNull() if rule.Item != "COL.012" { t.Error("Rule not match:", rule.Item, "Expect : COL.012") } @@ -2430,6 +2437,18 @@ func TestRuleCantBeNull(t *testing.T) { t.Error("sqlparser.Parse Error:", err) } } + + for _, sql := range sqls[1] { + q, err := NewQuery4Audit(sql) + if err == nil { + rule := q.RuleBLOBNotNull() + if rule.Item != "OK" { + t.Error("Rule not match:", rule.Item, "Expect : OK") + } + } else { + t.Error("sqlparser.Parse Error:", err) + } + } common.Log.Debug("Exiting function: %s", common.GetFunctionName()) } @@ -2806,7 +2825,13 @@ func TestRuleBlobDefaultValue(t *testing.T) { }, { "CREATE TABLE `tb` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` blob NOT NULL, PRIMARY KEY (`id`));", - "alter table `tb` add column `c` blob NOT NULL DEFAULT NULL;", + "CREATE TABLE `tb` (`col` text NOT NULL);", + "alter table `tb` add column `c` blob NOT NULL;", + "ALTER TABLE tb ADD COLUMN a BLOB DEFAULT NULL", + "CREATE TABLE tb ( a BLOB DEFAULT NULL)", + "alter TABLE `tbl` add column `c` longblob;", + "alter TABLE `tbl` add column `c` text;", + "alter TABLE `tbl` add column `c` blob;", }, } diff --git a/advisor/rules.go b/advisor/rules.go index dc553b42..5d0013c6 100644 --- a/advisor/rules.go +++ b/advisor/rules.go @@ -507,10 +507,10 @@ func init() { "COL.012": { Item: "COL.012", Severity: "L5", - Summary: "BLOB 和 TEXT 类型的字段不可设置为 NULL", - Content: `BLOB 和 TEXT 类型的字段不可设置为 NULL`, - Case: "CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` longblob, PRIMARY KEY (`id`));", - Func: (*Query4Audit).RuleCantBeNull, + Summary: "BLOB 和 TEXT 类型的字段不建议设置为 NOT NULL", + Content: `BLOB 和 TEXT 类型的字段无法指定非 NULL 的默认值，如果添加了 NOT NULL 限制，写入数据时又未对该字段指定值可能导致写入失败。`, + Case: "CREATE TABLE `tb`(`c` longblob NOT NULL);", + Func: (*Query4Audit).RuleBLOBNotNull, }, "COL.013": { Item: "COL.013", @@ -528,12 +528,13 @@ func init() { Case: "CREATE TABLE `tb2` ( `id` int(11) DEFAULT NULL, `col` char(10) CHARACTER SET utf8 DEFAULT NULL)", Func: (*Query4Audit).RuleColumnWithCharset, }, + // https://stackoverflow.com/questions/3466872/why-cant-a-text-column-have-a-default-value-in-mysql "COL.015": { Item: "COL.015", Severity: "L4", - Summary: "BLOB 类型的字段不可指定默认值", - Content: `BLOB 类型的字段不可指定默认值`, - Case: "CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` blob NOT NULL DEFAULT '', PRIMARY KEY (`id`));", + Summary: "TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值", + Content: `MySQL 数据库中 TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值。TEXT最大长度为2^16-1个字符，MEDIUMTEXT最大长度为2^32-1个字符，LONGTEXT最大长度为2^64-1个字符。`, + Case: "CREATE TABLE `tbl` (`c` blob DEFAULT NULL);", Func: (*Query4Audit).RuleBlobDefaultValue, }, "COL.016": { diff --git a/advisor/testdata/TestListHeuristicRules.golden b/advisor/testdata/TestListHeuristicRules.golden index 32dc038a..ba8a11c8 100644 --- a/advisor/testdata/TestListHeuristicRules.golden +++ b/advisor/testdata/TestListHeuristicRules.golden @@ -452,15 +452,15 @@ create table tab1(status ENUM('new','in progress','fixed')) ```sql select c1,c2,c3 from tbl where c4 is null or c4 <> 1 ``` -## BLOB 和 TEXT 类型的字段不可设置为 NULL +## BLOB 和 TEXT 类型的字段不建议设置为 NOT NULL * **Item**:COL.012 * **Severity**:L5 -* **Content**:BLOB 和 TEXT 类型的字段不可设置为 NULL +* **Content**:BLOB 和 TEXT 类型的字段无法指定非 NULL 的默认值，如果添加了 NOT NULL 限制，写入数据时又未对该字段指定值可能导致写入失败。 * **Case**: ```sql -CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` longblob, PRIMARY KEY (`id`)); +CREATE TABLE `tb`(`c` longblob NOT NULL); ``` ## TIMESTAMP 类型未设置默认值 @@ -482,15 +482,15 @@ CREATE TABLE tbl( `id` bigint not null, `create_time` timestamp); ```sql CREATE TABLE `tb2` ( `id` int(11) DEFAULT NULL, `col` char(10) CHARACTER SET utf8 DEFAULT NULL) ``` -## BLOB 类型的字段不可指定默认值 +## TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值 * **Item**:COL.015 * **Severity**:L4 -* **Content**:BLOB 类型的字段不可指定默认值 +* **Content**:MySQL 数据库中 TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值。TEXT最大长度为2^16-1个字符，MEDIUMTEXT最大长度为2^32-1个字符，LONGTEXT最大长度为2^64-1个字符。 * **Case**: ```sql -CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` blob NOT NULL DEFAULT '', PRIMARY KEY (`id`)); +CREATE TABLE `tbl` (`c` blob DEFAULT NULL); ``` ## 整型定义建议采用 INT(10) 或 BIGINT(20) diff --git a/advisor/testdata/TestMergeConflictHeuristicRules.golden b/advisor/testdata/TestMergeConflictHeuristicRules.golden index a4ff936f..e60fc057 100644 --- a/advisor/testdata/TestMergeConflictHeuristicRules.golden +++ b/advisor/testdata/TestMergeConflictHeuristicRules.golden @@ -42,10 +42,10 @@ advisor.Rule{Item:"COL.008", Severity:"L1", Summary:"可使用 VARCHAR 代替 CH advisor.Rule{Item:"COL.009", Severity:"L2", Summary:"建议使用精确的数据类型", Content:"实际上，任何使用 FLOAT, REAL 或 DOUBLE PRECISION 数据类型的设计都有可能是反模式。大多数应用程序使用的浮点数的取值范围并不需要达到IEEE 754标准所定义的最大/最小区间。在计算总量时，非精确浮点数所积累的影响是严重的。使用 SQL 中的 NUMERIC 或 DECIMAL 类型来代替 FLOAT 及其类似的数据类型进行固定精度的小数存储。这些数据类型精确地根据您定义这一列时指定的精度来存储数据。尽可能不要使用浮点数。", Case:"CREATE TABLE tab2 (p_id BIGINT UNSIGNED NOT NULL,a_id BIGINT UNSIGNED NOT NULL,hours float not null,PRIMARY KEY (p_id, a_id))", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} advisor.Rule{Item:"COL.010", Severity:"L2", Summary:"不建议使用 ENUM 数据类型", Content:"ENUM 定义了列中值的类型，使用字符串表示 ENUM 里的值时，实际存储在列中的数据是这些值在定义时的序数。因此，这列的数据是字节对齐的，当您进行一次排序查询时，结果是按照实际存储的序数值排序的，而不是按字符串值的字母顺序排序的。这可能不是您所希望的。没有什么语法支持从 ENUM 或者 check 约束中添加或删除一个值；您只能使用一个新的集合重新定义这一列。如果您打算废弃一个选项，您可能会为历史数据而烦恼。作为一种策略，改变元数据——也就是说，改变表和列的定义——应该是不常见的，并且要注意测试和质量保证。有一个更好的解决方案来约束一列中的可选值:创建一张检查表，每一行包含一个允许在列中出现的候选值；然后在引用新表的旧表上声明一个外键约束。", Case:"create table tab1(status ENUM('new','in progress','fixed'))", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} advisor.Rule{Item:"COL.011", Severity:"L0", Summary:"当需要唯一约束时才使用 NULL，仅当列不能有缺失值时才使用 NOT NULL", Content:"NULL 和0是不同的，10乘以 NULL 还是 NULL。NULL 和空字符串是不一样的。将一个字符串和标准 SQL 中的 NULL 联合起来的结果还是 NULL。NULL 和 FALSE 也是不同的。AND、OR 和 NOT 这三个布尔操作如果涉及 NULL，其结果也让很多人感到困惑。当您将一列声明为 NOT NULL 时，也就是说这列中的每一个值都必须存在且是有意义的。使用 NULL 来表示任意类型不存在的空值。 当您将一列声明为 NOT NULL 时，也就是说这列中的每一个值都必须存在且是有意义的。", Case:"select c1,c2,c3 from tbl where c4 is null or c4 <> 1", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} -advisor.Rule{Item:"COL.012", Severity:"L5", Summary:"BLOB 和 TEXT 类型的字段不可设置为 NULL", Content:"BLOB 和 TEXT 类型的字段不可设置为 NULL", Case:"CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` longblob, PRIMARY KEY (`id`));", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} +advisor.Rule{Item:"COL.012", Severity:"L5", Summary:"BLOB 和 TEXT 类型的字段不建议设置为 NOT NULL", Content:"BLOB 和 TEXT 类型的字段无法指定非 NULL 的默认值，如果添加了 NOT NULL 限制，写入数据时又未对该字段指定值可能导致写入失败。", Case:"CREATE TABLE `tb`(`c` longblob NOT NULL);", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} advisor.Rule{Item:"COL.013", Severity:"L4", Summary:"TIMESTAMP 类型未设置默认值", Content:"TIMESTAMP 类型未设置默认值", Case:"CREATE TABLE tbl( `id` bigint not null, `create_time` timestamp);", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} advisor.Rule{Item:"COL.014", Severity:"L5", Summary:"为列指定了字符集", Content:"建议列与表使用同一个字符集，不要单独指定列的字符集。", Case:"CREATE TABLE `tb2` ( `id` int(11) DEFAULT NULL, `col` char(10) CHARACTER SET utf8 DEFAULT NULL)", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} -advisor.Rule{Item:"COL.015", Severity:"L4", Summary:"BLOB 类型的字段不可指定默认值", Content:"BLOB 类型的字段不可指定默认值", Case:"CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` blob NOT NULL DEFAULT '', PRIMARY KEY (`id`));", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} +advisor.Rule{Item:"COL.015", Severity:"L4", Summary:"TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值", Content:"MySQL 数据库中 TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值。TEXT最大长度为2^16-1个字符，MEDIUMTEXT最大长度为2^32-1个字符，LONGTEXT最大长度为2^64-1个字符。", Case:"CREATE TABLE `tbl` (`c` blob DEFAULT NULL);", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} advisor.Rule{Item:"COL.016", Severity:"L1", Summary:"整型定义建议采用 INT(10) 或 BIGINT(20)", Content:"INT(M) 在 integer 数据类型中，M 表示最大显示宽度。 在 INT(M) 中，M 的值跟 INT(M) 所占多少存储空间并无任何关系。 INT(3)、INT(4)、INT(8) 在磁盘上都是占用 4 bytes 的存储空间。", Case:"CREATE TABLE tab (a INT(1));", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} advisor.Rule{Item:"COL.017", Severity:"L2", Summary:"VARCHAR 定义长度过长", Content:"varchar 是可变长字符串，不预先分配存储空间，长度不要超过1024，如果存储长度过长 MySQL 将定义字段类型为 text，独立出来一张表，用主键来对应，避免影响其它字段索引效率。", Case:"CREATE TABLE tab (a varchar(3500));", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} advisor.Rule{Item:"DIS.001", Severity:"L1", Summary:"消除不必要的 DISTINCT 条件", Content:"太多DISTINCT条件是复杂的裹脚布式查询的症状。考虑将复杂查询分解成许多简单的查询，并减少DISTINCT条件的数量。如果主键列是列的结果集的一部分，则DISTINCT条件可能没有影响。", Case:"SELECT DISTINCT c.c_id,count(DISTINCT c.c_name),count(DISTINCT c.c_e),count(DISTINCT c.c_n),count(DISTINCT c.c_me),c.c_d FROM (select distinct id, name from B) as e WHERE e.country_id = c.country_id", Position:0, Func:func(*advisor.Query4Audit) advisor.Rule {...}} diff --git a/common/chardet.go b/common/chardet.go index 223b3300..a0664327 100644 --- a/common/chardet.go +++ b/common/chardet.go @@ -17,6 +17,7 @@ package common import ( + "github.com/kr/pretty" "github.com/saintfish/chardet" ) @@ -39,6 +40,7 @@ func Chardet(buf []byte) string { if err != nil { return charset } + Log.Debug("Chardet DetectAll Result: %s", pretty.Sprint(result)) // SOAR's main user speak Chinese, GB-18030, UTF-8 are higher suggested for _, r := range result { diff --git a/database/explain_test.go b/database/explain_test.go index 01ef50f9..7983496d 100644 --- a/database/explain_test.go +++ b/database/explain_test.go @@ -38,7 +38,7 @@ func init() { Database: common.Config.OnlineDSN.Schema, } if _, err := connTest.Version(); err != nil { - common.Log.Critical("Test env Error: %v", err) + fmt.Printf("Test env Error: %v", err) os.Exit(0) } } diff --git a/doc/heuristic.md b/doc/heuristic.md index 32dc038a..ba8a11c8 100644 --- a/doc/heuristic.md +++ b/doc/heuristic.md @@ -452,15 +452,15 @@ create table tab1(status ENUM('new','in progress','fixed')) ```sql select c1,c2,c3 from tbl where c4 is null or c4 <> 1 ``` -## BLOB 和 TEXT 类型的字段不可设置为 NULL +## BLOB 和 TEXT 类型的字段不建议设置为 NOT NULL * **Item**:COL.012 * **Severity**:L5 -* **Content**:BLOB 和 TEXT 类型的字段不可设置为 NULL +* **Content**:BLOB 和 TEXT 类型的字段无法指定非 NULL 的默认值，如果添加了 NOT NULL 限制，写入数据时又未对该字段指定值可能导致写入失败。 * **Case**: ```sql -CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` longblob, PRIMARY KEY (`id`)); +CREATE TABLE `tb`(`c` longblob NOT NULL); ``` ## TIMESTAMP 类型未设置默认值 @@ -482,15 +482,15 @@ CREATE TABLE tbl( `id` bigint not null, `create_time` timestamp); ```sql CREATE TABLE `tb2` ( `id` int(11) DEFAULT NULL, `col` char(10) CHARACTER SET utf8 DEFAULT NULL) ``` -## BLOB 类型的字段不可指定默认值 +## TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值 * **Item**:COL.015 * **Severity**:L4 -* **Content**:BLOB 类型的字段不可指定默认值 +* **Content**:MySQL 数据库中 TEXT 和 BLOB 类型的字段不可指定非 NULL 的默认值。TEXT最大长度为2^16-1个字符，MEDIUMTEXT最大长度为2^32-1个字符，LONGTEXT最大长度为2^64-1个字符。 * **Case**: ```sql -CREATE TABLE `tbl` ( `id` int(10) unsigned NOT NULL AUTO_INCREMENT, `c` blob NOT NULL DEFAULT '', PRIMARY KEY (`id`)); +CREATE TABLE `tbl` (`c` blob DEFAULT NULL); ``` ## 整型定义建议采用 INT(10) 或 BIGINT(20)