Expression filter: UPDATE filter always return "TRUE" when there are INSERT/DELETE filters for the same table

[kennytm]

What did you do?

0. create and populate the table...

create table db.tbl (p bigint primary key, v int);
insert into db.tbl values (1, 1), (3, 3);

1. prepare a DM task with the following filters

expression-filter:
  e11:
    schema: db
    table: tbl
    insert-value-expr: v = 1
  e12:
    schema: db
    table: tbl
    update-old-value-expr: v = 1
    update-new-value-expr: v = 1

mysql-instances:
- expression-filters: ["e11", "e12"]
  ....

2. Perform an UPDATE

update db.tbl set v = 2 where v = 3;

What did you expect to see?

The UPDATE statement should be synchronized since it hits none of the filters

select * from db.tbl;

p	v
1	1
3	2

What did you see instead?

The UPDATE statement is skipped

p	v
1	1
3	3

(The log indicates the expression "1" is evaluated inside SkipDMLByExpression)

Versions of the cluster

v6.4.0

current status of DM cluster (execute query-status <task-name> in dmctl)

No response

[kennytm]

tiflow/dm/syncer/expr_filter_group.go

Lines 115 to 139 in 387b81a

	if _, ok := g.hasUpdateFilter[tableID]; ok {
	for _, c := range g.configs[tableID] {
	if c.UpdateOldValueExpr != "" {
	expr, err := getSimpleExprOfTable(g.tidbCtx, c.UpdateOldValueExpr, ti, g.logCtx.L())
	if err != nil {
	// TODO: terror
	return nil, nil, err
	}
	g.updateOldExprs[tableID] = append(g.updateOldExprs[tableID], expr)
	} else {
	g.updateOldExprs[tableID] = append(g.updateOldExprs[tableID], expression.NewOne())
	}
	if c.UpdateNewValueExpr != "" {
	expr, err := getSimpleExprOfTable(g.tidbCtx, c.UpdateNewValueExpr, ti, g.logCtx.L())
	if err != nil {
	// TODO: terror
	return nil, nil, err
	}
	g.updateNewExprs[tableID] = append(g.updateNewExprs[tableID], expr)
	} else {
	g.updateNewExprs[tableID] = append(g.updateNewExprs[tableID], expression.NewOne())
	}
	}
	}

When a table db.tbl contains both UPDATE and non-UPDATE filters, we will find g.hasUpdateFilter[tableID] == true but some of the g.configs[tableID] contains rules that both c.UpdateOldValueExpr and c.UpdateNewValueExpr are empty.

The current algorithm will push "1", "1" onto the result which caused everything to be skipped.

The correct way is to not push anything when both c.UpdateOldValueExpr and c.UpdateNewValueExpr are empty, just like the correct implementation for GetInsertExprs and GetDeleteExprs.