← Back to team overview

maria-developers team mailing list archive

Re: 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@xxxxxxxxxxx>
> committer: Sachin <sachin.setiya@xxxxxxxxxxx>
> 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@xxxxxxxxxxx


Follow ups