Skip to content

checker(dm): improve same table precheck error #12729

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

Open
GMHDBJD wants to merge 1 commit into
base: master
Choose a base branch
from
+152 −0
Open

checker(dm): improve same table precheck error #12729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
93 changes: 93 additions & 0 deletions dm/checker/check_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/pingcap/tiflow/dm/ctl/common"
"github.com/pingcap/tiflow/dm/pkg/conn"
"github.com/pingcap/tiflow/dm/pkg/cputil"
"github.com/pingcap/tiflow/dm/unit"
"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -794,6 +795,98 @@ func TestSameTargetTableDetection(t *testing.T) {
require.ErrorContains(t, err, "same table name in case-insensitive")
}

func TestSameTargetTableDetectionBeforeRouteDupMatch(t *testing.T) {
cfgs := []*config.SubTaskConfig{
{
RouteRules: []*router.TableRule{
{
SchemaPattern: "test",
TargetSchema: "test",
TablePattern: "T",
TargetTable: "T",
}, {
SchemaPattern: "test",
TargetSchema: "test",
TablePattern: "t",
TargetTable: "t",
},
},
Mode: config.ModeAll,
IgnoreCheckingItems: ignoreExcept(map[string]struct{}{config.TableSchemaChecking: {}}),
},
}
cfgsWithSameTarget := []*config.SubTaskConfig{
{
RouteRules: []*router.TableRule{
{
SchemaPattern: "test",
TargetSchema: "test",
TablePattern: "T",
TargetTable: "T",
}, {
SchemaPattern: "test",
TargetSchema: "test",
TablePattern: "t",
TargetTable: "T",
},
},
Mode: config.ModeAll,
IgnoreCheckingItems: ignoreExcept(map[string]struct{}{config.TableSchemaChecking: {}}),
},
}

require.NoError(t, sameTargetTableNameDetectionForRouteRules(false, []*router.TableRule{
{
SchemaPattern: "test",
TargetSchema: "test",
TablePattern: "a",
TargetTable: "T",
}, {
SchemaPattern: "test",
TargetSchema: "test",
TablePattern: "b",
TargetTable: "t",
},
}))
require.NoError(t, sameTargetTableNameDetectionForRouteRules(true, cfgs[0].RouteRules))

msg, err := CheckSyncConfig(context.Background(), cfgs, common.DefaultErrorCnt, common.DefaultWarnCnt)
require.ErrorContains(t, err, "fail to initialize checker")
require.ErrorContains(t, err, "same table name in case-insensitive")
require.ErrorContains(t, err, "same target table `test`.`T` vs `test`.`t`")
require.NotContains(t, err.Error(), "matches more than one rule")
require.Len(t, msg, 0)

_, err = RunCheckOnConfigs(context.Background(), cfgs, false, 100, 100)
require.ErrorContains(t, err, "fail to initialize checker")
require.ErrorContains(t, err, "same table name in case-insensitive")
require.ErrorContains(t, err, "same target table `test`.`T` vs `test`.`t`")
require.NotContains(t, err.Error(), "matches more than one rule")

processErr := unit.NewProcessError(err)
require.Contains(t, processErr.Message, "same table name in case-insensitive")
require.Contains(t, processErr.Message, "same target table `test`.`T` vs `test`.`t`")
require.Empty(t, processErr.RawCause)

msg, err = CheckSyncConfig(context.Background(), cfgsWithSameTarget, common.DefaultErrorCnt, common.DefaultWarnCnt)
require.ErrorContains(t, err, "fail to initialize checker")
require.ErrorContains(t, err, "same table name in case-insensitive")
require.ErrorContains(t, err, "same target table `test`.`T` vs `test`.`T`")
require.NotContains(t, err.Error(), "matches more than one rule")
require.Len(t, msg, 0)

_, err = RunCheckOnConfigs(context.Background(), cfgsWithSameTarget, false, 100, 100)
require.ErrorContains(t, err, "fail to initialize checker")
require.ErrorContains(t, err, "same table name in case-insensitive")
require.ErrorContains(t, err, "same target table `test`.`T` vs `test`.`T`")
require.NotContains(t, err.Error(), "matches more than one rule")

processErr = unit.NewProcessError(err)
require.Contains(t, processErr.Message, "same table name in case-insensitive")
require.Contains(t, processErr.Message, "same target table `test`.`T` vs `test`.`T`")
require.Empty(t, processErr.RawCause)
}

