Hi Serg! Thanks for review On Tue, Aug 20, 2019 at 6:08 PM Sergei Golubchik wrote:

Hi, Sachin!

On Aug 20, Sachin Setiya wrote:

...

revision-id: 4ca016237f1 (mariadb-10.4.5-152-g4ca016237f1) parent(s): 4a5cd407289 author: Sachin committer: Sachin 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