MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Ingo Strüwing Date:November 24 2008 8:01am
Subject:Re: bzr commit into mysql-5.1 branch (ingo.struewing:2712) Bug#39277
View as plain text  
Hi Sergey,

Sergey Vojtovich, 20.11.2008 16:41:
> Hi Ingo,
> 
> thanks for the patch. It is mostly what I suggested. But still I have a few
> concerns.
> 
> First of all, I believe doing double comparision is way too expensive when we
> open a table, whereas single comparision is sufficient.
> 
> Besides, I believe there is some misunderstanding re partitioning. It needs
> the same function as MyISAM when we perform create table.
> 
> The suggestions are:
> - swap names of test_if_data_home_dir() and test_for_data_home_dir()
>   functions. This way we would avoid the need to assign
>   myisam_test_invalid_symlink and maria_test_invalid_symlink new values;
> - make newly implemented test_for_data_home_dir() non-static;
> - remove second comparision from newly implemented test_for_data_home_dir();
> - use test_for_data_home_dir() instead of test_if_data_home_dir() in
>   sql_table.cc.

I am pretty unhappy with the idea of having two global functions with so
similar names.

I did some further investigation and can confirm that you are right, if
I understand you correctly. test_if_data_home_dir() is currently used
when opening index file and data file of a MyISAM table. Since the files
must exist for a successful open, we don't have a problem with
realpath(3) here. A second check would be wasted time.

So we have the problem in CREATE/ALTER TABLE only. As this is the only
case where the files don't exist in advance. But then I think, my first
patch was correct. It fixed it in mysql_create_table_no_lock(). At this
place, partitioned tables use check_partition_dirs() to call
test_if_data_home_dir() for every partition. These do not have the table
(partition) name added, so they check the directory, which has to exist.
So we should be safe here.

Regards
Ingo
-- 
Ingo Strüwing, Database Group
Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schröder, Wolfgang Engels, Dr. Roland Bömer
Vorsitzender des Aufsichtsrates: Martin Häring   HRB München 161028

Thread
bzr commit into mysql-5.1 branch (ingo.struewing:2712) Bug#39277Ingo Struewing19 Nov
Re: bzr commit into mysql-5.1 branch (ingo.struewing:2712) Bug#39277Ingo Strüwing19 Nov
Re: bzr commit into mysql-5.1 branch (ingo.struewing:2712) Bug#39277Ingo Strüwing24 Nov
Re: bzr commit into mysql-5.1 branch (ingo.struewing:2712) Bug#39277Ingo Strüwing24 Nov