Re: [Maria-developers] [Commits] Rev 2816: Fixed second part of the LP BUG#615760 - incorrect parameters of the temporary heap table index (unique & nulls are equal). in file:///home/bell/maria/bzr/work-maria-5.3-lb615760/
Hi, Sanja! On Sep 07, sanja@askmonty.org wrote:
At file:///home/bell/maria/bzr/work-maria-5.3-lb615760/
------------------------------------------------------------ revno: 2816 revision-id: sanja@askmonty.org-20100907061732-yyuwtfg0ucyqiu8j parent: sanja@askmonty.org-20100906123424-2sco3ghittvidm6l committer: sanja@askmonty.org branch nick: work-maria-5.3-lb615760 timestamp: Tue 2010-09-07 09:17:32 +0300 message: Fixed second part of the LP BUG#615760 - incorrect parameters of the temporary heap table index (unique & nulls are equal).
=== modified file 'sql/sql_expression_cache.cc' --- a/sql/sql_expression_cache.cc 2010-09-06 12:34:24 +0000 +++ b/sql/sql_expression_cache.cc 2010-09-07 06:17:32 +0000
I cannot review that change, I don't know sql_expression_cache code :( Anyway, you asked me to look at the "incorrect heap table index flags" changes only.
=== modified file 'sql/table.cc' --- a/sql/table.cc 2010-07-10 10:37:30 +0000 +++ b/sql/table.cc 2010-09-07 06:17:32 +0000 @@ -5191,7 +5191,7 @@ keyinfo->usable_key_parts= keyinfo->key_parts = key_parts; keyinfo->key_length=0; keyinfo->algorithm= HA_KEY_ALG_UNDEF; - keyinfo->flags= HA_GENERATED_KEY; + keyinfo->flags= HA_GENERATED_KEY | HA_NOSAME; sprintf(buf, "key%i", key); if (!(keyinfo->name= strdup_root(&mem_root, buf))) return TRUE; @@ -5230,6 +5230,7 @@ { key_part_info->store_length+= HA_KEY_NULL_LENGTH; keyinfo->key_length+= HA_KEY_NULL_LENGTH; + keyinfo->flags|= HA_NULL_ARE_EQUAL; // def. that NULL == NULL } if ((*reg_field)->type() == MYSQL_TYPE_BLOB || (*reg_field)->real_type() == MYSQL_TYPE_VARCHAR)
Difficult to say. Sure, temporary tables should have keyinfo->flags=HA_NOSAME and keyinfo->flags|= HA_NULL_ARE_EQUAL. You can see that in create_tmp_table(). HA_GENERATED_KEY has no effect, so it doesn't matter if you specify it or not. But I don't know what are requirements for temporary tables in your subquery cache code. Unrelated question - why did you duplicate creation of temporary tables? I understand that you may've needed something slightly different from create_tmp_table(), but then you should've changed create_tmp_table() to use your new code, TABLE::add_tmp_key() etc. By the way, if you had done that (reused the code, not duplicated it) this bug would've been impossible - many tests in the test suite would fail at once if all temporary tables would have non-unique indexes. Regards, Sergei
Hi! On 07.09.2010 17:49, Sergei Golubchik wrote:
Hi, Sanja!
On Sep 07, sanja@askmonty.org wrote:
At file:///home/bell/maria/bzr/work-maria-5.3-lb615760/
------------------------------------------------------------ revno: 2816 revision-id: sanja@askmonty.org-20100907061732-yyuwtfg0ucyqiu8j parent: sanja@askmonty.org-20100906123424-2sco3ghittvidm6l committer: sanja@askmonty.org branch nick: work-maria-5.3-lb615760 timestamp: Tue 2010-09-07 09:17:32 +0300 message: Fixed second part of the LP BUG#615760 - incorrect parameters of the temporary heap table index (unique& nulls are equal).
=== modified file 'sql/sql_expression_cache.cc' --- a/sql/sql_expression_cache.cc 2010-09-06 12:34:24 +0000 +++ b/sql/sql_expression_cache.cc 2010-09-07 06:17:32 +0000
I cannot review that change, I don't know sql_expression_cache code :( Anyway, you asked me to look at the "incorrect heap table index flags" changes only.
=== modified file 'sql/table.cc' --- a/sql/table.cc 2010-07-10 10:37:30 +0000 +++ b/sql/table.cc 2010-09-07 06:17:32 +0000 @@ -5191,7 +5191,7 @@ keyinfo->usable_key_parts= keyinfo->key_parts = key_parts; keyinfo->key_length=0; keyinfo->algorithm= HA_KEY_ALG_UNDEF; - keyinfo->flags= HA_GENERATED_KEY; + keyinfo->flags= HA_GENERATED_KEY | HA_NOSAME; sprintf(buf, "key%i", key); if (!(keyinfo->name= strdup_root(&mem_root, buf))) return TRUE; @@ -5230,6 +5230,7 @@ { key_part_info->store_length+= HA_KEY_NULL_LENGTH; keyinfo->key_length+= HA_KEY_NULL_LENGTH; + keyinfo->flags|= HA_NULL_ARE_EQUAL; // def. that NULL == NULL } if ((*reg_field)->type() == MYSQL_TYPE_BLOB || (*reg_field)->real_type() == MYSQL_TYPE_VARCHAR)
Difficult to say.
Sure, temporary tables should have keyinfo->flags=HA_NOSAME and keyinfo->flags|= HA_NULL_ARE_EQUAL. You can see that in create_tmp_table(). HA_GENERATED_KEY has no effect, so it doesn't matter if you specify it or not.
But I don't know what are requirements for temporary tables in your subquery cache code.
I need unique index on almost all fields (actually primary index).
Unrelated question - why did you duplicate creation of temporary tables? I understand that you may've needed something slightly different from create_tmp_table(), but then you should've changed create_tmp_table() to use your new code, TABLE::add_tmp_key() etc.
The function is not mine, and it is made by Evgen in 6.0 then redone by Igor, and here is good point, AFAIK Igor will need not only unique indeces for his tables.
By the way, if you had done that (reused the code, not duplicated it) this bug would've been impossible - many tests in the test suite would fail at once if all temporary tables would have non-unique indexes.
Everything we have in create_tmp_table is not suitable for my task and especially for Igor task. He creates all possible useful indeces over temporary table which, then drop unneeded and use only suitable for join optimisation. So it could not be done in one function, and need several calls to add index by index. [skip]
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik