maria-developers team mailing list archive

[Aleksey Midenkov <midenok@xxxxxxxxx>]

Hi Sergei!

On Mon, Jul 26, 2021 at 7:35 PM Sergei Golubchik <serg@xxxxxxxxxxx> wrote:
>
> Hi, Aleksey!
>
> On Jul 23, Aleksey Midenkov wrote:
> > Now updated the patch to
> >
> > git diff dfa2d0bc13..9b564832e3
>
> Thanks, no new comments about it. Just one question below still
>
> > > > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc
> > > > index c53732a2b51..70ed14448ea 100644
> > > > --- a/sql/ha_partition.cc
> > > > +++ b/sql/ha_partition.cc
> > > > @@ -5130,7 +5135,18 @@ bool ha_partition::init_record_priority_queue()
> > > >          i= bitmap_get_next_set(&m_part_info->read_partitions, i))
> > > >    {
> > > >      DBUG_PRINT("info", ("init rec-buf for part %u", i));
> > > > -    int2store(ptr, i);
> > > > +    if (table->s->blob_fields)
> > > > +    {
> > > > +      for (uint j= 0; j < table->s->blob_fields; ++j, ++objs)
> > > > +      {
> > > > +        blob_storage[j]= new (objs) Ordered_blob_storage;
> > > > +        if (!blob_storage[j])
> > > > +          DBUG_RETURN(true);
> > >
> > > is this possible?
> >
> > If we don't use exceptions we should handle OOM this way. If we can't
> > use -fno-exceptions we should make a wrapper by overloading default
> > new() and catching std::badalloc.
>
> I mean, it's a placement new, it doesn't allocate anything.
> How can it return NULL if it simply returns objs?

Oh, right! Updated that.

>
> ...
> > > > +    else
> > > > +    {
> > > > +      bool set_read_value;
> > > > +      String *cached= blob->cached(set_read_value);
> > > > +      if (cached)
> > > > +      {
> > > > +        cached->swap(s.blob);
> > > > +        s.set_read_value= set_read_value;
> > > > +      }
> > > > +    }
> > > > +  }
> > > > +  table->move_fields(table->field, table->record[0], rec_buf);
> > >
> > > On swap_blobs() you always move all fields from rec_buf to table->record[0]?
> > > Why?
> > > This looks very suspicious.
> >
> > Have you seen the first move_fields() call? We work on rec_buf and if
> > it is the same as record[0] move_fields() does nothing.
>
> Oops, no. I missed the first one. And it was very suspicious that you
> always move in one direction.
>
> Regards,
> Sergei
> VP of MariaDB Server Engineering
> and security@xxxxxxxxxxx



-- 
All the best,

Aleksey Midenkov
@midenok