Hi, Mattias!
On Sep 03, Mattias Jonsson wrote:
> #At file:///Users/mattiasj/clones/bzrroot/b32430_2-60-bugteam/
>
> 2753 Mattias Jonsson 2008-09-03
> Bug#32430:'show innodb status' causes errors Invalid (old?) table
> or database name in logs
>
> Problem was that InnoDB used filename_to_tablename,
> which do not handle partitions (due to the '#' in
> the filename).
>
> Solution is to add a new function to use to explain
> what the filename means, explain_filename
> It expands to Database, Table, [Temporary|Renamed] Partition,
> Subpartition and uses errmsg.txt for localization.
>
> This will first go through review in Sun for the server part,
> and if approved, go through review by Oracle/InnoBASE and if
> approved it will then be included in both server and InnoDB.
Pretty good.
There're a few comments though, please see below
> === modified file 'sql/sql_table.cc'
> --- a/sql/sql_table.cc 2008-07-26 16:38:20 +0000
> +++ b/sql/sql_table.cc 2008-09-03 20:07:50 +0000
...
> +/**
> + @brief Explain a file name by split it to [database,]tablename[,partname...]
> +
> + @details Break down the filename to its logic parts and add the partname
> + (database, table, partition, subpartition).
> + filename_to_tablename cannot be used on partitions, due to the #P# part.
> + There can be up to 6 '#', #P# for partition, #SP# for subpartition
> + and #TMP# or #REN# for temporary or renamed partitions.
Please explain where this function should be used (in generating
diagnostic / error messages about something happened in a file, e.g.
read error, corruption, whatever).
> + @param from The file name in my_charset_filename.
> + @param to The explained name in my_charset_filename.
> + @param to_length size of to buffer
> + @param lpt alter table data structure
> +
> + @retval length of returned string
> +*/
> +
> +uint explain_filename(const char *from, char *to, uint to_length)
> +{
> + uint res= 0;
> + char *to_p= to;
> + char *end_p= to_p + to_length;
> + const char *db_name= NULL;
> + int db_name_len= 0;
> + const char *table_name;
> + int table_name_len= 0;
> + const char *part_name= NULL;
> + int part_name_len= 0;
> + const char *subpart_name= NULL;
> + int subpart_name_len= 0;
> + int temp_or_renamed= 0; /* 1= temp, 2= renamed */
make it a
enum { NORMAL, TEMP, RENAMED} status;
please
> + const char *tmp_p;
> + DBUG_ENTER("explain_filename");
> + DBUG_PRINT("enter", ("from '%s'", from));
> + /*
> + if '/' then take last directory part as database
> + Compose the explained name as
> + [Database %s, ]Table %s[,[ Temporary| Renamed] Partition %s
> + [, Subpartition %s]]
> + */
> + tmp_p= from;
> + table_name= from;
> + while ((tmp_p= strchr(tmp_p, '/')))
I'm not sure the name is normalized here, perhaps you need to use
FN_LIBCHAR and not '/'. Try it on Windows.
> + {
> + db_name= table_name;
> + /* calculate the length */
> + db_name_len= tmp_p - db_name;
> + tmp_p++;
> + table_name= tmp_p;
> + }
> + tmp_p= table_name;
> + while (!res && (tmp_p= strchr(tmp_p, '#')))
> + {
> + tmp_p++;
> + switch (tmp_p[0]) {
> + case 'P':
> + case 'p':
> + if (tmp_p[1] == '#')
> + part_name= tmp_p + 2;
> + else
> + res= 1;
> + tmp_p+= 2;
> + break;
> + case 'S':
> + case 's':
> + if (tmp_p[2] == '#' && (tmp_p[1] == 'P' || tmp_p[1] == 'p'))
> + {
> + part_name_len= tmp_p - part_name - 1;
> + subpart_name= tmp_p + 3;
> + }
> + else
> + res= 2;
> + tmp_p+= 3;
> + break;
> + case 'T':
> + case 't':
> + if (tmp_p[3] == '#' && (tmp_p[1] == 'M' || tmp_p[1] == 'm')
> &&
> + (tmp_p[1] == 'P' || tmp_p[1] == 'p') && !tmp_p[4])
here, above and below - you need to make sure you don't look beyond the
end of the string, it may be unallocated memory.
So, you should always test for
tmp_p[1] == ...
&& tmp_p[2] == ...
&& tmp_p[3] == ...
etc
that is, do it in order, and never look at tmp_p[3] before you made sure
that tmp_p[2] and tmp_p[1] aren't '\0'.
> + temp_or_renamed= 1; /* 1= temp, 2= renamed */
> + else
> + res= 3;
> + tmp_p+= 4;
> + break;
> + case 'R':
> + case 'r':
> + if (tmp_p[3] == '#' && (tmp_p[1] == 'E' || tmp_p[1] == 'e')
> &&
> + (tmp_p[1] == 'N' || tmp_p[1] == 'n') && !tmp_p[4])
> + temp_or_renamed= 2; /* 1= temp, 2= renamed */
see, enums were introduced in the language exactly for this purpose :)
> + else
> + res= 4;
> + tmp_p+= 4;
> + break;
Regards / Mit vielen Grüßen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect
/_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB München 161028
<___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring
