Hi!
On Jul 03, Ingo Strüwing wrote:
> 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.
I simply noticed that table->s->fields in bitmap_init() is likely the
number of bits, not bytes, and looked inside to see how the number of
bytes is calculated.
> 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.
Of course. Thanks for fixing it!
> 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:
Yes, please do.
> 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:
...
> 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);
> + }
Does it mean that partition fields will be in read_set even for SELECT's
when there's no need to calculate partition function ?
I'd expect read_set to be extended only if a write is expected (e.g. if
write_set is not empty).
Regards / Mit vielen Grüssen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Developer
/_/ /_/\_, /___/\___\_\___/ MySQL GmbH, Radlkoferstr. 2, D-81373 München
<___/ Geschäftsführer: Kaj Arnö - HRB
München 162140
