maria-developers team mailing list archive
Mailing list archive
Re: Review of last part of discover code
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;
> >> >@@ -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)
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.