← Back to team overview

maria-developers team mailing list archive

Re: MDEV-5359 CREATE OR REPLACE, CREATE IF NOT EXISTS, DROP IF EXISTS

 

Hi, Alexander!

This is all pretty good. Great work, thanks.

I didn't have many comments. Few about small stylistic changes, and a
couple of more serious ones - about tests and about events. See below.

On Oct 02, Alexander Barkov wrote:
> diff --git a/mysql-test/r/create_drop_db.result b/mysql-test/r/create_drop_db.result
> new file mode 100644
> index 0000000..e7d4c18
> --- /dev/null
> +++ b/mysql-test/r/create_drop_db.result
> @@ -0,0 +1,17 @@
> +CREATE DATABASE db1;
> +CREATE DATABASE IF NOT EXISTS db1;
> +Warnings:
> +Note	1007	Can't create database 'db1'; database exists
> +CREATE OR REPLACE DATABASE db1;
> +CREATE OR REPLACE DATABASE IF NOT EXISTS db2;
> +ERROR HY000: Incorrect usage of OR REPLACE and IF NOT EXISTS
> +DROP DATABASE db1;
> +DROP DATABASE IF EXISTS db1;
> +Warnings:
> +Note	1008	Can't drop database 'db1'; database doesn't exist
> +DROP DATABASE db1;
> +ERROR HY000: Can't drop database 'db1'; database doesn't exist
> +CREATE OR REPLACE DATABASE db1;
> +CREATE DATABASE db1;
> +ERROR HY000: Can't create database 'db1'; database exists
> +DROP DATABASE IF EXISTS db1;

I don't see where this test verifies that CREATE OR REPLACE actually
replaces a database. Add the following:

  create database;
  create table;
  create or replace database;
  the table must not exist now;

and add a test for create if not exists when a database does not exist.
you can combine it with a previous test, if you'd like.

> diff --git a/mysql-test/r/create_drop_event.result b/mysql-test/r/create_drop_event.result
> new file mode 100644
> index 0000000..40d16e6
> --- /dev/null
> +++ b/mysql-test/r/create_drop_event.result
> @@ -0,0 +1,31 @@
> +SET GLOBAL event_scheduler=off;
> +CREATE DATABASE db1;
> +CREATE EVENT event_1 ON SCHEDULE EVERY 1 SECOND DO DROP DATABASE db1;
> +SHOW DATABASES LIKE 'db1';
> +Database (db1)
> +db1
> +SET GLOBAL event_scheduler=on;
> +SHOW DATABASES LIKE 'db1';
> +Database (db1)
> +SET GLOBAL event_scheduler=off;
> +DROP DATABASE db1;
> +ERROR HY000: Can't drop database 'db1'; database doesn't exist
> +CREATE EVENT event_1 ON SCHEDULE EVERY 1 SECOND DO DROP DATABASE db1;
> +ERROR HY000: Event 'event_1' already exists
> +CREATE EVENT IF NOT EXISTS event_1 ON SCHEDULE EVERY 1 SECOND DO DROP DATABASE db1;
> +Warnings:
> +Note	1537	Event 'event_1' already exists
> +CREATE OR REPLACE EVENT IF NOT EXISTS event_1 ON SCHEDULE EVERY 1 SECOND DO DROP DATABASE db1;
> +ERROR HY000: Incorrect usage of OR REPLACE and IF NOT EXISTS
> +CREATE OR REPLACE EVENT event_1 ON SCHEDULE EVERY 1 SECOND DO DROP DATABASE db1;

again, same thing. I don't see a test that the command actually worked.
please use different event definitions in all three create variants.
and add show create event after every create to verify that
create if not exists did *not* modify the event, but create or replace
did modify it.

