Hi Mats,
A few comments below. I implemented most of the suggestions save the
net_store_length() as we discussed.
The latest patches are:
WL#3228: http://lists.mysql.com/commits/31729
WL#3915: http://lists.mysql.com/commits/31730
> I'm still wondering what makes the old unpack method work when the
> real_types are not the same. Could you given an example?
That's very much a mystery to me. I discovered during testing of ndb that
ndb can send a row somehow that has not had a tablemap processed. So the
default return value from my new methods is 0. Thus, I had to pass it to the
old unpack() method.
> Remove the commented lines.
DOH! Classic newbie goof. My apologies. *blush*
> > + uint length;
> > + uint l_bytes= (param_data && (param_data < field_length)) ?
> > + (param_data <= 255) ? 1 : 2 : length_bytes;
> > + if (l_bytes == 1)
> > + {
> > + to[0]= *from++;
> > + length= to[0];
> > + if (length_bytes == 2)
> > + to[1]= 0;
> > + }
> > + else
> > + {
> > + length= uint2korr(from);
> > + to[0]= *from++;
> > + to[1]= *from++;
> >
>
> This part is used both if l_bytes is 2 or if it is length_bytes and
> you're always reading two bytes. Is it possible that
> length_bytes can be
> 1 and you are trying to read two bytes?
BUG#22086 will take care of this situation.
>
> > Table_map_log_event::~Table_map_log_event()
> > {
> > + my_free(m_meta_memory, MYF(MY_ALLOW_ZERO_PTR));
> > my_free(m_memory, MYF(MY_ALLOW_ZERO_PTR));
> >
>
> Is it possible to combine these two allocations into one?
No, the timing of the information is such that I cannot allocate them until
I know the size.
>
> > + {
> > + Field_blob *fb= new Field_blob(m_field_metadata[col]);
> >
>
> Is it not possible to at least allocate the Field_blob
> instance on the
> stack by removing the "new"?
Yes. I did.
> const_cast<RELAY_LOG_INFO*>(rli)->get_tabledef(table);
> >
>
> Why do you need to cast away const-ness? Conceptually, I
> would say that
> get_tabledef() does not require a non-const RELAY_LOG_INFO
> instance: if
> it changes things internally, I would suggest placing a cast there
> instead of forcing the interface to adapt to implementation issues.
I dunno. Me fix. ;)
Chuck
