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.

Sign up for GitHub

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

Jump to bottom

fix issue 1783 2 #1817

Merged
merged 8 commits into from
Sep 14, 2023
Merged

fix issue 1783 2 #1817

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f3d3fe3
check union index
hasa1K Sep 11, 2023
2ef7b20
change unit test
hasa1K Sep 12, 2023
32c67bc
use GetTableRowCount and GetColumnCardinality instead GetIndexSelecti…
hasa1K Sep 13, 2023
0cbe0ba
update unit test
hasa1K Sep 13, 2023
31dbca3
Merge branch 'main' of github.com:actiontech/sqle into fix-issue1783-2
hasa1K Sep 13, 2023
74214a7
change error code
hasa1K Sep 13, 2023
f8a93e9
add CreateTableStmt && change message
hasa1K Sep 13, 2023
f38817d
update unit test
hasa1K Sep 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 108 additions & 1 deletion sqle/driver/mysql/audit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func NewMockInspectWithIsExecutedSQL(e *executor.Executor) *MysqlDriverImpl {
DDLOSCMinSize: 16,
DDLGhostMinSize: 16,
DMLRollbackMaxRows: 1000,
isExecutedSQL: true,
isExecutedSQL: true,
},
dbConn: e,
}
Expand Down Expand Up @@ -155,6 +155,7 @@ func runDefaultRulesInspectCase(t *testing.T, desc string, i *MysqlDriverImpl, s
rulepkg.DMLCheckAggregate: {},
rulepkg.DDLCheckColumnNotNULL: {},
rulepkg.DDLCheckTableRows: {},
rulepkg.DDLCheckCompositeIndexDistinction: {},
}
for i := range rulepkg.RuleHandlers {
handler := rulepkg.RuleHandlers[i]
Expand Down Expand Up @@ -5720,3 +5721,109 @@ func TestCheckTableRows(t *testing.T) {
inspect3 := NewMockInspectWithIsExecutedSQL(e)
runSingleRuleInspectCase(rule, t, "", inspect3, "CREATE INDEX idx_union1 ON exist_db.exist_tb_1 (v1,v2);", newTestResult())
}

func TestDDLCheckCompositeIndexDistinction(t *testing.T) {
rule := rulepkg.RuleHandlerMap[rulepkg.DDLCheckCompositeIndexDistinction].Rule
e, handler, err := executor.NewMockExecutor()
assert.NoError(t, err)

inspect1 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
runSingleRuleInspectCase(rule, t, "", inspect1, "CREATE INDEX idx_union1 ON exist_db.exist_tb_1 (v1,v2);", newTestResult())

inspect2 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect2, "CREATE INDEX idx_union1 ON exist_db.exist_tb_1 (v1,v2);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2)可调整为(v2，v1)"))

inspect3 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect3, "ALTER TABLE exist_db.exist_tb_1 ADD INDEX index_name ( v1, v2);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2)可调整为(v2，v1)"))

inspect5 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("90"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect5, "ALTER TABLE exist_db.exist_tb_1 ADD INDEX index_name ( v1, v2);", newTestResult())

inspect6 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect6, "ALTER TABLE exist_db.exist_tb_1 ADD unique INDEX index_name ( v1, v2);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2)可调整为(v2，v1)"))

inspect7 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("90"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect7, "CREATE index idx_union1 ON exist_db.exist_tb_1 (v1,v2);", newTestResult())

inspect8 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect8, "CREATE index idx_union1 ON exist_db.exist_tb_1 (v1,v2);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2)可调整为(v2，v1)"))

inspect9 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect9, "ALTER TABLE exist_db.exist_tb_1 ADD key index_name ( v1, v2);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2)可调整为(v2，v1)"))

inspect10 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect10, "ALTER TABLE exist_db.exist_tb_1 ADD unique key index_name ( v1, v2);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2)可调整为(v2，v1)"))

inspect11 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_1`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect11, "ALTER TABLE exist_db.exist_tb_1 ADD unique key index_name (v1, v2),ADD unique key index_name1 (v1);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2)可调整为(v2，v1)"))

inspect12 := NewMockInspect(e)
runSingleRuleInspectCase(rule, t, "", inspect12, "ALTER TABLE exist_db.exist_tb_1 ADD unique key index_name1 (v1);", newTestResult())

inspect13 := NewMockInspect(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_3`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("100"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_3`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v3`) as cardinality from `exist_db`.`exist_tb_3`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect13, "ALTER TABLE exist_db.exist_tb_3 ADD index index_name (v1, v2, v3), add index index_name1(v3,v2,v1);", newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2，v3)可调整为(v1，v3，v2)，(v3，v2，v1)可调整为(v1，v3，v2)"))

