MySQL Bugs: #116502: Duplicate Detection Flaw for NULL Values in UK During DDL Rollback Operations

Bug #116502	Duplicate Detection Flaw for NULL Values in UK During DDL Rollback Operations
Submitted:	30 Oct 2024 4:31	Modified:	31 Oct 2024 7:12
Reporter:	Yichang SONG (OCA)	Email Updates:
Status:	Verified	Impact on me:	None
Category:	MySQL Server: InnoDB storage engine	Severity:	S2 (Serious)
Version:	8.0.40	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	Contribution

Description:
Scenario Description:
Create table t1 (id int primary key, k1 int, k2 int);
INSERT INTO t1 values (1,10,null);

Session 1                    Session 2                              Session 3
ALTER t1 ADD uk(k1,k2);
  Build b-tree               BEGIN;
  Build b-tree.              DELETE id=1;
  row_log_apply
  (before ddl done)                                                 INSERT (3,10,null);
  (before ddl done)          ROLLBACK;
  ddl done

Detailed Explanation of Key Points:

1. Session 2’s DELETE Operation:
While Session 1 is building the B-tree for the unique key (k1, k2), the DELETE statement in Session 2 does not immediately modify the unique key's B-tree.
Instead, the DELETE operation records a row log indicating the intent to delete id = 1. This log is queued for application once the B-tree construction reaches a certain phase.

2. Applying Row Logs During DDL:
As the DDL operation in Session 1 progresses, it begins applying the queued row logs.
Specifically, the row log for DELETE id = 1 is processed, resulting in the removal of the corresponding leaf node from the unique key's B-tree.

3. Transition Between Row Log Application and DDL Completion:
After the row logs have been applied but before the DDL operation is fully completed, any subsequent DML operations bypass the row logging mechanism.
Instead, these DML operations interact directly with the now partially constructed unique key B-tree.

4. Session 3’s INSERT Operation:
The INSERT statement executed by Session 3 adds a new leaf node {k1=10, k2=NULL, id=3} to the unique key's B-tree.

5. Session 2’s ROLLBACK Operation:
Upon rolling back, Session 2 attempts to undo the DELETE operation by re-inserting the original row {k1=10, k2=NULL, id=1}.
Since the unique key includes a NULL value in k2, standard SQL semantics dictate that this should not constitute a duplicate entry.
However, the current implementation erroneously identifies the re-inserted row as a duplicate of the already inserted {k1=10, k2=NULL, id=3} from Session 3.
This false duplicate detection causes the DDL operation to fail because the system incorrectly assumes a uniqueness constraint violation, even though NULL values should permit such entries.

This scenario highlights a flaw in the current implementation where NULL values in unique keys are not correctly exempted from duplicate detection during rollback operations in the context of ongoing DDL changes.

How to repeat:
I have developed a test case and applied a corresponding bug fix. Kindly review the attached file below.

Suggested fix:
The bugfix is similar with the logic in row_log.cc

diff --git a/storage/innobase/row/row0umod.cc b/storage/innobase/row/row0umod.cc
index 98d9009c912..e895bf9f75e 100644
--- a/storage/innobase/row/row0umod.cc
+++ b/storage/innobase/row/row0umod.cc
@@ -679,8 +679,9 @@ try_again:
             << " at: " << rec_index_print(btr_cur_get_rec(btr_cur), index);
       }
 
-      if (btr_cur->up_match >= dict_index_get_n_unique(index) ||
-          btr_cur->low_match >= dict_index_get_n_unique(index)) {
+      if ((btr_cur->up_match >= dict_index_get_n_unique(index) ||
+           btr_cur->low_match >= dict_index_get_n_unique(index)) &&
+          (!index->n_nullable || !dtuple_contains_null(entry))) {
         if (index->is_committed()) {
           ib::warn(ER_IB_MSG_1040) << "Record in index " << index->name
                                    << " was not found on rollback, and"

testcase and bugfix

Attachment: changes.patch (application/octet-stream, text), 3.86 KiB.

Hello Yichang Song,

Thank you for the report and test case.

regards,
Umesh

Thank you very much for your patch contribution, we appreciate it!

In order for us to continue the process of reviewing your contribution to MySQL, please send us a signed copy of the Oracle Contributor Agreement (OCA) as outlined in https://oca.opensource.oracle.com

Signing an OCA needs to be done only once and it's valid for all other Oracle governed Open Source projects as well.

Getting a signed/approved OCA on file will help us facilitate your contribution - this one, and others in the future.  

Please let me know, if you have any questions.

Thank you for your interest in MySQL.

Hi, all:

It may be a better solution to fix the bug:

diff --git a/storage/innobase/row/row0umod.cc b/storage/innobase/row/row0umod.cc
index e44d60cb0b7..eadeea2682f 100644
--- a/storage/innobase/row/row0umod.cc
+++ b/storage/innobase/row/row0umod.cc
@@ -668,8 +668,9 @@ try_again:
             << " at: " << rec_index_print(btr_cur_get_rec(btr_cur), index);
       }

-      if (btr_cur->up_match >= dict_index_get_n_unique(index) ||
-          btr_cur->low_match >= dict_index_get_n_unique(index)) {
+      if ((btr_cur->up_match >= dict_index_get_n_unique(index) ||
+          btr_cur->low_match >= dict_index_get_n_unique(index)) &&
+          (!index->n_nullable || index->nulls_equal || !dtuple_contains_null(entry))) {
         if (index->is_committed()) {
           ib::warn(ER_IB_MSG_1040) << "Record in index " << index->name
                                    << " was not found on rollback, and"

Testcase and bugfix (*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: changes.patch (application/octet-stream, text), 3.86 KiB.