← Back to team overview

maria-developers team mailing list archive

Re: Review of last part of discover code

 

Hi, Michael!

On Apr 08, Michael Widenius wrote:
> >>>>> "Sergei" == Sergei Golubchik <serg@xxxxxxxxxxxx> writes:
> 
> I meant that you could have used:
> 
> if (!tabledef_version.str= (uchar*) memdup_root(&mem_root, extra2, length))
>   goto err;

Done.

> >> >@@ -3367,7 +3204,7 @@ rename_file_ext(const char * from,const
> >> >   char from_b[FN_REFLEN],to_b[FN_REFLEN];
> >> >   (void) strxmov(from_b,from,ext,NullS);
> >> >   (void) strxmov(to_b,to,ext,NullS);
> >> >-  return (mysql_file_rename(key_file_frm, from_b, to_b, MYF(MY_WME)));
> >> >+  return (mysql_file_rename(key_file_frm, from_b, to_b, MYF(0)));
> >> 
> >> Why not give a warning if rename fales?
> >> 
> >> At least, use 'ME_JUST_WARNING'
> 
> > Because it's neither a warning nor an error, it's a normal situation.
> > For a discovering table some files with some extensions may not exist.
> 
> At least, add it the flag to rename_file_ext() so that the caller can
> decide if there should be a warning or not.
> (We don't want any silent failures, do we)
> 
> The caller can't know if the above succeeded or not.
> (For example because of a permission error).

The caller does:

    if (rename_file_ext(from, to, *ext))
    {
      if ((error=my_errno) != ENOENT)
        break;
      error= 0;
    }

A permission error will be noticed and reported, as far as I can see.

> > The comment means that if we ever need to handle options_len > 64K,
> > we should extend this length-encoding logic to support larger
> > options_len.  If I'd put int3store, as you suggest, the assert
> > would've looked
> >    DBUG_ASSERT(options_len <= 16777216); // FIXME if necessary
> > it wouldn't go away.
> 
> > But for now, I think, 64K is large enough. We can extend it when we
> > need it.
> 
> Then please add a better comment. I thought that FIXME meant that
> someone should fix the code ASAP.

Done.

Regards,
Sergei


References