bug: update ignore return incorrect result set

[davidshiz]

Have you read the Contributing Guidelines on issues?

• I have read the Contributing Guidelines on issues.

Please confirm if bug report does NOT exists already ?

• I confirm there is no existing issue for this

Describe the problem

mysql> CREATE TABLE t1  (id int(11) NOT NULL auto_increment,  parent_id int(11) DEFAULT '0' NOT NULL,  level tinyint(4)
DEFAULT '0' NOT NULL, PRIMARY KEY (id));
Query OK, 0 rows affected (0.02 sec)

mysql> INSERT INTO t1 VALUES (1,0,0),(3,1,1),(4,1,1),(8,2,2),(9,2,2),(17,3,2),(22,4,2),(24,4,2),(28,5,2),(29,5,2),(30,5,2),(31,6,2),(32,6,2),(33,6,2),(203,7,2),(202,7,2),(20,3,2),(157,0,0);
Query OK, 18 rows affected (0.01 sec)
Records: 18  Duplicates: 0  Warnings: 0

mysql> update ignore t1 set id=id+1;
Query OK, 11 rows affected, 7 warnings (0.43 sec)
Rows matched: 18  Changed: 11  Warnings: 7

mysql> select * from t1 where id = 203;
+-----+-----------+-------+
| id  | parent_id | level |
+-----+-----------+-------+
| 203 |         7 |     2 |
+-----+-----------+-------+
1 row in set (0.00 sec)

Expected behavior

mysql> select * from t1 where id = 203;
Empty set (0.00 sec)

How To Reproduce

No response

Environment

root@ub01:/stonedb57/install/bin# ./mysqld --version
./mysqld  Ver 5.7.36-StoneDB-v1.0.3.31919be01 for Linux on x86_64 (build-)
build information as follow:
        Repository address: https://github.com/stoneatom/stonedb.git:stonedb-5.7-dev
        Branch name: stonedb-5.7-dev
        Last commit ID: 31919be01
        Last commit time: Date:   Thu Apr 20 10:19:54 2023 +0800
        Build time: Date: Sun Apr 23 17:43:35 CST 2023

Are you interested in submitting a PR to solve the problem?

• Yes, I will!

[RingsC]

[UPDATE]: With IGNORE, rows for which duplicate-key conflicts occur on a unique key value are not updated. Rows updated to values that would cause data conversion errors are updated to the closest valid values instead.

When the option ignore is set, the mechanism of update between innodb and tianm is totally different.

In tianmu, when we update a row, it does not do uniqueness check. therefore, the new data can be inserted without constraint check.

  if (tianmu_sysvar_insert_delayed && table->s->tmp_table == NO_TMP_TABLE && tianmu_sysvar_enable_rowstore) {
    UpdateToDelta(table_path, share, table, row_id, old_data, new_data);
    tianmu_stat.delta_update++;
  } else {
    auto tm_table = current_txn_->GetTableByPath(table_path);
    ret = tm_table->Update(table, row_id, old_data, new_data);
  }

In int TianmuTable::Update(TABLE *table, uint64_t row_id, const uchar *old_data, uchar *new_data) function, line 1041, gets the new value via old one, then inserts into table. From here, there's not constraint check here.

   1034            if (field->real_maybe_null()) {                                                                                                                                                       │
│   1035              if (field->is_null_in_record(old_data) && field->is_null_in_record(new_data)) {                                                                                                     │
│   1036                continue;                                                                                                                                                                         │
│   1037              }                                                                                                                                                                                   │
│   1038                                                                                                                                                                                                  │
│   1039              if (field->is_null_in_record(new_data)) {                                                                                                                                           │
│   1040                core::Value old_v, new_v;                                                                                                                                                         │
│   1041                UpdateGetOldNewValue(table, col_id, old_v, new_v);                                                                                                                                │
│   1042                res.insert(eng->delete_or_update_thread_pool.add_task(&core::TianmuTable::UpdateItem, this, row_id, col_id,                                                                       │
│   1043                                                                      old_v, new_v, current_txn_));                                                                                               │
│   1044                continue;                                                                                                                                                                         │
│   1045              }                                                                                                                                                                                   │
│   1046            }                                                                                                                                                                                     │
│   1047                                                                                                                                                                                                  │
│   1048            auto o_ptr = field->ptr - table->record[0] + old_data;                                                                                                                                │
│   1049            auto n_ptr = field->ptr - table->record[0] + new_data;                                                                                                                                │
│   1050            if (field->is_null_in_record(old_data) || std::memcmp(o_ptr, n_ptr, field->pack_length()) != 0) {                                                                                     │
│   1051              core::Value old_v, new_v;                                                                                                                                                           │
│   1052              UpdateGetOldNewValue(table, col_id, old_v, new_v);                                                                                                                                  │
│  >1053              res.insert(eng->delete_or_update_thread_pool.add_task(&core::TianmuTable::UpdateItem, this, row_id, col_id, old_v,                                                                  │
│   1054                                                                    new_v, current_txn_));                                                                                                        │
│   1055            }                                                                                                                                                                                     │
│   1056          }                                                                                                                                                                                       │
│   1057          res.get_all_with_except();                                                                                                                                                              │
│   1058          return 0;                                                                                                                                                                               │
│   1059        }          

The update operations were done by a thread and which was put into a thread pool at

res.insert(eng->delete_or_update_thread_pool.add_task(&core::TianmuTable::UpdateItem, this, row_id, col_id,
                                                              old_v, new_v, current_txn_));

[RingsC]

In this function, here, tianum still to update the index if its uniqueness is DUP_KEY.