> +CREATE DATABASE db1;
> +SHOW DATABASES LIKE 'db1';
> +Database (db1)
> +db1
> +SET GLOBAL event_scheduler=on;
> +SHOW DATABASES LIKE 'db1';
> +Database (db1)
> +SET GLOBAL event_scheduler=off;
> +DROP DATABASE db1;
> +ERROR HY000: Can't drop database 'db1'; database doesn't exist
> +DROP EVENT IF EXISTS event_1;
> diff --git a/mysql-test/r/create_drop_function.result b/mysql-test/r/create_drop_function.result
> new file mode 100644
> index 0000000..750d179
> --- /dev/null
> +++ b/mysql-test/r/create_drop_function.result
> @@ -0,0 +1,34 @@
> +SET timestamp=UNIX_TIMESTAMP('2014-09-30 08:00:00');
> +CREATE FUNCTION f1(str char(20))
> +RETURNS CHAR(100)
> +RETURN CONCAT('Hello, ', str, '!');
> +SELECT * FROM mysql.proc WHERE name like 'f1';
> +db	name	type	specific_name	language	sql_data_access	is_deterministic	security_type	param_list	returns	body	definer	created	modified	sql_mode	comment	character_set_client	collation_connection	db_collation	body_utf8
> +test	f1	FUNCTION	f1	SQL	CONTAINS_SQL	NO	DEFINER	str char(20)	char(100) CHARSET latin1	RETURN CONCAT('Hello, ', str, '!')	root@localhost	2014-09-30 08:00:00	2014-09-30 08:00:00			latin1	latin1_swedish_ci	latin1_swedish_ci	RETURN CONCAT('Hello, ', str, '!')
> +SELECT f1('world');
> +f1('world')
> +Hello, world!
> +CREATE FUNCTION f1(str char(20))
> +RETURNS TEXT
> +RETURN CONCAT('Hello, ', str, '!');
> +ERROR 42000: FUNCTION f1 already exists
> +CREATE FUNCTION IF NOT EXISTS f1(str char(20))
> +RETURNS CHAR(100)
> +RETURN CONCAT('Hello, ', str, '!');
> +Warnings:
> +Note	1304	FUNCTION f1 already exists

