diff --git a/advisor/heuristic.go b/advisor/heuristic.go index 8867dd1db636661216a180bdc2c0c4a5c124cc27..82f430faf0903fff989c86c291afac0dec675cc0 100644 --- a/advisor/heuristic.go +++ b/advisor/heuristic.go @@ -1347,6 +1347,83 @@ func (q *Query4Audit) RuleMultiCompare() Rule { return rule } +// RuleCreateOnUpdate RES.010 +func (q *Query4Audit) RuleCreateOnUpdate() Rule { + var rule = q.RuleOK() + switch q.Stmt.(type) { + case *sqlparser.DDL: + for _, tiStmt := range q.TiStmt { + switch node := tiStmt.(type) { + case *tidb.CreateTableStmt: + for _, col := range node.Cols { + if col.Tp == nil { + continue + } + for _, op := range col.Options { + if op.Tp == tidb.ColumnOptionOnUpdate { + rule = HeuristicRules["RES.010"] + return rule + } + } + } + + case *tidb.AlterTableStmt: + for _, spec := range node.Specs { + switch spec.Tp { + case tidb.AlterTableAddColumns, tidb.AlterTableModifyColumn, tidb.AlterTableChangeColumn: + for _, col := range spec.NewColumns { + if col.Tp == nil { + continue + } + for _, op := range col.Options { + if op.Tp == tidb.ColumnOptionOnUpdate { + rule = HeuristicRules["RES.010"] + return rule + } + } + } + } + } + } + } + } + return rule +} + +// RuleUpdateOnUpdate RES.011 +func (idxAdv *IndexAdvisor) RuleUpdateOnUpdate() Rule { + rule := HeuristicRules["OK"] + // 未开启测试环境不进行检查 + if common.Config.TestDSN.Disable { + return rule + } + err := sqlparser.Walk(func(node sqlparser.SQLNode) (kontinue bool, err error) { + switch stmt := node.(type) { + case *sqlparser.Update: + for _, tbExpr := range stmt.TableExprs { + ddl, err := idxAdv.vEnv.ShowCreateTable(sqlparser.String(tbExpr)) + if err != nil { + common.Log.Error("RuleMaxTextColsCount create statement got failed: %s", err.Error()) + return false, err + } + if strings.Contains(ddl, "ON UPDATE") { + rule = HeuristicRules["RES.011"] + break + } + } + for _, setExpr := range stmt.Exprs { + tup := strings.Split(sqlparser.String(setExpr), " = ") + if len(tup) == 2 && tup[0] == tup[1] { + rule = HeuristicRules["OK"] + } + } + } + return true, nil + }, idxAdv.Ast) + common.LogIfError(err, "") + return rule +} + // RuleStandardINEQ STA.001 func (q *Query4Audit) RuleStandardINEQ() Rule { var rule = q.RuleOK() diff --git a/advisor/heuristic_test.go b/advisor/heuristic_test.go index 0243243282d4d8eec471431a118af42a0f6bafbc..51de9e7fd15a1cb29e15b542bc54b6c7d5133cb6 100644 --- a/advisor/heuristic_test.go +++ b/advisor/heuristic_test.go @@ -1004,6 +1004,111 @@ func TestRuleMultiCompare(t *testing.T) { common.Log.Debug("Exiting function: %s", common.GetFunctionName()) } +// RES.010 +func TestRuleCreateOnUpdate(t *testing.T) { + common.Log.Debug("Entering function: %s", common.GetFunctionName()) + sqls := [][]string{ + { + `CREATE TABLE category ( + category_id TINYINT UNSIGNED NOT NULL AUTO_INCREMENT, + name VARCHAR(25) NOT NULL, + last_update TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, + PRIMARY KEY (category_id) +)`, + }, + { + `CREATE TABLE category ( + category_id TINYINT UNSIGNED NOT NULL AUTO_INCREMENT, + name VARCHAR(25) NOT NULL, + last_update TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + PRIMARY KEY (category_id) +)`, + }, + } + + for _, sql := range sqls[0] { + q, err := NewQuery4Audit(sql) + if err == nil { + rule := q.RuleCreateOnUpdate() + if rule.Item != "RES.010" { + t.Error("Rule not match:", rule.Item, "Expect : RES.010, SQL: ", sql) + } + } else { + t.Error("sqlparser.Parse Error:", err) + } + } + + for _, sql := range sqls[1] { + q, err := NewQuery4Audit(sql) + if err == nil { + rule := q.RuleCreateOnUpdate() + if rule.Item != "OK" { + t.Error("Rule not match:", rule.Item, "Expect : OK, SQL: ", sql) + } + } else { + t.Error("sqlparser.Parse Error:", err) + } + } + common.Log.Debug("Exiting function: %s", common.GetFunctionName()) +} + +// RES.011 +func TestRuleUpdateOnUpdate(t *testing.T) { + common.Log.Debug("Entering function: %s", common.GetFunctionName()) + sqls := [][]string{ + { + `UPDATE category SET name='ActioN' WHERE category_id=1`, + }, + { + `select * from film limit 1`, + "UPDATE category SET name='ActioN', last_update=last_update WHERE category_id=1", + }, + } + + for _, sql := range sqls[0] { + vEnv.BuildVirtualEnv(rEnv, sql) + stmt, syntaxErr := sqlparser.Parse(sql) + if syntaxErr != nil { + t.Error(syntaxErr) + } + + q := &Query4Audit{Query: sql, Stmt: stmt} + idxAdvisor, err := NewAdvisor(vEnv, *rEnv, *q) + if err != nil { + t.Error("NewAdvisor Error: ", err, "SQL: ", sql) + } + + if idxAdvisor != nil { + rule := idxAdvisor.RuleUpdateOnUpdate() + if rule.Item != "RES.011" { + t.Error("Rule not match:", rule.Item, "Expect : RES.011, SQL:", sql) + } + } + } + + for _, sql := range sqls[1] { + vEnv.BuildVirtualEnv(rEnv, sql) + stmt, syntaxErr := sqlparser.Parse(sql) + if syntaxErr != nil { + t.Error(syntaxErr) + } + + q := &Query4Audit{Query: sql, Stmt: stmt} + idxAdvisor, err := NewAdvisor(vEnv, *rEnv, *q) + if err != nil { + t.Error("NewAdvisor Error: ", err, "SQL: ", sql) + } + + if idxAdvisor != nil { + rule := idxAdvisor.RuleUpdateOnUpdate() + if rule.Item != "OK" { + t.Error("Rule not match:", rule.Item, "Expect : OK, SQL:", sql) + } + } + } + common.Log.Debug("Exiting function: %s", common.GetFunctionName()) +} + // STA.001 func TestRuleStandardINEQ(t *testing.T) { common.Log.Debug("Entering function: %s", common.GetFunctionName()) diff --git a/advisor/rules.go b/advisor/rules.go index 6f09e13124cfaa32c8796fbd73dfbe258f51d4c3..f899b320106aa7fbac4e7ffd75955007c87f2e81 100644 --- a/advisor/rules.go +++ b/advisor/rules.go @@ -429,7 +429,7 @@ func init() { Summary: "不要 UPDATE 主键", Content: `主键是数据表中记录的唯一标识符，不建议频繁更新主键列，这将影响元数据统计信息进而影响正常的查询。`, Case: "update tbl set col=1", - Func: (*Query4Audit).RuleOK, // 该建议在indexAdvisor中给 + Func: (*Query4Audit).RuleOK, // 该建议在indexAdvisor中给 RuleUpdatePrimaryKey }, "COL.001": { Item: "COL.001", @@ -689,7 +689,7 @@ func init() { Summary: "不建议对等值查询列使用 GROUP BY", Content: `GROUP BY 中的列在前面的 WHERE 条件中使用了等值查询，对这样的列进行 GROUP BY 意义不大。`, Case: "select film_id, title from film where release_year='2006' group by release_year", - Func: (*Query4Audit).RuleOK, // 该建议在indexAdvisor中给 + Func: (*Query4Audit).RuleOK, // 该建议在indexAdvisor中给 RuleGroupByConst }, "JOI.001": { Item: "JOI.001", @@ -989,6 +989,22 @@ func init() { Case: "SELECT * FROM tbl WHERE col = col = 'abc'", Func: (*Query4Audit).RuleMultiCompare, }, + "RES.010": { + Item: "RES.010", + Severity: "L2", + Summary: "建表语句中定义为 ON UPDATE CURRENT_TIMESTAMP 的字段不建议包含业务逻辑", + Content: "定义为 ON UPDATE CURRENT_TIMESTAMP 的字段在该表其他字段更新时会联动修改，如果包含业务逻辑用户可见会埋下隐患。后续如有批量修改数据却又不想修改该字段时会导致数据错误。", + Case: `CREATE TABLE category (category_id TINYINT UNSIGNED NOT NULL AUTO_INCREMENT, name VARCHAR(25) NOT NULL, last_update TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, PRIMARY KEY (category_id)`, + Func: (*Query4Audit).RuleCreateOnUpdate, + }, + "RES.011": { + Item: "RES.011", + Severity: "L2", + Summary: "更新请求操作的表包含 ON UPDATE CURRENT_TIMESTAMP 字段", + Content: "定义为 ON UPDATE CURRENT_TIMESTAMP 的字段在该表其他字段更新时会联动修改，请注意检查。如不想修改字段的更新时间可以使用如下方法：UPDATE category SET name='ActioN', last_update=last_update WHERE category_id=1", + Case: "UPDATE category SET name='ActioN', last_update=last_update WHERE category_id=1", + Func: (*Query4Audit).RuleOK, // 该建议在indexAdvisor中给 RuleUpdateOnUpdate + }, "SEC.001": { Item: "SEC.001", Severity: "L0", diff --git a/doc/heuristic.md b/doc/heuristic.md index f0f74a9bc06f0a7941935fb9f02a6e5bc661d9b1..820793a7108cde3a49752f439a2dbdb35fef0871 100644 --- a/doc/heuristic.md +++ b/doc/heuristic.md @@ -1052,6 +1052,26 @@ LOAD DATA INFILE 'data.txt' INTO TABLE db2.my_table; ```sql SELECT * FROM tbl WHERE col = col = 'abc' ``` +## 建表语句中定义为 ON UPDATE CURRENT\_TIMESTAMP 的字段不建议包含业务逻辑 + +* **Item**:RES.010 +* **Severity**:L2 +* **Content**:定义为 ON UPDATE CURRENT\_TIMESTAMP 的字段在该表其他字段更新时会联动修改，如果包含业务逻辑用户可见会埋下隐患。后续如有批量修改数据却又不想修改该字段时会导致数据错误。 +* **Case**: + +```sql +CREATE TABLE category (category_id TINYINT UNSIGNED NOT NULL AUTO_INCREMENT, name VARCHAR(25) NOT NULL, last_update TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, PRIMARY KEY (category_id) +``` +## 更新请求操作的表包含 ON UPDATE CURRENT\_TIMESTAMP 字段 + +* **Item**:RES.011 +* **Severity**:L2 +* **Content**:定义为 ON UPDATE CURRENT\_TIMESTAMP 的字段在该表其他字段更新时会联动修改，请注意检查。如不想修改字段的更新时间可以使用如下方法：UPDATE category SET name='ActioN', last\_update=last\_update WHERE category\_id=1 +* **Case**: + +```sql +UPDATE category SET name='ActioN', last_update=last_update WHERE category_id=1 +``` ## 请谨慎使用TRUNCATE操作 * **Item**:SEC.001