Hi Sergei! Now updated the patch to
git diff dfa2d0bc13..9b564832e3 On Wed, Jul 21, 2021 at 11:33 PM Sergei Golubchik <[email protected]> wrote: > > Hi, Aleksey! > > As before, despite the email subject, it's > > git diff dfa2d0bc13..e533b5fb307 > > Don't be confused :) > > On Jul 21, Aleksey Midenkov wrote: > > > 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. > > + } > > + *((Ordered_blob_storage ***) ptr)= blob_storage; > > + blob_storage+= table->s->blob_fields; > > + } > > + int2store(ptr + sizeof(String **), i); > > ptr+= m_priority_queue_rec_len; > > } > > m_start_key.key= (const uchar*)ptr; > > @@ -6291,6 +6333,43 @@ int > > ha_partition::handle_ordered_index_scan_key_not_found() > > } > > > > > > +void ha_partition::swap_blobs(uchar * rec_buf, Ordered_blob_storage ** > > storage, bool restore) > > +{ > > + uint *ptr, *end; > > + uint blob_n= 0; > > + table->move_fields(table->field, rec_buf, table->record[0]); > > + for (ptr= table->s->blob_field, end= ptr + table->s->blob_fields; > > + ptr != end; ++ptr, ++blob_n) > > + { > > + DBUG_ASSERT(*ptr < table->s->fields); > > + Field_blob *blob= (Field_blob*) table->field[*ptr]; > > + DBUG_ASSERT(blob->flags & BLOB_FLAG); > > + DBUG_ASSERT(blob->field_index == *ptr); > > + if (!bitmap_is_set(table->read_set, *ptr) || blob->is_null()) > > + continue; > > + > > + Ordered_blob_storage &s= *storage[blob_n]; > > + > > + if (restore) > > + { > > + if (!s.blob.is_empty()) > > + blob->swap(s.blob, s.set_read_value); > > don't you need here something like > > else > blob->reset(); > > to make blob empty? I believe no. We protect only blob cache (value or read_value). If the cache was empty that doesn't mean the blob was empty. AFAIR blobs allocated by a storage engine should work just fine. Added a comment about 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. > > +} > > + > > + > > /* > > Common routine to handle index_next with ordered results > > > > Regards, > Sergei > VP of MariaDB Server Engineering > and [email protected] -- All the best, Aleksey Midenkov @midenok _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
