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.

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

TiDB can insert duplicate key by insert ingore statements during modify column #50993

Closed
wjhuang2016 opened this issue Feb 5, 2024 · 4 comments · Fixed by #40198
Closed

TiDB can insert duplicate key by insert ingore statements during modify column #50993

wjhuang2016 opened this issue Feb 5, 2024 · 4 comments · Fixed by #40198
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. severity/critical sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@wjhuang2016
Copy link
Member

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

modify code:

 var TestReorgGoroutineRunning = make(chan interface{})

+var MockDMLExec func()
+
 // updateCurrentElement update the current element for reorgInfo.
 func (w *worker) updateCurrentElement(t table.Table, reorgInfo *reorgInfo) error {
        failpoint.Inject("mockInfiniteReorgLogic", func(val failpoint.Value) {
@@ -1146,6 +1148,9 @@ func (w *worker) updateCurrentElement(t table.Table, reorgInfo *reorgInfo) error
                if err != nil {
                        return errors.Trace(err)
                }
+               if MockDMLExec != nil {
+                       MockDMLExec()
+               }
                err = w.addTableIndex(t, reorgInfo)

Then tests:

func TestIssueInsertIngore(t *testing.T) {
	store, _ := testkit.CreateMockStoreAndDomainWithSchemaLease(t, dbTestLease)

	tk := testkit.NewTestKit(t, store)
	tk.MustExec("use test")
	tk1 := testkit.NewTestKit(t, store)
	tk1.MustExec("use test")

	tk.MustExec("drop table if exists t")
	tk.MustExec("create table t (a double, b int, c int, unique key (a));")

	j := 0
	var cerr error
	ddl.MockDMLExec = func() {
		j++
		logutil.BgLogger().Info("mock dml", zap.Int("j", j))
		if j == 1 {
			_, cerr = tk1.Exec("insert ignore into t values (200, 1, 1), (300, 1, 1), (400, 1, 1);")
		}
	}

	done := make(chan error, 1)
	// test transaction on add column.
	go backgroundExec(store, "alter table t modify column a tinyint;", done)
	err := <-done
	require.NoError(t, err)
	require.NoError(t, cerr)
	tk.MustExec("admin check table t")
}

2. What did you expect to see? (Required)

test pass

3. What did you see instead (Required)

test failed with

Received unexpected error:
        	            	[admin:8223]data inconsistency in table: t, index: a, handle: 1, index-values:"" != record-values:"handle: 1, values: [KindInt64 127]"
        	            	github.com/pingcap/errors.AddStack

4. What is your TiDB version? (Required)

v6.5.8

@wjhuang2016 wjhuang2016 added the type/bug The issue is confirmed as a bug. label Feb 5, 2024
@tangenta
Copy link
Contributor

Fixed by #40198, we need to cherry-pick the fix to v6.5.

@tangenta tangenta added affects-6.5 This bug affects the 6.5.x(LTS) versions. and removed may-affects-5.4 This bug maybe affects 5.4.x versions. may-affects-6.1 may-affects-6.5 may-affects-7.1 may-affects-7.5 labels Feb 19, 2024
@cfzjywxk
Copy link
Contributor

@tangenta
Can this issue be closed or it need more fixes?

@jebter jebter added component/ddl This issue is related to DDL of TiDB. sig/sql-infra SIG: SQL Infra and removed sig/transaction SIG:Transaction component/ddl This issue is related to DDL of TiDB. labels Mar 19, 2024
@jebter
Copy link

jebter commented Mar 19, 2024

@YangKeao Can this issue be closed or it need more fixes?

@YangKeao
Copy link
Member

Yes. Close it now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.5 This bug affects the 6.5.x(LTS) versions. severity/critical sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants