Hi Sergei!

The updated patches are here:

https://github.com/MariaDB/server/commits/2c2e79487a9189e02fce8f884a9ab2b42b9aa333


On Tue, Nov 3, 2020 at 8:28 PM Aleksey Midenkov <midenok@gmail.com> wrote:
>
> Sergei,
>
> the updated patches are here:
>
> https://github.com/MariaDB/server/commits/5f6bcd6cd0145513974b0dfc5b2cefba28b72f1a
>
> On Mon, Nov 2, 2020 at 11:33 PM Sergei Golubchik <serg@mariadb.org> wrote:
> >
> > Hi, Aleksey!
> >
> > Don't be confused, the commit header and the comment come from your last
> > commit, but the diff below includes all three commits that mention MDEV-18734.
> >
...
>
> > > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc
> > > index 9d82df0235c7..7c658082d397 100644
> > > --- a/sql/ha_partition.cc
> > > +++ b/sql/ha_partition.cc
> > > @@ -5125,14 +5128,14 @@ bool ha_partition::init_record_priority_queue()
> > >    uint alloc_len;
> > >    uint used_parts= bitmap_bits_set(&m_part_info->read_partitions);
> > >    /* Allocate record buffer for each used partition. */
> > > -  m_priority_queue_rec_len= m_rec_length + PARTITION_BYTES_IN_POS;
> > > +  m_priority_queue_rec_len= m_rec_length + ORDERED_REC_OFFSET;
> > >    if (!m_using_extended_keys)
> > >        m_priority_queue_rec_len += m_file[0]->ref_length;
> > >    alloc_len= used_parts * m_priority_queue_rec_len;
> > >    /* Allocate a key for temporary use when setting up the scan. */
> > >    alloc_len+= table_share->max_key_length;
> > >
> > > -  if (!(m_ordered_rec_buffer= (uchar*)my_malloc(alloc_len, MYF(MY_WME))))
> > > +  if (!(m_ordered_rec_buffer= (uchar*) alloc_root(&m_ordered_root, alloc_len)))
> >
> > I don't see why you need a memroot here. memroot is needed when you
> > allocate later at difeerent points in time some initially unpredictable
> > amount of memory that has the same lifetime and needs to be
> > freed all at once.
>
> I know... This is just another convenient way of allocation with presumably minimized number of mallocs (probably pre_alloc_size should be tweaked to a better value).
>
> >
> > But here it seems that you only allocate once. You could just make
> >
> >    m_priority_queue_rec_len = m_rec_length + PARTITION_BYTES_IN_POS +
> >               table->s->blob_fields * sizeof(Ordered_blob_storage)
>
> I don't want this data serialized. It is inconvenient to read the dump of the record. Why we should make it harder to code, read the code and debug while we can make it easier? If it is only for guaranteed 1 malloc, I don't think it's worth it.
>