same. please *different* definitions in different create variants.
and some verification that the definition changed (or didn't change) -
like show create or select f1();

> +CREATE OR REPLACE FUNCTION IF NOT EXISTS f1(str char(20))
> +RETURNS CHAR(100)
> +RETURN CONCAT('Hello, ', str, '!');
> +ERROR HY000: Incorrect usage of OR REPLACE and IF NOT EXISTS
> +CREATE OR REPLACE FUNCTION f1(str char(20))
> +RETURNS CHAR(100)
> +RETURN CONCAT('Hello, ', str, '!');
> +DROP FUNCTION f1;
> +CREATE FUNCTION IF NOT EXISTS f1(str char(20))
> +RETURNS CHAR(100)
> +RETURN CONCAT('Hello, ', str, '!');
> +SELECT f1('world');
> +f1('world')
> +Hello, world!
> +DROP FUNCTION f1;
> diff --git a/mysql-test/r/create_drop_index.result b/mysql-test/r/create_drop_index.result
> new file mode 100644
> index 0000000..23f6388
> --- /dev/null
> +++ b/mysql-test/r/create_drop_index.result
> @@ -0,0 +1,34 @@
> +CREATE TABLE t1(id INT);
> +CREATE INDEX idx ON t1(id);
> +ALTER TABLE t1 ADD INDEX idx(id);
> +ERROR 42000: Duplicate key name 'idx'
> +ALTER TABLE t1 ADD INDEX IF NOT EXISTS idx(id);
> +Warnings:
> +Note	1061	Duplicate key name 'idx'
> +CREATE OR REPLACE INDEX idx2 ON t1(id);

same comment here
(and everywhere below)

> +Warnings:
> +Note	1831	Duplicate index 'idx2' defined on the table 'test.t1'. This is deprecated and will be disallowed in a future release.
> +ALTER TABLE t1 ADD INDEX IF NOT EXISTS idx2(id);
> +Warnings:
> +Note	1061	Duplicate key name 'idx2'
> +CREATE OR REPLACE UNIQUE INDEX idx ON t1(id);
> +CREATE INDEX idx ON t1(id);
> +ERROR 42000: Duplicate key name 'idx'
> +CREATE TABLE t2(col INT);
> +CREATE OR REPLACE INDEX idx ON t2(col);
> +CREATE INDEX idx ON t2(col);
> +ERROR 42000: Duplicate key name 'idx'
> +# Is this correct behaviour?
> +ALTER TABLE t2 ADD FOREIGN KEY fk(col) REFERENCES t1(id);
> +CREATE INDEX fk ON t2(col);
> +Warnings:
> +Note	1831	Duplicate index 'fk' defined on the table 'test.t2'. This is deprecated and will be disallowed in a future release.
> +# Clearing up
> +DROP INDEX idx ON t1;
> +DROP TABLE t1;
> +ALTER TABLE t1 DROP INDEX idx2;
> +ERROR 42S02: Table 'test.t1' doesn't exist
> +ALTER TABLE t2 DROP INDEX idx;
> +ALTER TABLE t2 DROP INDEX fk;
> +ALTER TABLE t2 DROP FOREIGN KEY fk;
> +DROP TABLE t2;
> diff --git a/mysql-test/r/proc_func.result b/mysql-test/r/proc_func.result
> new file mode 100644
> index 0000000..1105ded
> --- /dev/null
> +++ b/mysql-test/r/proc_func.result
> @@ -0,0 +1,63 @@
> +#
> +# Testing CREATE PROCEDURE IF NOT EXISTS
> +#
> +DROP TABLE IF EXISTS t1;
> +DROP PROCEDURE IF EXISTS proc1;
> +CREATE TABLE t1 (id INT);
> +CREATE PROCEDURE proc1 (OUT cnt INT) BEGIN SELECT COUNT(*) INTO cnt FROM t1; END$$

how is this test (and other tests below) different from create_drop_xxx
tests above?

> +CALL proc1(@cnt);
> +SELECT @cnt;
> +@cnt
> +0
> +INSERT INTO t1 VALUES (1), (2), (3);
> +CALL proc1(@cnt);
> +SELECT @cnt;
> +@cnt
> +3
> +CREATE PROCEDURE proc1 (OUT cnt INT) BEGIN SELECT COUNT(*) INTO cnt FROM t1; END$$
> diff --git a/sql/event_db_repository.cc b/sql/event_db_repository.cc
> index b20269b..01cdabb 100644
> --- a/sql/event_db_repository.cc
> +++ b/sql/event_db_repository.cc
> @@ -684,18 +684,28 @@ Event_db_repository::create_event(THD *thd, Event_parse_data *parse_data,
>    DBUG_PRINT("info", ("check existance of an event with the same name"));
>    if (!find_named_event(parse_data->dbname, parse_data->name, table))
>    {
> -    if (create_if_not)
> +    if (thd->lex->is_create_or_replace())
> +    {
> +      if ((ret= table->file->ha_delete_row(table->record[0])))
> +      {
> +        table->file->print_error(ret, MYF(0));
> +        goto end;
> +      }
> +    }
> +    else if (create_if_not)
>      {
>        *event_already_exists= true;
>        push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
>                            ER_EVENT_ALREADY_EXISTS, ER(ER_EVENT_ALREADY_EXISTS),
>                            parse_data->name.str);
>        ret= 0;
> +      goto end;
>      }
>      else
> +    {
>        my_error(ER_EVENT_ALREADY_EXISTS, MYF(0), parse_data->name.str);
> -
> -    goto end;
> +      goto end;
> +    }
>    } else
>      *event_already_exists= false;

I wonder whether this works as it should. On create-or-replace you
delete the old event row and a new one is supposedly inserted. So, the
table is fine.

But event_already_exists will stay unset, undefined, actually, because
the caller didn't set it either. If you set it to TRUE, then the caller
won't update in-memory even list, so your new event won't be enabled.
If you set it to FALSE, it will try to insert a new event without
deleting the old one and I don't know what will happen (either both will
stay active or the insertion fails).

Do you have a test to verify that after CREATE OR REPLACE EVENT the old
event no longer exists and the new event does exist and is executed as
expected?

> diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt
> index ed7976e..1cfaaa4 100644
> --- a/sql/share/errmsg-utf8.txt
> +++ b/sql/share/errmsg-utf8.txt
> @@ -7110,3 +7110,11 @@ ER_IT_IS_A_VIEW 42S02
>          eng "'%-.192s' is a view"
>  ER_SLAVE_SKIP_NOT_IN_GTID
>  	eng "When using GTID, @@sql_slave_skip_counter can not be used. Instead, setting @@gtid_slave_pos explicitly can be used to skip to after a given GTID position."
> +ER_USER_CREATE_EXISTS
> +        eng "Can't create user '%-.64s'@'%-.64s'; it already exists"
> +ER_USER_DROP_EXISTS
> +        eng "Can't drop user '%-.64s'@'%-.64s'; it doesn't exist"
> +ER_ROLE_CREATE_EXISTS
> +        eng "Can't create role '%-.64s'; it already exists"
> +ER_ROLE_DROP_EXISTS
> +        eng "Can't drop role '%-.64s'; it doesn't exist"

