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

FK constraint might not enforced when child table is in state write_only #55813

Closed
D3Hunter opened this issue Sep 3, 2024 · 3 comments · Fixed by #55891
Closed

FK constraint might not enforced when child table is in state write_only #55813

D3Hunter opened this issue Sep 3, 2024 · 3 comments · Fixed by #55891
Assignees
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. severity/major sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug.

Comments

@D3Hunter
Copy link
Contributor

D3Hunter commented Sep 3, 2024

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

use the case in the design doc:

```sql
-- In TiDB-1 and Schema Version is 1
insert into t_has_foreign_key values (1, 1);
-- In TiDB-0 and Schema Version is 0
delete from t_reference where id = 1; --Since doesn't know foreign key information in old version, so doesn't do foreign key constrain check.
```
So, when creating a table with foreign key, we need multi-schema version change:
1. None -> Write Only: Create table with the state is `write-only`.
2. Write Only -> Public: Update the created table state to `public`.

steps:

  1. apply this diff to code of v8.1.0, after that we will wait when create child table and it's in write-only state, and on 4001 node, we skip load schema diff of child table, so we can make a case that 4000 and 4001 have different version
diff --git a/pkg/ddl/ddl_worker.go b/pkg/ddl/ddl_worker.go
index 916458f1d0..1dc34f79f0 100644
--- a/pkg/ddl/ddl_worker.go
+++ b/pkg/ddl/ddl_worker.go
@@ -1430,6 +1430,9 @@ func waitSchemaChanged(d *ddlCtx, waitTime time.Duration, latestSchemaVersion in
                         return nil
                 }
         }
+        if job.TableName == "child" {
+                time.Sleep(time.Hour)
+        }

         return checkAllVersions(d, job, latestSchemaVersion, timeStart)
 }
diff --git a/pkg/infoschema/builder.go b/pkg/infoschema/builder.go
index d03b07d7d1..62347c888e 100644
--- a/pkg/infoschema/builder.go
+++ b/pkg/infoschema/builder.go
@@ -659,6 +659,9 @@ func applyCreateTable(b *Builder, m *meta.Meta, dbInfo *model.DBInfo, tableID in
                         fmt.Sprintf("(Table ID %d)", tableID),
                 )
         }
+        if tblInfo.Name.L == "child" && config.GetGlobalConfig().Port == 4001 {
+                return []int64{}, nil
+        }

         b.updateBundleForCreateTable(tblInfo, tp)
  1. start 2 tidb on port 4000 and 4001
  2. connect 4000, and run below, it will block on create child, check admin show ddl jobs 1 to make sure child is in write_only state.
CREATE TABLE parent (
    id INT KEY
);

insert into parent values(1)

CREATE TABLE child (
    id INT,
    pid INT,
    INDEX idx_pid (pid),
    FOREIGN KEY (pid) REFERENCES parent(id) ON DELETE CASCADE
);
  1. start 2 sessions, and run the case in the design doc
connect 4000 session 4001 session
begin
insert into child values(1,1)
begin
delete from parent where id=1; // it blocks until prev txn commit
commit
commit
select * from child; // we can see 1 row
select * from parent;// empty result

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

FK constraint enforced

3. What did you see instead (Required)

it's not

4. What is your TiDB version? (Required)

8.1.0

@D3Hunter D3Hunter added type/bug The issue is confirmed as a bug. affects-8.1 This bug affects the 8.1.x(LTS) versions. labels Sep 3, 2024
@YangKeao YangKeao self-assigned this Sep 4, 2024
@YangKeao YangKeao mentioned this issue Sep 5, 2024
17 tasks
@YangKeao
Copy link
Member

YangKeao commented Sep 5, 2024

Maybe we should add an extra WriteReorg phase to check the FK constraint before finishing the CREATE TABLE. Though, it's not intuitive for a CREATE TABLE to return an error 🤔 .

@D3Hunter
Copy link
Contributor Author

D3Hunter commented Sep 5, 2024

make new created table in write-only state invisible should do the work

@YangKeao
Copy link
Member

YangKeao commented Sep 5, 2024

make new created table in write-only state invisible should do the work

To make it clear. For the write-only state:

  1. The table is not visible for any explicit DML (or any query? Maybe only DML is enough 🤔, but make it always invisible is clearer).
  2. The table will be modified by implicit modification caused by ON DELETE CASCADE or ON UPDATE CASCADE

Then it should be fine. (Let me think twice).

@YangKeao YangKeao added affects-6.1 This bug affects the 6.1.x(LTS) versions. and removed may-affects-5.4 This bug maybe affects 5.4.x versions. labels Sep 9, 2024
@YangKeao YangKeao added may-affects-6.1 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. labels Sep 9, 2024
@YangKeao YangKeao added may-affects-6.5 may-affects-7.1 affects-7.5 This bug affects the 7.5.x(LTS) versions. labels Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.5 This bug affects the 7.5.x(LTS) versions. affects-8.1 This bug affects the 8.1.x(LTS) versions. severity/major 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.

3 participants