common::ErrorCode TianmuTableIndex::UpdateIndex(core::Transaction *tx, std::string &nkey, std::string &okey,
                                                uint64_t row) {
  StringWriter value, packkey;
  std::vector<std::string> ofields, nfields;

  ofields.emplace_back(okey);
  nfields.emplace_back(nkey);

  rocksdb_key_->pack_key(packkey, ofields, value);
  common::ErrorCode err_code = CheckUniqueness(tx, {(const char *)packkey.ptr(), packkey.length()});
  if (err_code == common::ErrorCode::DUPP_KEY) {
    const auto cf = rocksdb_key_->get_cf();
    tx->KVTrans().Delete(cf, {(const char *)packkey.ptr(), packkey.length()});
  } else {
    TIANMU_LOG(LogCtl_Level::WARN, "RockDb: don't have the key for update!");
  }
  err_code = InsertIndex(tx, nfields, row);
  return err_code;
}

The logical of CheckUniqueness is wierd.

common::ErrorCode TianmuTableIndex::CheckUniqueness(core::Transaction *tx, const rocksdb::Slice &pk_slice) {
  std::string retrieved_value;

  rocksdb::Status s = tx->KVTrans().Get(rocksdb_key_->get_cf(), pk_slice, &retrieved_value);

  if (s.IsBusy() && !s.IsDeadlock()) {
    tx->KVTrans().Releasesnapshot();
    tx->KVTrans().Acquiresnapshot();
    s = tx->KVTrans().Get(rocksdb_key_->get_cf(), pk_slice, &retrieved_value);
  }

  if (!s.ok() && !s.IsNotFound()) {
    TIANMU_LOG(LogCtl_Level::ERROR, "RockDb read fail:%s", s.getState());
    return common::ErrorCode::FAILED;
  }

  if (s.ok()) {
    return common::ErrorCode::DUPP_KEY;
  }

  return common::ErrorCode::SUCCESS;
}

[RingsC]

The root cause:
1: In Update or insert oper, it can not deal with CheckUniqueness properly.
2: the logic of CheckUniqueness is handled properly.

[RingsC]

In TianmuTableIndex::InsertIndex, it always do uniqueness check. But, we think that the check should be done according to whether the primary key or unique constraints exists or not. And, the key we check is not

Taking the following cases as an instance.

1. create table t1(a int, primary key(a), ) engine =tianmu;
2. alter table t1 add ADD CONSTRAINT unique_check UNIQUE (an);

[RingsC]

mysql> create database test; 
Query OK, 1 row affected (0.00 sec)

mysql> use test; 
Database changed
mysql> CREATE TABLE t1  (id int(11) NOT NULL auto_increment,  parent_id int(11) DEFAULT '0' NOT NULL,  level tinyint(4)
    -> DEFAULT '0' NOT NULL, PRIMARY KEY (id)) engine=tianmu;
Query OK, 0 rows affected (0.01 sec)

mysql> INSERT INTO t1 VALUES (3,1,1),(4,1,1);
Query OK, 2 rows affected (1 min 49.52 sec)
Records: 2  Duplicates: 0  Warnings: 0

mysql> INSERT INTO t1 VALUES (3,1,1),(4,1,1);
ERROR 1062 (23000): Duplicate entry '3' for key 'PRIMARY'
mysql> select * from t1; 
+----+-----------+-------+
| id | parent_id | level |
+----+-----------+-------+
|  3 |         1 |     1 |
|  4 |         1 |     1 |
+----+-----------+-------+
2 rows in set (0.00 sec)

mysql> update ignore t1 set id=id+1;
Query OK, 2 rows affected (3.80 sec)
Rows matched: 2  Changed: 2  Warnings: 0

mysql> select * from t1; 
+----+-----------+-------+
| id | parent_id | level |
+----+-----------+-------+
|  3 |         1 |     1 |
|  5 |         1 |     1 |
+----+-----------+-------+
2 rows in set (0.00 sec)

and for multi-keys:

mysql> CREATE TABLE t1  (id int(11) NOT NULL auto_increment,  parent_id int(11) DEFAULT '0' NOT NULL,  level tinyint(4)
    -> DEFAULT '0' NOT NULL, PRIMARY KEY (id, parent_id)) engine=tianmu;
Query OK, 0 rows affected (0.01 sec)

mysql> show create table t1; 
+-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Table | Create Table                                                                                                                                                                                                                |
+-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| t1    | CREATE TABLE `t1` (
  `id` int(11) NOT NULL AUTO_INCREMENT,
  `parent_id` int(11) NOT NULL DEFAULT '0',
  `level` tinyint(4) NOT NULL DEFAULT '0',
  PRIMARY KEY (`id`,`parent_id`)
) ENGINE=TIANMU DEFAULT CHARSET=utf8mb4 |
+-------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
1 row in set (0.01 sec)

mysql> INSERT INTO t1 VALUES (3,1,1),(4,1,1);
Query OK, 2 rows affected (5 min 41.22 sec)
Records: 2  Duplicates: 0  Warnings: 0

mysql> INSERT INTO t1 VALUES (3,1,1),(4,1,1);
ERROR 1062 (23000): Duplicate entry '3-1' for key 'PRIMARY'
mysql> update ignore t1 set id=id+1;
Query OK, 2 rows affected (2.80 sec)
Rows matched: 2  Changed: 2  Warnings: 0

mysql> select * from t1; 
+----+-----------+-------+
| id | parent_id | level |
+----+-----------+-------+
|  4 |         1 |     1 |
|  5 |         1 |     1 |
+----+-----------+-------+
2 rows in set (0.00 sec)