Re: [Maria-developers] Review of last part of discover code

8 Apr 2013


      Hi, Michael!

On Apr 08, Michael Widenius wrote:
...
...
...
...
...
...
"Sergei" == Sergei Golubchik  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

    