func TestMetaPositionChecking(t *testing.T) {
cfgs := []*config.SubTaskConfig{
{
Expand Down
59 changes: 59 additions & 0 deletions dm/checker/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"github.com/pingcap/tidb/pkg/util/dbutil"
"github.com/pingcap/tidb/pkg/util/filter"
regexprrouter "github.com/pingcap/tidb/pkg/util/regexpr-router"
router "github.com/pingcap/tidb/pkg/util/table-router"
"github.com/pingcap/tiflow/dm/config"
"github.com/pingcap/tiflow/dm/config/dbconfig"
"github.com/pingcap/tiflow/dm/loader"
Expand Down Expand Up @@ -566,6 +567,9 @@ func (c *Checker) fetchSourceTargetDB(
return nil, nil, terror.ErrTaskCheckGenBAList.Delegate(err)
}
instance.baList = bAList
if err := sameTargetTableNameDetectionForRouteRules(instance.cfg.CaseSensitive, instance.cfg.RouteRules); err != nil {
return nil, nil, err
}
r, err := regexprrouter.NewRegExprRouter(instance.cfg.CaseSensitive, instance.cfg.RouteRules)
if err != nil {
return nil, nil, terror.ErrTaskCheckGenTableRouter.Delegate(err)
Expand Down Expand Up @@ -876,6 +880,61 @@ func sameTableNameDetection(tables map[filter.Table][]filter.Table) error {
return nil
}

func sameTargetTableNameDetectionForRouteRules(caseSensitive bool, rules []*router.TableRule) error {
if caseSensitive {
return nil
}

// regexprrouter matches source patterns case-insensitively in this mode and
// reports "matches more than one rule" before the checker can build target
// table mappings. Detect the same-target-table conflict from the route
// rules first so the precheck reports the actionable table names.
sourceTableToTargetNameSets := make(map[string]map[string]string)
var messages []string
for _, rule := range rules {
if rule == nil || rule.TablePattern == "" || rule.TargetSchema == "" {
continue
}

targetTable := rule.TargetTable
if targetTable == "" {
targetTable = rule.TablePattern
}
if targetTable == "" {
continue
}

source := filter.Table{
Schema: rule.SchemaPattern,
Name: rule.TablePattern,
}
sourceNameL := strings.ToLower(source.String())
tableNameSets := sourceTableToTargetNameSets[sourceNameL]
if tableNameSets == nil {
tableNameSets = make(map[string]string)
sourceTableToTargetNameSets[sourceNameL] = tableNameSets
}

target := filter.Table{
Schema: rule.TargetSchema,
Name: targetTable,
}
name := target.String()
nameL := strings.ToLower(name)
if nameO, ok := tableNameSets[nameL]; !ok {
tableNameSets[nameL] = name
} else {
messages = append(messages, fmt.Sprintf("same target table %v vs %s", nameO, name))
}
Comment on lines +924 to +928

@gemini-code-assist gemini-code-assist Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If two route rules collide under case-insensitive matching but map to the exact same target table name (including case), nameO == name will be true. Because of the else if nameO != name condition, this collision will not be detected by this preflight check, and the user will still encounter the confusing matches more than one rule error from regexprrouter.

Changing this to a simple else ensures that any duplicate or colliding rules for the same source table pattern are caught and reported with a clear error message.

Suggested change
if nameO, ok := tableNameSets[nameL]; !ok {
tableNameSets[nameL] = name
} else if nameO != name {
messages = append(messages, fmt.Sprintf("same target table %v vs %s", nameO, name))
}
if nameO, ok := tableNameSets[nameL]; !ok {
tableNameSets[nameL] = name
} else {
messages = append(messages, fmt.Sprintf("same target table %v vs %s", nameO, name))
}

@GMHDBJD GMHDBJD Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, this is reasonable. Updated the preflight to catch any case-insensitive source-rule collision, including collisions that map to the exact same target table name, and added regression coverage for that case.

}

if len(messages) > 0 {
return terror.ErrTaskCheckSameTableName.Generate(messages)
}

return nil
}

// lightningPrecheckAdaptor implements the importer.PreRestoreInfoGetter interface.
type lightningPrecheckAdaptor struct {
importer.TargetInfoGetter
Expand Down
Loading