Hi Sergei,
thanks for the review. Please see below for comments.
Sergei Golubchik wrote:
> Hi!
>
> On Jun 27, ingo@stripped wrote:
>> ChangeSet@stripped, 2007-06-27 20:26:52+02:00, istruewing@stripped +4 -0
>> Bug#26827 - table->read_set is set incorrectly,
>> causing update of a different column
...
>> + if (bitmap_init(&part_info->full_part_field_set, NULL,
>> + table->s->fields, FALSE))
>> + {
>> + mem_alloc_error(table->s->fields);
>
> not terribly important, but
> strictly speaking it should be bitmap_buffer_size(table->s->fields)
I agree. I learned about this macro later.
On request of Calvin we tested this patch in 5.1-falcon already. It
turned out that valgrind was not happy with the allocation as a free is
missing. I plan to change the patch like so:
- if (bitmap_init(&part_info->full_part_field_set, NULL,
+ if (!(bitmap_buf= (my_bitmap_map*)
+ thd->alloc(bitmap_buffer_size(table->s->fields))))
+ {
+ mem_alloc_error(bitmap_buffer_size(table->s->fields));
+ result= TRUE;
+ goto end;
+ }
+ if (bitmap_init(&part_info->full_part_field_set, bitmap_buf,
Do you agree with this change? This allocation scheme is already used
elsewhere in the partition code.
As agreed before, this(these) patch(es) shall go into mysql-5.1 too.
===
Unrelated changes, which I do not propose as a fix for this bug, but
which I would add, if you like:
I noticed that in rnd_init() and index_init() the function
include_partition_fields_in_used_fields() is called. This could be
optimized, using the new bitmap:
void ha_partition::include_partition_fields_in_used_fields()
{
- Field **ptr= m_part_field_array;
DBUG_ENTER("ha_partition::include_partition_fields_in_used_fields");
- do
- {
- bitmap_set_bit(table->read_set, (*ptr)->field_index);
- } while (*(++ptr));
+ bitmap_union(table->read_set, &m_part_info->full_part_field_set);
DBUG_VOID_RETURN;
}
It could even be eliminated completely:
rnd_init():
- include_partition_fields_in_used_fields();
...
if (bitmap_is_overlapping(&m_part_info->full_part_field_set,
table->write_set))
bitmap_set_all(table->read_set);
+ else
+ {
+ /*
+ Some handlers only read fields as specified by the bitmap for the
+ read set. For partitioned handlers we always require that the
+ fields of the partition functions are read such that we can
+ calculate the partition id to place updated and deleted records.
+ */
+ bitmap_union(table->read_set, &m_part_info->full_part_field_set);
+ }
index_init():
- include_partition_fields_in_used_fields();
+ /*
+ Some handlers only read fields as specified by the bitmap for the
+ read set. For partitioned handlers we always require that the
+ fields of the partition functions are read such that we can
+ calculate the partition id to place updated and deleted records.
+ */
+ bitmap_union(table->read_set, &m_part_info->full_part_field_set);
Regards
Ingo
--
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Radlkoferstr. 2, D-81373 München
Geschäftsführer: Kaj Arnö - HRB München 162140
