11.07.2012 20:14, Sergei Golubchik wrote:
+Note 1061 Duplicate key name 'x_param' +ALTER TABLE t1 ADD KEY IF NOT EXISTS x_param(x_param); typo? you test ADD KEY IF NOT EXISTS twice.
Yes, but not this line. The 'KEY x_param' wasn't supposed to be created with the CREATE TABLE.
uint8 create_view_check; bool drop_if_exists, drop_temporary, local_file, one_shot_set; + bool check_exists; /* Enabled if the IF [NOT] EXISTS specified for ALTER */ I think, you can remove drop_if_exists and use check_exists instead
Seems to work so far...
+opt_if_not_exists_ident: + opt_if_not_exists opt_ident + { + LEX *lex= Lex; + if (lex->check_exists && lex->sql_command != SQLCOM_ALTER_TABLE) + { + my_parse_error(ER(ER_SYNTAX_ERROR)); + MYSQL_YYABORT; + } + $$= $2; + }; + why did you create opt_if_not_exists_ident rule, instead of adding opt_if_not_exists before ident in the alter table grammar?
I'm afraid if i didn't create this rule, i had to add this code to 4 cases of the 'key_def:' And i'd also had to modify more lines, as numbers of parameters after the opt_if_not_exists would increase.
+opt_column_if_exists: + opt_column if_exists + {} + ; + same as above - why a special rule?
Here the sole reason is to avoid modifying the code with the increased parameter's numbers. Ok, i can it's not enough to add a separate rule, going to get rid of this one.
=== modified file 'sql/sql_table.cc' --- sql/sql_table.cc 2012-04-05 21:07:18 +0000 +++ sql/sql_table.cc 2012-07-11 14:28:40 +0000 @@ -5788,6 +5788,226 @@ is_index_maintenance_unique (TABLE *tabl
/* + Preparation for table creation + + SYNOPSIS + handle_if_exists_option() Why did you do that in a separate function? You have basically duplicated all checks. normal alter table code checks for duplicate columns/key names and for non-existant columns/keys for DROP.
this code is still in place after your patch. so, basically, you'll check for duplicates and non-existant names twice. what's the point of that?
I'd expect you to take check_exists flag into account in the existing checking code and convert errors into warnings as appropriate.
Naturally i started doing it that way initially. Then i realized the code looks rather ugly. These checks are scattered around the code and parts of the code with these checks are used not only for ALTER TABLE, but for CREATE and other internal needs. There we need to be sure these check_if_exist flags aren't set. And the loops should be done in a different way in these places so we can remove items from the list in the process. Thus i decided to do that in the separate function to be called from the mysql_alter_table exclusively. In my opinion, the fuction won't affect the performance of the existing functions at all. And it's easy to understand what and why it does, so the code duplication will produce the more error-prone code here. Well, did i convince you the separate function is nicer? Best regards. HF