inspect14 := NewMockInspectWithIsExecutedSQL(e)
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v1`) as cardinality from `exist_db`.`exist_tb_3`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("100"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v2`) as cardinality from `exist_db`.`exist_tb_3`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("60"))
handler.ExpectQuery(regexp.QuoteMeta("select count(distinct `v3`) as cardinality from `exist_db`.`exist_tb_3`")).
WillReturnRows(sqlmock.NewRows([]string{"cardinality"}).AddRow("80"))
runSingleRuleInspectCase(rule, t, "", inspect14, `
CREATE TABLE exist_db.exist_tb_3 (
id bigint unsigned NOT NULL AUTO_INCREMENT COMMENT "unit test",
v1 varchar(255) NOT NULL COMMENT "unit test",
v2 varchar(255) COMMENT "unit test",
v3 int COMMENT "unit test",
Index index_name (v1, v2, v3),
Index index_name1(v3,v2,v1)
)ENGINE=InnoDB AUTO_INCREMENT=3 DEFAULT CHARSET=utf8mb4 COMMENT="uint test";`,
newTestResult().addResult(rulepkg.DDLCheckCompositeIndexDistinction, "(v1，v2，v3)可调整为(v1，v3，v2)，(v3，v2，v1)可调整为(v1，v3，v2)"))
}
109 changes: 109 additions & 0 deletions sqle/driver/mysql/rule/rule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ const (
DDLCheckAllIndexNotNullConstraint = "ddl_check_all_index_not_null_constraint"
DDLCheckColumnNotNULL = "ddl_check_column_not_null"
DDLCheckTableRows = "ddl_check_table_rows"
DDLCheckCompositeIndexDistinction = "ddl_check_composite_index_distinction"
)

// inspector DML rules
Expand Down Expand Up @@ -2181,6 +2182,18 @@ var RuleHandlers = []RuleHandler{
Message: "表行数超过阈值，建议对表进行拆分",
Func: checkTableRows,
},
{
Rule: driverV2.Rule{
Name: DDLCheckCompositeIndexDistinction,
Desc: "建议在组合索引中将区分度高的字段靠前放",
Annotation: "将区分度高的字段靠前放置在组合索引中有助于提高索引的查询性能，因为它能更快地减小数据范围，提高检索效率。",
Level: driverV2.RuleLevelNotice,
Category: RuleTypeDDLConvention,
},
AllowOffline: false,
Message: "建议在组合索引中将区分度高的字段靠前放，%v",
Func: checkCompositeIndexSelectivity,
},
}

func checkFieldNotNUllMustContainDefaultValue(input *RuleHandlerInput) error {
Expand Down Expand Up @@ -6233,3 +6246,99 @@ func checkTableRows(input *RuleHandlerInput) error {
}
return nil
}

func checkCompositeIndexSelectivity(input *RuleHandlerInput) error {
indexSlices := [][]string{}
table := &ast.TableName{}
switch stmt := input.Node.(type) {
case *ast.CreateIndexStmt:
singleIndexSlice := []string{}
if len(stmt.IndexPartSpecifications) == 1 {
return nil
}
for _, indexPart := range stmt.IndexPartSpecifications {
singleIndexSlice = append(singleIndexSlice, indexPart.Column.Name.O)
}
indexSlices = append(indexSlices, singleIndexSlice)
table = stmt.Table
case *ast.AlterTableStmt:
for _, spec := range stmt.Specs {
if spec.Constraint == nil {
continue
}
if spec.Constraint.Tp != ast.ConstraintIndex && spec.Constraint.Tp != ast.ConstraintUniq {
continue
}
singleIndexSlice := []string{}
if len(spec.Constraint.Keys) == 1 {
continue
}
for _, key := range spec.Constraint.Keys {
singleIndexSlice = append(singleIndexSlice, key.Column.Name.O)
}
indexSlices = append(indexSlices, singleIndexSlice)
}
table = stmt.Table
case *ast.CreateTableStmt:
if stmt.Constraints == nil {
return nil
}
for _, con := range stmt.Constraints {
if con.Tp != ast.ConstraintIndex && con.Tp != ast.ConstraintUniq {
continue
}
singleIndexSlice := []string{}
if len(con.Keys) == 1 {
continue
}
for _, key := range con.Keys {
singleIndexSlice = append(singleIndexSlice, key.Column.Name.O)
}
indexSlices = append(indexSlices, singleIndexSlice)
}
table = stmt.Table
}
exist, err := input.Ctx.IsTableExist(table)
if err != nil {
return err
}
if !exist {
return nil
}

noticeInfos := []string{}
for _, singleIndexSlice := range indexSlices {
var indexSelectValueSlice []struct {
Index string
Value int
}
sortIndexes := make([]string, len(singleIndexSlice))
for _, indexColumn := range singleIndexSlice {
columnCardinality, err := input.Ctx.GetColumnCardinality(table, indexColumn)
if err != nil {
return err
}
selectivityValue := columnCardinality
indexSelectValueSlice = append(indexSelectValueSlice, struct {
Index string
Value int
}{indexColumn, selectivityValue})
}
sort.Slice(indexSelectValueSlice, func(i, j int) bool {
return indexSelectValueSlice[i].Value > indexSelectValueSlice[j].Value
})
for i, kv := range indexSelectValueSlice {
sortIndexes[i] = kv.Index
}
for ind, indexColumn := range singleIndexSlice {
if indexColumn != indexSelectValueSlice[ind].Index {
noticeInfos = append(noticeInfos, fmt.Sprintf("(%s)可调整为(%s)", strings.Join(singleIndexSlice, "，"), strings.Join(sortIndexes, "，")))
break
}
}
}
if len(noticeInfos) > 0 {
addResult(input.Res, input.Rule, input.Rule.Name, strings.Join(noticeInfos, "，"))
}
return nil
}
Loading