just a reminder - when merging (preferrably - rebasing) don't forget to
put these errror messages after all 10.0 error messages.

> diff --git a/sql/sp.cc b/sql/sp.cc
> index 188b311..058a617 100644
> --- a/sql/sp.cc
> +++ b/sql/sp.cc
> @@ -835,6 +835,8 @@ db_load_routine(THD *thd, stored_procedure_type type,
>  
>    thd->lex= &newlex;
>    newlex.current_select= NULL;
> +  // Resetting all the flags in create_info
> +  newlex.create_info.options= 0;

Isn't it safer to reset options in the parser, after CREATE clause?
Like

   | CREATE { Lex->create_info.options= 0; } opt_or_replace ...

Ok, see my very last comment in this review.

>    defstr.set_charset(creation_ctx->get_client_cs());
>  
> @@ -1015,6 +1063,47 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
>      ret= SP_OPEN_TABLE_FAILED;
>    else
>    {
> +    /* Checking if the routine already exists */
> +    if (db_find_routine_aux(thd, type, lex->spname, table) == SP_OK)
> +    {
> +      if (lex->is_create_or_replace())
> +      {
> +        if ((ret= sp_drop_routine_internal(thd, type, lex->spname, table)))
> +          goto done;
> +      }
> +      else if (lex->is_create_if_not_exists())
> +      {
> +        push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
> +                              ER_SP_ALREADY_EXISTS, ER(ER_SP_ALREADY_EXISTS),
> +                              type == TYPE_ENUM_FUNCTION ?
> +                               "FUNCTION" : "PROCEDURE",
> +                              lex->spname->m_name.str);
> +
> +        ret= SP_OK;
> +
> +        // Setting retstr as it is used for logging.
> +        if (sp->m_type == TYPE_ENUM_FUNCTION)
> +        {
> +          sp_returns_type(thd, retstr, sp);

this is ok

> +          store_failed= store_failed ||
> +            table->field[MYSQL_PROC_FIELD_RETURNS]->
> +              store(retstr.ptr(), retstr.length(), system_charset_info);

but why do you need to update table->field[MYSQL_PROC_FIELD_RETURNS] ?

> +        }
> +
> +        if (store_failed)
> +        {
> +          ret= SP_FLD_STORE_FAILED;
> +          goto done;
> +        }
> +        goto log;
> +      }
> +      else
> +      {
> +        ret= SP_WRITE_ROW_FAILED;
> +        goto done;
> +      }
> +    }
> +
>      restore_record(table, s->default_values); // Get default values for fields
>  
>      /* NOTE: all needed privilege checks have been already done. */
> diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
> index 5f077c2..1e156be 100644
> --- a/sql/sql_acl.cc
> +++ b/sql/sql_acl.cc
> @@ -9148,6 +9148,42 @@ end:
>    DBUG_RETURN(result);
>  }
>  
> +
> +class Warning_user
> +{
> +private:
> +  bool m_handle_as_role;
> +  void push_warning_role(THD *thd, uint err, const LEX_USER *user)
> +  {
> +    push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
> +                        err, ER(err), user->user.str);
> +  }
> +  void push_warning_user(THD *thd, uint err, const LEX_USER *user)
> +  {
> +    push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
> +                        err, ER(err), user->user.str, user->host.str);
> +  }
> +public:
> +  Warning_user(bool handle_as_role)
> +    :m_handle_as_role(handle_as_role)
> +  { }
> +  void push_create_exists(THD *thd, const LEX_USER *user)
> +  {
> +    if (m_handle_as_role)
> +      push_warning_role(thd, ER_ROLE_CREATE_EXISTS, user);
> +    else
> +      push_warning_user(thd, ER_USER_CREATE_EXISTS, user);
> +  }
> +  void push_drop_doesnt_exist(THD *thd, const LEX_USER *user)
> +  {
> +    if (m_handle_as_role)
> +      push_warning_role(thd, ER_ROLE_DROP_EXISTS, user);
> +    else
> +      push_warning_user(thd, ER_USER_DROP_EXISTS, user);
> +  }
> +};
> +
> +
>  /*
>    Create a list of users.
>  
> @@ -9215,10 +9252,26 @@ bool mysql_create_user(THD *thd, List <LEX_USER> &list, bool handle_as_role)
>      */
>      if (handle_grant_data(tables, 0, user_name, NULL))
>      {
> -      append_user(thd, &wrong_users, user_name);
> -
> -      result= TRUE;
> -      continue;
> +      if (thd->lex->is_create_or_replace())
> +      {
> +        if (handle_grant_data(tables, 1, user_name, NULL) <= 0)
> +        {
> +          append_user(thd, &wrong_users, user_name);

I don't understand that at all. If user creation failed, you try to drop
it, but don't create a user again, after the old one was dropped. How
could it work?

> +          result= TRUE;
> +          continue;
> +        }
> +      }
> +      else if (thd->lex->is_create_if_not_exists())
> +      {
> +        Warning_user(handle_as_role).push_create_exists(thd, user_name);

Eh? Why do you need a class for that?
Especially as it's temporarily instantiated here, and just to push a warning.
Please, remove it.

> +        continue;
> +      }
> +      else
> +      {
> +        append_user(thd, &wrong_users, user_name);
> +        result= TRUE;
> +        continue;
> +      }
>      }
>  
>      some_users_created= TRUE;
> @@ -9268,7 +9321,7 @@ bool mysql_create_user(THD *thd, List <LEX_USER> &list, bool handle_as_role)
>               (handle_as_role) ? "CREATE ROLE" : "CREATE USER",
>               wrong_users.c_ptr_safe());
>  
> -  if (some_users_created)
> +  if (some_users_created || !result)

