Hi!
On Thu, Nov 20, 2008 at 11:09:17AM +0000, Sergey Glukhov wrote:
> #At file:///home/gluh/MySQL/mysql-5.1-bug-37284/ based on
> revid:sergey.glukhov@stripped
>
> 2717 Sergey Glukhov 2008-11-20
> Bug#31399 Wrong query result when doing join buffering over BIT fields
> if table has bit fields then uneven bits(if exist) are stored into null bits
> place.
> So we need to copy null bits in case of bit field presence.
> modified:
> mysql-test/r/type_bit.result
> mysql-test/t/type_bit.test
> sql/sql_select.cc
>
> per-file messages:
> mysql-test/r/type_bit.result
> test result
> mysql-test/t/type_bit.test
> test case
> sql/sql_select.cc
> if table has bit fields then uneven bits(if exist) are stored into null bits
> place.
> So we need to copy null bits in case of bit field presence.
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2008-11-03 10:40:58 +0000
> +++ b/sql/sql_select.cc 2008-11-20 11:08:48 +0000
> @@ -13817,6 +13817,7 @@ join_init_cache(THD *thd,JOIN_TAB *table
> CACHE_FIELD *copy,**blob_ptr;
> JOIN_CACHE *cache;
> JOIN_TAB *join_tab;
> + bool have_bit_fields= FALSE;
Join buffer may contain fields from different tables. have_bit_fields is an
attribute of a table. You never reset it, so if we have a join order
t1 t2 t3 j4 [join buffering use here]
and there's a BIT field in table t1, then we'll have have_bit_fields==TRUE
for t2 and t3, which is wrong.
Please move have_bit_fields definition into the for (each table) loop.
> DBUG_ENTER("join_init_cache");
>
> cache= &tables[table_count].cache;
> @@ -13862,11 +13863,14 @@ join_init_cache(THD *thd,JOIN_TAB *table
> (*blob_ptr++)=copy;
> if (field->maybe_null())
> null_fields++;
(*)
> + if (field->type() == MYSQL_TYPE_BIT)
> + have_bit_fields= TRUE;
> copy++;
> }
> }
> /* Copy null bits from table */
> - if (null_fields && tables[i].table->s->null_fields)
(**)
> + if ((null_fields && tables[i].table->s->null_fields) ||
> + have_bit_fields)
> { /* must copy null bits */
> copy->str= tables[i].table->null_flags;
> copy->length= tables[i].table->s->null_bytes;
Do you understand the lines (*) and (**)?
(*) uses maybe_null() which takes into account that the field be NULL
because the table is an inner table of an outer join with and the record
is the NULL-complemented record. On the other hand, we see that NULL
complementation bit is handled separately. Wouldn't it be more logical to
use real_maybe_null() there?
(**) has "... && tables[i].table->s->null_fields" part, whose only meaning
seems to be to protect against the effect of use of maybe_null() instead of
real_maybe_null() at line (*).
Does the above miss anything? If not, I suggest changing maybe_null() to
real_maybe_null() at (*) and removhing the "&&
tables[i].table->s->null_fields"
part at (**).
BR
Sergey
--
Sergey Petrunia, Lead Software Engineer
MySQL AB, www.mysql.com
Office: N/A
Blog: http://s.petrunia.net/blog
