Re: [Maria-developers] 4ca016237f1: MDEV-20001 Potential dangerous regression: INSERT INTO >=100 rows fail for myisam table with HASH indexes
Hi, Sachin! On Aug 20, Sachin Setiya wrote:
revision-id: 4ca016237f1 (mariadb-10.4.5-152-g4ca016237f1) parent(s): 4a5cd407289 author: Sachin <sachin.setiya@mariadb.com> committer: Sachin <sachin.setiya@mariadb.com> timestamp: 2019-07-29 19:33:05 +0530 message:
MDEV-20001 Potential dangerous regression: INSERT INTO >=100 rows fail for myisam table with HASH indexes
Dont deactivate the long unique keys on bulk insert.
diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index f478e01e441..c1169737911 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -1749,7 +1749,16 @@ void ha_myisam::start_bulk_insert(ha_rows rows, uint flags) else { my_bool all_keys= MY_TEST(flags & HA_CREATE_UNIQUE_INDEX_BY_SORT); - mi_disable_indexes_for_rebuild(file, rows, all_keys); + if (table->s->long_unique_table) + { + ulonglong hash_key_map= 0ULL; + for(uint i= 0; i < table->s->keys; i++) + if (table->key_info[i].algorithm == HA_KEY_ALG_LONG_HASH) + mi_set_key_active(hash_key_map, i); + mi_disable_indexes_for_rebuild(file, rows, all_keys, hash_key_map); + } + else + mi_disable_indexes_for_rebuild(file, rows, all_keys, 0ULL);
I agree with the fix, but here's a comment about the implementation. mi_disable_indexes_for_rebuild() is _only_ used here, nowhere else. So, I think I'd just removed mi_disable_indexes_for_rebuild(), inlined it here and do everything in one loop. Note, that you also need to fix Aria engine, it has almost the same code. And add a test for it, please.
} }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
Hi Serg! Thanks for review On Tue, Aug 20, 2019 at 6:08 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Sachin!
On Aug 20, Sachin Setiya wrote:
revision-id: 4ca016237f1 (mariadb-10.4.5-152-g4ca016237f1) parent(s): 4a5cd407289 author: Sachin <sachin.setiya@mariadb.com> committer: Sachin <sachin.setiya@mariadb.com> timestamp: 2019-07-29 19:33:05 +0530 message:
MDEV-20001 Potential dangerous regression: INSERT INTO >=100 rows fail for myisam table with HASH indexes
Dont deactivate the long unique keys on bulk insert.
diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index f478e01e441..c1169737911 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -1749,7 +1749,16 @@ void ha_myisam::start_bulk_insert(ha_rows rows, uint flags) else { my_bool all_keys= MY_TEST(flags & HA_CREATE_UNIQUE_INDEX_BY_SORT); - mi_disable_indexes_for_rebuild(file, rows, all_keys); + if (table->s->long_unique_table) + { + ulonglong hash_key_map= 0ULL; + for(uint i= 0; i < table->s->keys; i++) + if (table->key_info[i].algorithm == HA_KEY_ALG_LONG_HASH) + mi_set_key_active(hash_key_map, i); + mi_disable_indexes_for_rebuild(file, rows, all_keys, hash_key_map); + } + else + mi_disable_indexes_for_rebuild(file, rows, all_keys, 0ULL);
I agree with the fix, but here's a comment about the implementation. mi_disable_indexes_for_rebuild() is _only_ used here, nowhere else. So, I think I'd just removed mi_disable_indexes_for_rebuild(), inlined it here and do everything in one loop.
Done.
Note, that you also need to fix Aria engine, it has almost the same code. And add a test for it, please.
Actually long unique index does not work in Aria (Mdev-18791), So I have
not changed anything in Aria engine. Commit link http://lists.askmonty.org/pipermail/commits/2019-October/014023.html
} }
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
-- Regards Sachin Setiya Software Engineer at MariaDB
participants (2)
-
Sachin Setiya -
Sergei Golubchik