What is that for? To binlog CREATE IF NOT EXISTS?

>      result |= write_bin_log(thd, FALSE, thd->query(), thd->query_length());
>  
>    mysql_rwlock_unlock(&LOCK_grant);
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index 922f0d7..648fd48 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -3855,7 +3855,9 @@ end_with_restore_list:
>        }
>      }
>  #endif
> -    if (check_access(thd, CREATE_ACL, lex->name.str, NULL, NULL, 1, 0))
> +    if (check_access(thd, lex->is_create_or_replace() ?
> +                          CREATE_ACL | DROP_ACL : CREATE_ACL,

good

> +                     lex->name.str, NULL, NULL, 1, 0))
>        break;
>      res= mysql_create_db(thd, lex->name.str, &create_info, 0);
>      break;
> @@ -3995,9 +3997,8 @@ end_with_restore_list:
>      switch (lex->sql_command) {
>      case SQLCOM_CREATE_EVENT:
>      {
> -      bool if_not_exists= (lex->create_info.options &
> -                           HA_LEX_CREATE_IF_NOT_EXISTS);
> -      res= Events::create_event(thd, lex->event_parse_data, if_not_exists);
> +      res= Events::create_event(thd, lex->event_parse_data,
> +                                lex->is_create_if_not_exists());

This doesn't seem to be particularly logical.
You pass lex->is_create_if_not_exists)() here an argument,
but you check thd->lex->is_create_or_replace() in the
Event_db_repository::create_event anyway. I'd suggest not to pass
lex->is_create_if_not_exists() as an argument, and check both in
Event_db_repository::create_event().

>        break;
>      }
>      case SQLCOM_ALTER_EVENT:
> @@ -6514,7 +6520,11 @@ bool add_field_to_list(THD *thd, LEX_STRING *field_name, enum_field_types type,
>      lex->col_list.push_back(new Key_part_spec(*field_name, 0));
>      key= new Key(Key::PRIMARY, null_lex_str,
>                        &default_key_create_info,
> -                      0, lex->col_list, NULL, lex->check_exists);
> +                      0, lex->col_list, NULL);
> +
> +    if (lex->check_exists)
> +      key->key_create_info.options = HA_LEX_CREATE_IF_NOT_EXISTS;

Hmm, is it for

   ALTER TABLE t1 ADD COLUMN IF NOT EXISTS foo INT PRIMARY KEY

