diff --git a/go.mod b/go.mod index 965c6f6f28a..d5456c29942 100644 --- a/go.mod +++ b/go.mod @@ -70,7 +70,7 @@ require ( github.com/pingcap/log v1.1.1-0.20230317032135-a0d097d16e22 github.com/pingcap/tidb v1.1.0-beta.0.20240228123331-27ce02afd2e3 github.com/pingcap/tidb-dashboard v0.0.0-20240127080020-2171c6e6b9d7 - github.com/pingcap/tidb-tools v0.0.0-20240202030925-a6014db89eb8 + github.com/pingcap/tidb-tools v0.0.0-20240305021104-9f9bea84490b github.com/pingcap/tidb/pkg/parser v0.0.0-20240228123331-27ce02afd2e3 github.com/prometheus/client_golang v1.18.0 github.com/prometheus/client_model v0.6.0 diff --git a/go.sum b/go.sum index cf68b4f6e5f..338079ac17f 100644 --- a/go.sum +++ b/go.sum @@ -877,8 +877,8 @@ github.com/pingcap/tidb v1.1.0-beta.0.20240228123331-27ce02afd2e3 h1:vLUU7S6yhHe github.com/pingcap/tidb v1.1.0-beta.0.20240228123331-27ce02afd2e3/go.mod h1:pwZI+N72Xb+2KsEragMzRmb4EEUFSbrOS0mMcaHMFCM= github.com/pingcap/tidb-dashboard v0.0.0-20240127080020-2171c6e6b9d7 h1:M+kqgNnOnzjRb5NubF7QFhyDeQKYcgtLHB12gzYPoUk= github.com/pingcap/tidb-dashboard v0.0.0-20240127080020-2171c6e6b9d7/go.mod h1:ucZBRz52icb23T/5Z4CsuUHmarYiin7p2MeiVBe+o8c= -github.com/pingcap/tidb-tools v0.0.0-20240202030925-a6014db89eb8 h1:oJVvhhIpYrBxiIPUC2M9mXZw4M7EdX8DBiac35YCXzs= -github.com/pingcap/tidb-tools v0.0.0-20240202030925-a6014db89eb8/go.mod h1:FupVn04P2/dkvzK4Tt5URj5z2Y4ILcULw7dU3GnJXgQ= +github.com/pingcap/tidb-tools v0.0.0-20240305021104-9f9bea84490b h1:UC4lLT2OBHGImFJbiXQfIxIck7AoFLIiRJ6Eo6uFVaw= +github.com/pingcap/tidb-tools v0.0.0-20240305021104-9f9bea84490b/go.mod h1:FupVn04P2/dkvzK4Tt5URj5z2Y4ILcULw7dU3GnJXgQ= github.com/pingcap/tidb/pkg/parser v0.0.0-20240228123331-27ce02afd2e3 h1:YF17teH7c/WKeIc6MO8gEKiNe25X36nKxxtPPwO+/O0= github.com/pingcap/tidb/pkg/parser v0.0.0-20240228123331-27ce02afd2e3/go.mod h1:MWQK6otJgZRI6zcCVPV22U4qE26qOGJnN4fq8XawgBs= github.com/pingcap/tipb v0.0.0-20240116032918-9bb28c43bbfc h1:sEp4lbExDfnMX8HXQyhZrhqo2/SgeFY5KOdo5akc8FM= diff --git a/pkg/filter/sql_event_filter.go b/pkg/filter/sql_event_filter.go index d9c1bf79469..0cea3622870 100644 --- a/pkg/filter/sql_event_filter.go +++ b/pkg/filter/sql_event_filter.go @@ -132,7 +132,7 @@ func (f *sqlEventFilter) getRules(schema, table string) []*sqlEventRule { } // skipDDLEvent skips ddl event by its type and query. -func (f *sqlEventFilter) shouldSkipDDL(ddl *model.DDLEvent) (bool, error) { +func (f *sqlEventFilter) shouldSkipDDL(ddl *model.DDLEvent) (skip bool, err error) { if len(f.rules) == 0 { return false, nil } diff --git a/pkg/filter/sql_event_filter_test.go b/pkg/filter/sql_event_filter_test.go index bbf4acd5b0b..d2331033431 100644 --- a/pkg/filter/sql_event_filter_test.go +++ b/pkg/filter/sql_event_filter_test.go @@ -41,167 +41,253 @@ func TestShouldSkipDDL(t *testing.T) { err error } - testCases := []testCase{ - { - cfg: &config.FilterConfig{ - EventFilters: []*config.EventFilterRule{ - { - Matcher: []string{"test.t1"}, - IgnoreEvent: []bf.EventType{bf.AllDDL}, - }, - }, - }, - cases: []innerCase{ - { - schema: "test", - table: "t1", - query: "alter table t1 modify column age int", - ddlType: timodel.ActionModifyColumn, - skip: true, - }, - { - schema: "test", - table: "t1", - query: "create table t1(id int primary key)", - ddlType: timodel.ActionCreateTable, - skip: true, - }, + // filter all ddl + case1 := testCase{ + cfg: &config.FilterConfig{ + EventFilters: []*config.EventFilterRule{ { - schema: "test", - table: "t2", // table name not match - query: "alter table t2 modify column age int", - ddlType: timodel.ActionModifyColumn, - skip: false, - }, - { - schema: "test2", // schema name not match - table: "t1", - query: "alter table t1 modify column age int", - ddlType: timodel.ActionModifyColumn, - skip: false, + Matcher: []string{"test.t1"}, + IgnoreEvent: []bf.EventType{bf.AllDDL}, }, }, }, - { - cfg: &config.FilterConfig{ - EventFilters: []*config.EventFilterRule{ - { - Matcher: []string{"*.t1"}, - IgnoreEvent: []bf.EventType{bf.DropDatabase, bf.DropSchema}, - }, - }, + cases: []innerCase{ + { + schema: "test", + table: "t1", + query: "alter table t1 modify column age int", + ddlType: timodel.ActionModifyColumn, + skip: true, }, - cases: []innerCase{ - { - schema: "test", - table: "t1", - query: "alter table t1 modify column age int", - ddlType: timodel.ActionModifyColumn, - skip: false, - }, - { - schema: "test", - table: "t1", - query: "alter table t1 drop column age", - ddlType: timodel.ActionDropColumn, - skip: false, - }, - { - schema: "test2", - table: "t1", - query: "drop database test2", - ddlType: timodel.ActionDropSchema, - skip: true, + { + schema: "test", + table: "t1", + query: "create table t1(id int primary key)", + ddlType: timodel.ActionCreateTable, + skip: true, + }, + { + schema: "test", + table: "t2", // table name not match + query: "alter table t2 modify column age int", + ddlType: timodel.ActionModifyColumn, + skip: false, + }, + { + schema: "test2", // schema name not match + table: "t1", + query: "alter table t1 modify column age int", + ddlType: timodel.ActionModifyColumn, + skip: false, + }, + }, + } + f, err := newSQLEventFilter(case1.cfg) + require.True(t, errors.ErrorEqual(err, case1.err), "case: %+s", err) + for _, c := range case1.cases { + ddl := &model.DDLEvent{ + TableInfo: &model.TableInfo{ + TableName: model.TableName{ + Schema: c.schema, + Table: c.table, }, + }, + Query: c.query, + Type: c.ddlType, + } + skip, err := f.shouldSkipDDL(ddl) + require.NoError(t, err) + require.Equal(t, c.skip, skip, "case: %+v", c) + } + + // filter some ddl + case2 := testCase{ + cfg: &config.FilterConfig{ + EventFilters: []*config.EventFilterRule{ { - schema: "test3", - table: "t1", - query: "drop index i3 on t1", - ddlType: timodel.ActionDropIndex, - skip: false, + Matcher: []string{"*.t1"}, + IgnoreEvent: []bf.EventType{bf.DropDatabase, bf.DropSchema}, }, }, }, - { - cfg: &config.FilterConfig{ - EventFilters: []*config.EventFilterRule{ - { - Matcher: []string{"*.t1"}, - IgnoreSQL: []string{"MODIFY COLUMN", "DROP COLUMN", "^DROP DATABASE"}, - }, - }, + cases: []innerCase{ + { + schema: "test", + table: "t1", + query: "alter table t1 modify column age int", + ddlType: timodel.ActionModifyColumn, + skip: false, }, - cases: []innerCase{ - { - schema: "test", - table: "t1", - query: "ALTER TABLE t1 MODIFY COLUMN age int(11) NOT NULL", - ddlType: timodel.ActionModifyColumn, - skip: true, + { + schema: "test", + table: "t1", + query: "alter table t1 drop column age", + ddlType: timodel.ActionDropColumn, + skip: false, + }, + { + schema: "test2", + table: "t1", + query: "drop database test2", + ddlType: timodel.ActionDropSchema, + skip: true, + }, + { + schema: "test3", + table: "t1", + query: "drop index i3 on t1", + ddlType: timodel.ActionDropIndex, + skip: false, + }, + }, + } + f, err = newSQLEventFilter(case2.cfg) + require.True(t, errors.ErrorEqual(err, case2.err), "case: %+s", err) + for _, c := range case2.cases { + ddl := &model.DDLEvent{ + TableInfo: &model.TableInfo{ + TableName: model.TableName{ + Schema: c.schema, + Table: c.table, }, + }, + Query: c.query, + Type: c.ddlType, + } + skip, err := f.shouldSkipDDL(ddl) + require.NoError(t, err) + require.Equal(t, c.skip, skip, "case: %+v", c) + } + + // filter ddl by IgnoreSQL + case3 := testCase{ + cfg: &config.FilterConfig{ + EventFilters: []*config.EventFilterRule{ { - schema: "test", - table: "t1", - query: "ALTER TABLE t1 DROP COLUMN age", - ddlType: timodel.ActionDropColumn, - skip: true, + Matcher: []string{"*.t1"}, + IgnoreSQL: []string{"MODIFY COLUMN", "DROP COLUMN", "^DROP DATABASE"}, }, - { // no table name - schema: "test2", - query: "DROP DATABASE test", - ddlType: timodel.ActionDropSchema, - skip: true, + }, + }, + cases: []innerCase{ + { + schema: "test", + table: "t1", + query: "ALTER TABLE t1 MODIFY COLUMN age int(11) NOT NULL", + ddlType: timodel.ActionModifyColumn, + skip: true, + }, + { + schema: "test", + table: "t1", + query: "ALTER TABLE t1 DROP COLUMN age", + ddlType: timodel.ActionDropColumn, + skip: true, + }, + { // no table name + schema: "test2", + query: "DROP DATABASE test", + ddlType: timodel.ActionDropSchema, + skip: true, + }, + { + schema: "test3", + table: "t1", + query: "Drop Index i1 on test3.t1", + ddlType: timodel.ActionDropIndex, + skip: false, + }, + }, + } + f, err = newSQLEventFilter(case3.cfg) + require.True(t, errors.ErrorEqual(err, case3.err), "case: %+s", err) + for _, c := range case3.cases { + ddl := &model.DDLEvent{ + TableInfo: &model.TableInfo{ + TableName: model.TableName{ + Schema: c.schema, + Table: c.table, }, + }, + Query: c.query, + Type: c.ddlType, + } + skip, err := f.shouldSkipDDL(ddl) + require.NoError(t, err) + require.Equal(t, c.skip, skip, "case: %+v", c) + } + + // config error + case4 := testCase{ + cfg: &config.FilterConfig{ + EventFilters: []*config.EventFilterRule{ { - schema: "test3", - table: "t1", - query: "Drop Index i1 on test3.t1", - ddlType: timodel.ActionDropIndex, - skip: false, + Matcher: []string{"*.t1"}, + IgnoreEvent: []bf.EventType{bf.EventType("aa")}, }, }, }, - { // config error - cfg: &config.FilterConfig{ - EventFilters: []*config.EventFilterRule{ - { - Matcher: []string{"*.t1"}, - IgnoreEvent: []bf.EventType{bf.EventType("aa")}, - }, + err: cerror.ErrInvalidIgnoreEventType, + } + _, err = newSQLEventFilter(case4.cfg) + require.True(t, errors.ErrorEqual(err, case4.err), "case: %+s", err) + + // config error + case5 := testCase{ + cfg: &config.FilterConfig{ + EventFilters: []*config.EventFilterRule{ + { + Matcher: []string{"*.t1"}, + IgnoreSQL: []string{"--6"}, // this is a valid regx }, }, - err: cerror.ErrInvalidIgnoreEventType, }, - { - cfg: &config.FilterConfig{ - EventFilters: []*config.EventFilterRule{ - { - Matcher: []string{"*.t1"}, - IgnoreSQL: []string{"--6"}, // this is a valid regx - }, + } + _, err = newSQLEventFilter(case5.cfg) + require.True(t, errors.ErrorEqual(err, case5.err), "case: %+s", err) + + // cover all ddl event types + allEventTypes := make([]bf.EventType, 0, len(ddlWhiteListMap)) + for _, et := range ddlWhiteListMap { + allEventTypes = append(allEventTypes, et) + } + innerCases := make([]innerCase, 0, len(ddlWhiteListMap)) + for at := range ddlWhiteListMap { + innerCases = append(innerCases, innerCase{ + schema: "test", + table: "t1", + query: "no matter", + ddlType: at, + skip: true, + }) + } + case6 := testCase{ + cfg: &config.FilterConfig{ + EventFilters: []*config.EventFilterRule{ + { + Matcher: []string{"*.t1"}, + IgnoreEvent: allEventTypes, }, }, }, + cases: innerCases, } - - for _, tc := range testCases { - f, err := newSQLEventFilter(tc.cfg) - require.True(t, errors.ErrorEqual(err, tc.err), "case: %+s", err) - for _, c := range tc.cases { - ddl := &model.DDLEvent{ - TableInfo: &model.TableInfo{ - TableName: model.TableName{ - Schema: c.schema, - Table: c.table, - }, + f, err = newSQLEventFilter(case6.cfg) + require.NoError(t, err) + for _, c := range case6.cases { + ddl := &model.DDLEvent{ + TableInfo: &model.TableInfo{ + TableName: model.TableName{ + Schema: c.schema, + Table: c.table, }, - Query: c.query, - Type: c.ddlType, - } - skip, err := f.shouldSkipDDL(ddl) - require.NoError(t, err) - require.Equal(t, c.skip, skip, "case: %+v", c) + }, + Query: c.query, + Type: c.ddlType, } + skip, err := f.shouldSkipDDL(ddl) + require.NoError(t, err) + require.Equal(t, c.skip, skip, "case: %+v", c) } }