11.07.2012 20:14, Sergei Golubchik wrote:
+Note 1061Duplicate 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.cc2012-04-05 21:07:18 +
+++ sql/sql_table.cc2012-07-11 14:28:40 +
@@ -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
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