?
Do you have test cases for it? What does it do if the column exists but
the key doesn't? What if the key exists and the column doens't?

> +
>      lex->alter_info.key_list.push_back(key);
>      lex->col_list.empty();
>    }
> @@ -6524,7 +6534,11 @@ bool add_field_to_list(THD *thd, LEX_STRING *field_name, enum_field_types type,
>      lex->col_list.push_back(new Key_part_spec(*field_name, 0));
>      key= new Key(Key::UNIQUE, null_lex_str,
>                   &default_key_create_info, 0,
> -                 lex->col_list, NULL, lex->check_exists);
> +                 lex->col_list, NULL);
> +
> +    if (lex->check_exists)
> +      key->key_create_info.options = HA_LEX_CREATE_IF_NOT_EXISTS;

Same, needs tests (if there aren't).

> +
>      lex->alter_info.key_list.push_back(key);
>      lex->col_list.empty();
>    }
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index 8af77d9..b10c6d7 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -5846,6 +5846,8 @@ handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info)
>        }
>        if (remove_key)
>        {

take care when merging. "if (remove_key)" is not in the current 10.1,
instead there's "remove_key:" label and goto.

> +        if (key->is_create_if_not_exists())
> +        {
>          push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE,
>              ER_DUP_KEYNAME, ER(ER_DUP_KEYNAME), keyname);
>          key_it.remove();
> diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
> index 70ac626..b337451 100644
> --- a/sql/sql_trigger.cc
> +++ b/sql/sql_trigger.cc
> @@ -607,6 +607,89 @@ end:
>    DBUG_RETURN(result);
>  }
>  
> +/**
> +  Build stmt_query to write it in the bin-log.
> +
> +  @param thd           current thread context (including trigger definition in
> +                       LEX)
> +  @param tables        table list containing one open table for which the
> +                       trigger is created.
> +  @param[out] stmt_query    after successful return, this string contains
> +                            well-formed statement for creation this trigger.
> +
> +  @note
> +    - Assumes that trigger name is fully qualified.
> +    - NULL-string means the following LEX_STRING instance:
> +    { str = 0; length = 0 }.
> +    - In other words, definer_user and definer_host should contain
> +    simultaneously NULL-strings (non-SUID/old trigger) or valid strings
> +    (SUID/new trigger).
> +
> +  @retval
> +    False   success
> +  @retval
> +    True    error
> +*/
> +static bool build_trig_stmt_query(THD *thd, TABLE_LIST *tables,
> +                                  String *stmt_query,
> +                                  LEX_STRING *trg_definer,
> +                                  char trg_definer_holder[])
> +{
> +  LEX *lex = thd->lex;
> +  LEX_STRING definer_user;
> +  LEX_STRING definer_host;
> +
> +  if (trg_definer == NULL &&
> +      !(trg_definer= alloc_lex_string(&tables->table->mem_root)))
> +    return true;
> +
> +  if (lex->sphead->m_chistics->suid != SP_IS_NOT_SUID)
> +  {
> +    /* SUID trigger. */
> +    definer_user= lex->definer->user;
> +    definer_host= lex->definer->host;
> +    lex->definer->set_lex_string(trg_definer, trg_definer_holder);
> +  }
> +  else
> +  {
> +    /* non-SUID trigger. */
> +    definer_user.str= 0;
> +    definer_user.length= 0;
> +
> +    definer_host.str= 0;
> +    definer_host.length= 0;
> +
> +    trg_definer->str= (char*) "";
> +    trg_definer->length= 0;

Why do you bother setting definer here?
You can simply do below (see below):

> +  }
> +
> +  /*
> +    Create well-formed trigger definition query. Original query is not
> +    appropriated, because definer-clause can be not truncated.
> +  */
> +
> +  stmt_query->append(STRING_WITH_LEN("CREATE "));
> +
> +  if (lex->is_create_or_replace())
> +    stmt_query->append(STRING_WITH_LEN("OR REPLACE "));
> +
> +  if (lex->sphead->m_chistics->suid != SP_IS_NOT_SUID)
> +  {
> +    /*
> +      Append definer-clause if the trigger is SUID (a usual trigger in
> +      new MySQL versions).
> +    */
> +    append_definer(thd, stmt_query, &definer_user, &definer_host);

here:

       append_definer(thd, stmt_query, &lex->definer->user, &lex->definer->host);
 
and set trg_definer:

       lex->definer->set_lex_string(trg_definer, trg_definer_holder);
     }
     else
       trg_definer= empty_lex_string;

> +  }
> +
> +  LEX_STRING stmt_definition;
> +  stmt_definition.str= (char*) thd->lex->stmt_definition_begin;
> +  stmt_definition.length= thd->lex->stmt_definition_end -
> +                          thd->lex->stmt_definition_begin;
> +  trim_whitespace(thd->charset(), & stmt_definition);
> +  stmt_query->append(stmt_definition.str, stmt_definition.length);
> +  return false;
> +}
>  
>  /**
>    Create trigger for table.
> @@ -639,16 +722,16 @@ bool Table_triggers_list::create_trigger(THD *thd, TABLE_LIST *tables,
>    char file_buff[FN_REFLEN], trigname_buff[FN_REFLEN];
>    LEX_STRING file, trigname_file;
>    LEX_STRING *trg_def;
> -  LEX_STRING definer_user;
> -  LEX_STRING definer_host;
>    ulonglong *trg_sql_mode;
>    char trg_definer_holder[USER_HOST_BUFF_SIZE];
> -  LEX_STRING *trg_definer;
> +  LEX_STRING *trg_definer= NULL;
>    Item_trigger_field *trg_field;
>    struct st_trigname trigname;
>    LEX_STRING *trg_client_cs_name;
>    LEX_STRING *trg_connection_cl_name;
>    LEX_STRING *trg_db_cl_name;
> +  sp_head *trg_head= bodies[lex->trg_chistics.event]
> +                           [lex->trg_chistics.action_time];

trg_head as an element in a bodies array?
May be trg_body instead?

>  
>    if (check_for_broken_triggers())
>      return true;
> diff --git a/sql/sql_view.cc b/sql/sql_view.cc
> index a18193c..52b534e 100644
> --- a/sql/sql_view.cc
> +++ b/sql/sql_view.cc
> @@ -858,6 +863,14 @@ static int mysql_register_view(THD *thd, TABLE_LIST *view,
>    view->definer.host= lex->definer->host;
>    view->view_suid= lex->create_view_suid;
>    view->with_check= lex->create_view_check;
> +
> +  DBUG_EXECUTE_IF("simulate_register_view_failure",
> +                  my_error(ER_OUT_OF_RESOURCES, MYF(0)););
> +  DBUG_EXECUTE_IF("simulate_register_view_failure",
> +                  error= -1;);
> +  DBUG_EXECUTE_IF("simulate_register_view_failure",
> +                  goto err;);

combine these in one DBUG_EXECUTE_IF:

  DBUG_EXECUTE_IF("simulate_register_view_failure",
        {
          my_error(ER_OUT_OF_RESOURCES, MYF(0));
          error= -1;
          goto err;
        });

> +
>    if ((view->updatable_view= (can_be_merged &&
>                                view->algorithm != VIEW_ALGORITHM_TMPTABLE)))
>    {
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index d6b3fa4..7da047f 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -752,8 +752,18 @@ bool setup_select_in_parentheses(LEX *lex)
>    return FALSE;
>  }
>  
> -static bool add_create_index_prepare (LEX *lex, Table_ident *table)
> +static bool add_create_index_prepare (LEX *lex, Table_ident *table,
> +                                      uint create_or_replace,
> +                                      uint create_if_not_exists)
>  {
> +  if (create_or_replace && create_if_not_exists)
> +  {
> +    my_error(ER_WRONG_USAGE, MYF(0), "OR REPLACE", "IF NOT EXISTS");
> +    return TRUE;
> +  }

I'd rather see this check in the opt_if_not_exists rule, in only one
place, that is.

For this to work, though, the result of create_or_replace rule must be
stored somewhere where opt_if_not_exists can access it. For example, in
Lex->create_info.options.

> +
> +  lex->key_create_info.options = (create_or_replace | create_if_not_exists);
> +
>    lex->sql_command= SQLCOM_CREATE_INDEX;
>    if (!lex->current_select->add_table_to_list(lex->thd, table, NULL,
>                                                TL_OPTION_UPDATING,

Regards,
Sergei