maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #07973
Review of MDEV-7112 Split HA_CREATE_INFO
Hi.
It think the split is good, I liked it.
I didn't like that you've tried to put the refactoring of sql_db.cc and
sql_lang.h in the same commit, please move it to a separate patch.
MDEV-5359 should not depend on that.
Anyway, see all my comments below:
> diff --git a/sql/structs.h b/sql/structs.h
> index 99561c5..b38685c 100644
> --- a/sql/structs.h
> +++ b/sql/structs.h
> @@ -471,4 +471,67 @@ class Discrete_intervals_list {
> Discrete_interval* get_current() const { return current; };
> };
>
> +
> +struct DDL_options_st
> +{
> +public:
> + enum Options
> + {
> + OPT_NONE= 0,
> + OPT_IF_NOT_EXISTS= 2, // CREATE TABLE IF NOT EXISTS
> + OPT_LIKE= 4, // CREATE TABLE LIKE
> + OPT_OR_REPLACE= 16, // CREATE OR REPLACE TABLE
> + OPT_OR_REPLACE_SLAVE_GENERATED= 32,// REPLACE was added on slave, it was
> + // not in the original query on master.
> + OPT_IF_EXISTS= 64
> + };
> +
> +private:
> + Options m_options;
> +
> +protected:
> +
> +public:
> + Options create_like_options() const
> + {
> + return (DDL_options_st::Options)
> + (((uint) m_options) & (OPT_IF_NOT_EXISTS | OPT_OR_REPLACE));
> + }
> + void init() { m_options= OPT_NONE; }
> + void init(Options options) { m_options= options; }
> + void set(Options other)
> + {
> + m_options= other;
> + }
> + void set(const DDL_options_st other)
> + {
> + m_options= other.m_options;
> + }
> + bool if_not_exists() const { return m_options & OPT_IF_NOT_EXISTS; }
> + bool or_replace() const { return m_options & OPT_OR_REPLACE; }
> + bool or_replace_slave_generated() const
> + { return m_options & OPT_OR_REPLACE_SLAVE_GENERATED; }
> + bool like() const { return m_options & OPT_LIKE; }
> + bool if_exists() const { return m_options & OPT_IF_EXISTS; }
> + void add(const DDL_options_st::Options other)
> + {
> + m_options= (Options) ((uint) m_options | (uint) other);
> + }
> + void add(const DDL_options_st &other)
> + {
> + add(other.m_options);
> + }
> + DDL_options_st operator|(const DDL_options_st &other)
> + {
> + add(other.m_options);
> + return *this;
> + }
> + DDL_options_st operator|=(DDL_options_st::Options other)
> + {
> + add(other);
> + return *this;
> + }
> +};
I don't particularly like this style of C++ing...
It's way too verbose.
but ok
> +
> +
> #endif /* STRUCTS_INCLUDED */
> diff --git a/sql/handler.h b/sql/handler.h
> index 0044556..7b6a5f4 100644
> --- a/sql/handler.h
> +++ b/sql/handler.h
> @@ -376,10 +376,7 @@ enum enum_alter_inplace_result {
> #define HA_KEY_BLOB_LENGTH 2
>
> #define HA_LEX_CREATE_TMP_TABLE 1
> -#define HA_LEX_CREATE_IF_NOT_EXISTS 2
> -#define HA_LEX_CREATE_TABLE_LIKE 4
> #define HA_CREATE_TMP_ALTER 8
> -#define HA_LEX_CREATE_REPLACE 16
One empty line here, please, HA_MAX_REC_LENGTH is not a flag
> #define HA_MAX_REC_LENGTH 65535
>
> /* Table caching type */
> @@ -1571,9 +1568,9 @@ enum enum_stats_auto_recalc { HA_STATS_AUTO_RECALC_DEFAULT= 0,
> HA_STATS_AUTO_RECALC_ON,
> HA_STATS_AUTO_RECALC_OFF };
>
Could you add structure comments here? Like what you have in MDEV,
"This will be passed to mysql_create_db, mysql_drop_db,
mysqld_show_create_db", etc
To explain what every structure is for, what kind of data it should store,
and expected users (internal DB DDL, internal table DDL, handler, etc)
> -struct HA_CREATE_INFO
> +struct Table_contents_source_st
> {
> - CHARSET_INFO *table_charset, *default_table_charset;
> + CHARSET_INFO *table_charset;
> LEX_CUSTRING tabledef_version;
> LEX_STRING connect_string;
> const char *password, *tablespace;
> @@ -1635,6 +1635,55 @@ struct HA_CREATE_INFO
> };
>
comments for all new structures, please
> +struct Schema_specification_st
> +{
> + CHARSET_INFO *default_table_charset;
> + void init()
> + {
> + bzero(this, sizeof(*this));
> + }
> +};
> +
> +
> +struct HA_CREATE_INFO: public Table_contents_source_st,
> + public Schema_specification_st
> +{
> + void init()
> + {
> + Table_contents_source_st::init();
> + Schema_specification_st::init();
> + }
> +};
> +
> +
> +struct Table_specification_st: public HA_CREATE_INFO,
> + public DDL_options_st
> +{
> + // Deep initialization
> + void init()
> + {
> + HA_CREATE_INFO::init();
> + DDL_options_st::init();
> + }
> + void init(DDL_options_st::Options options)
> + {
> + HA_CREATE_INFO::init();
> + DDL_options_st::init(options);
> + }
> + /*
> + Quick initialization, for parser.
> + Most of the HA_CREATE_INFO is left uninitialized.
> + It gets fully initialized in sql_yacc.yy, only when the parser
> + scans a related keyword (e.g. CREATE, ALTER).
> + */
> + void lex_start()
> + {
> + HA_CREATE_INFO::options= 0;
> + DDL_options_st::init();
> + }
> +};
> +
> +
> /**
> In-place alter handler context.
>
> diff --git a/sql/sql_lang.h b/sql/sql_lang.h
> new file mode 100644
> index 0000000..76a1340
> --- /dev/null
> +++ b/sql/sql_lang.h
> @@ -0,0 +1,79 @@
> +/* Copyright (c) 2014, MariaDB Foundation
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; version 2 of the License.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program; if not, write to the Free Software
> + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */
> +
> +#ifndef SQL_LANG_INCLUDED
> +#define SQL_LANG_INCLUDED
> +
> +namespace SQL {
> +
> +class Identifier: protected LEX_CSTRING
> +{
> + void set(const LEX_STRING &other)
> + {
> + str= other.str;
> + length= other.length;
> + DBUG_ASSERT(str[length] == 0);
> + }
> +public:
> + Identifier(const LEX_STRING &other)
> + {
> + set(other);
> + }
> + Identifier(const LEX *);
> + const char *name() const { return str; }
> + const size_t name_length() const { return length; }
> +};
Eh? What all this has to do with MDEV-7112 "Split HA_CREATE_INFO" ?
it's not mentioned there. If it's unrelated refactoring,
please, put it in a separate patch.
> +
> +
> +class Identifier_normalized: public Identifier
> +{
> + char m_buf[NAME_LEN + 1 + MYSQL50_TABLE_NAME_PREFIX_LENGTH + 10];
> + void normalize();
> +public:
> + Identifier_normalized(const LEX_STRING &other)
> + :Identifier(other) { normalize(); }
> + Identifier_normalized(const LEX *lex)
> + :Identifier(lex) { normalize(); }
> +};
> +
> +
> +namespace Statement {
> +
> +
> +/*
> + All SQL statements that can be executed directly.
> + Statement instances are created using the data in thd->lex,
> + previously populated by the SQL parser.
> +*/
> +class Directly_executable
> +{
> +public:
> + /*
> + Perform all stages of a direct SQL execution:
> + - Check for validity (e.g. name in "CREATE SCHEMA name" is valid).
> + - Check access
> + - Apply filters (e.g. Rpl_filters on slave)
> + - Perform WSREP_TO_ISOLATION
> + - Execute the actual action
> + */
> + virtual int exec_direct(THD *thd) const = 0;
> +};
> +
> +} // End of namescpace Statement
> +
> +
> +} // End of namespace SQL
> +
> +#endif /* SQL_LANG_INCLUDED */
> diff --git a/sql/sql_base.cc b/sql/sql_base.cc
> index 0bbcca5..0edda65 100644
> --- a/sql/sql_base.cc
> +++ b/sql/sql_base.cc
> @@ -1923,7 +1923,9 @@ bool MDL_deadlock_handler::handle_condition(THD *,
> */
>
> static bool
> -open_table_get_mdl_lock(THD *thd, Open_table_context *ot_ctx,
> +open_table_get_mdl_lock(THD *thd,
> + const DDL_options_st &options,
> + Open_table_context *ot_ctx,
What's the point - you don't seem to be using options in this function?
And that recursively means that you don't need 'options' in all other
functions that only pass 'options' argument down to open_table_get_mdl_lock()
but otherwise don't use it.
> MDL_request *mdl_request,
> uint flags,
> MDL_ticket **mdl_ticket)
> @@ -4176,8 +4182,8 @@ thr_lock_type read_lock_type_for_table(THD *thd,
> DBUG_RETURN(FALSE);
>
> /* Check if CREATE TABLE without REPLACE was used */
> - create_table= (thd->lex->sql_command == SQLCOM_CREATE_TABLE &&
> - !(thd->lex->create_info.options & HA_LEX_CREATE_REPLACE));
> + create_table= thd->lex->sql_command == SQLCOM_CREATE_TABLE &&
> + !options.or_replace();
why, options are still in thd->lex, aren't they?
>
> if (!(flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK))
> {
> diff --git a/sql/sql_db.h b/sql/sql_db.h
> index 62d379c..dc62e6b 100644
> --- a/sql/sql_db.h
> +++ b/sql/sql_db.h
> @@ -35,13 +33,178 @@ bool mysql_opt_change_db(THD *thd,
> bool my_dboptions_cache_init(void);
> void my_dboptions_cache_free(void);
> bool check_db_dir_existence(const char *db_name);
> -bool load_db_opt(THD *thd, const char *path, HA_CREATE_INFO *create);
> +bool load_db_opt(THD *thd, const char *path, Schema_specification_st *create);
> bool load_db_opt_by_name(THD *thd, const char *db_name,
> - HA_CREATE_INFO *db_create_info);
> + Schema_specification_st *db_create_info);
> CHARSET_INFO *get_default_db_collation(THD *thd, const char *db_name);
> bool my_dbopt_init(void);
> void my_dbopt_cleanup(void);
>
> +
> +namespace SQL {
> +
> +
> +namespace Statement {
> +
> +
> +namespace Schema {
> +
> +
> +// Schema name
> +class Name: public Identifier_normalized
> +{
> +public:
> + Name(const LEX_STRING &other) :Identifier_normalized(other) { }
> + Name(const LEX *lex) :Identifier_normalized(lex) { }
> +#ifdef HAVE_REPLICATION
> + bool check_if_slave_ignored(THD *thd) const;
> +#endif
> + bool check() const;
> + bool check_with_error() const;
> +};
> +
> +
> +// Schema definition options: IF EXISTS, IF NOT EXISTS
> +class DDL_options: public DDL_options_st
> +{
> +public:
> + DDL_options() { init(); }
> + DDL_options(Options options)
> + {
> + set(options);
> + }
> + DDL_options(const DDL_options_st options)
> + {
> + set(options);
> + }
> + DDL_options(const LEX *lex);
> +};
> +
> +
> +// Schema specification, e.g. CHARACTER SET xxx
> +class Specification: public Schema_specification_st
> +{
> +public:
> + Specification(const Schema_specification_st &info)
> + :Schema_specification_st(info)
> + { }
> + Specification(const LEX *lex);
> +};
> +
> +
> +/**
> + A common class for all Schema DDL options:
> + - CREATE SCHEMA
> + - DROP SCHEMA
> + - ALTER SCHEMA
> + - ALTER SCHEMA UPGRADE DATA DIRECTORY NAME
> +*/
> +class DDL: public Directly_executable
> +{
> +protected:
> + bool rm_db(THD *thd, const Name &name, bool if_exists, bool silent) const;
> +
> + int create_db(THD *thd,
> + const Name &name,
> + const DDL_options_st options,
> + const Schema_specification_st &info,
> + bool silent) const;
> +};
> +
> +
> +class DDL_with_name: public DDL, public Name
> +{
> + /*
> + Execute a DDL statement.
> + Assumes that Name has already been checked for validity.
> + */
> + virtual bool exec(THD *thd) const = 0;
> + virtual bool check_access(THD *) const = 0;
> +protected:
> + DDL_with_name(const Name &name): Name(name) { }
> + DDL_with_name(const LEX *lex): Name(lex) { }
> +public:
> + int exec_direct(THD *thd) const;
> +};
> +
> +
> +class DDL_with_name_and_options: public DDL_with_name, public DDL_options
> +{
> +protected:
> + DDL_with_name_and_options(const LEX *lex)
> + :DDL_with_name(lex), DDL_options(lex) { }
> + DDL_with_name_and_options(const Name &name)
> + :DDL_with_name(name), DDL_options() { }
> +};
> +
> +
> +class Create: public DDL_with_name_and_options, public Specification
> +{
> + bool exec(THD *thd) const
> + {
> + return DDL::create_db(thd, *this, *this, *this, false);
> + }
> + bool check_access(THD *thd) const
> + {
> + return ::check_access(thd, CREATE_ACL, name(), NULL, NULL, 1, 0);
> + }
> +public:
> + Create(const LEX *lex)
> + :DDL_with_name_and_options(lex), Specification(lex)
> + { }
> + Create(const Name &name, const Schema_specification_st &info)
> + :DDL_with_name_and_options(name), Specification(info)
> + { }
> +};
> +
> +
> +class Drop: public DDL_with_name_and_options
> +{
> + bool exec(THD *thd) const
> + {
> + return rm_db(thd, *this, if_exists(), false);
> + }
> + bool check_access(THD *thd) const
> + {
> + return ::check_access(thd, DROP_ACL, name(), NULL, NULL, 1, 0);
> + }
> +public:
> + Drop(const Name &name) :DDL_with_name_and_options(name) { }
> + Drop(const LEX *lex) :DDL_with_name_and_options(lex) { }
> +};
> +
> +
> +class Alter: public DDL_with_name, public Specification
> +{
> + bool exec(THD *thd) const;
> + bool check_access(THD *thd) const
> + {
> + return ::check_access(thd, ALTER_ACL, name(), NULL, NULL, 1, 0);
> + }
> +public:
> + Alter(const LEX *lex) :DDL_with_name(lex), Specification(lex) { }
> +};
> +
> +
> +class Upgrade: public DDL_with_name
> +{
> + bool exec(THD *thd) const;
> + bool check_access(THD *thd) const
> + {
> + return ::check_access(thd, ALTER_ACL, name(), NULL, NULL, 1, 0) ||
> + ::check_access(thd, DROP_ACL, name(), NULL, NULL, 1, 0) ||
> + ::check_access(thd, CREATE_ACL, name(), NULL, NULL, 1, 0);
> + }
> +public:
> + Upgrade(const LEX *lex) :DDL_with_name(lex) { }
> + int exec_direct(THD *thd) const;
> +};
> +
> +
> +} // End of namespace SQL::Statement::Schema
> +} // End of namespace SQL::Statement
> +} // End of namespace SQL
Please, no. This and sql_lang.h - make it a separate patch
This one should be *only* about splitting HA_CREATE_INFO
> +
> #define MY_DB_OPT_FILE "db.opt"
>
> #endif /* SQL_DB_INCLUDED */
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index b905d8b..c59332e 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -4181,143 +4176,21 @@ static bool do_execute_sp(THD *thd, sp_head *sp)
> }
> break;
> case SQLCOM_CREATE_DB:
> - {
> - /*
> - As mysql_create_db() may modify HA_CREATE_INFO structure passed to
> - it, we need to use a copy of LEX::create_info to make execution
> - prepared statement- safe.
> - */
> - HA_CREATE_INFO create_info(lex->create_info);
> - if (check_db_name(&lex->name))
> - {
> - my_error(ER_WRONG_DB_NAME, MYF(0), lex->name.str);
> - break;
> - }
> - /*
> - If in a slave thread :
> - CREATE DATABASE DB was certainly not preceded by USE DB.
> - For that reason, db_ok() in sql/slave.cc did not check the
> - do_db/ignore_db. And as this query involves no tables, tables_ok()
> - above was not called. So we have to check rules again here.
> - */
> -#ifdef HAVE_REPLICATION
> - if (thd->slave_thread)
> - {
> - rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter;
> - if (!rpl_filter->db_ok(lex->name.str) ||
> - !rpl_filter->db_ok_with_wild_table(lex->name.str))
> - {
> - my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> - break;
> - }
> - }
> -#endif
> - if (check_access(thd, CREATE_ACL, lex->name.str, NULL, NULL, 1, 0))
> - break;
> - WSREP_TO_ISOLATION_BEGIN(lex->name.str, NULL, NULL)
> - res= mysql_create_db(thd, lex->name.str, &create_info, 0);
separate refactoring patch, please
> + if ((res= SQL::Statement::Schema::Create(lex).exec_direct(thd)) < 0)
> + goto error;
> break;
> - }
> case SQLCOM_DROP_DB:
> - {
> - if (check_db_name(&lex->name))
> - {
> - my_error(ER_WRONG_DB_NAME, MYF(0), lex->name.str);
> - break;
> - }
> - /*
> - If in a slave thread :
> - DROP DATABASE DB may not be preceded by USE DB.
> - For that reason, maybe db_ok() in sql/slave.cc did not check the
> - do_db/ignore_db. And as this query involves no tables, tables_ok()
> - above was not called. So we have to check rules again here.
> - */
> -#ifdef HAVE_REPLICATION
> - if (thd->slave_thread)
> - {
> - rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter;
> - if (!rpl_filter->db_ok(lex->name.str) ||
> - !rpl_filter->db_ok_with_wild_table(lex->name.str))
> - {
> - my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> - break;
> - }
> - }
> -#endif
> - if (check_access(thd, DROP_ACL, lex->name.str, NULL, NULL, 1, 0))
> - break;
> - WSREP_TO_ISOLATION_BEGIN(lex->name.str, NULL, NULL)
> - res= mysql_rm_db(thd, lex->name.str, lex->check_exists, 0);
> + if ((res= SQL::Statement::Schema::Drop(lex).exec_direct(thd)) < 0)
> + goto error;
> break;
> - }
> case SQLCOM_ALTER_DB_UPGRADE:
> - {
> - LEX_STRING *db= & lex->name;
> -#ifdef HAVE_REPLICATION
> - if (thd->slave_thread)
> - {
> - rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter;
> - if (!rpl_filter->db_ok(db->str) ||
> - !rpl_filter->db_ok_with_wild_table(db->str))
> - {
> - res= 1;
> - my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> - break;
> - }
> - }
> -#endif
> - if (check_db_name(db))
> - {
> - my_error(ER_WRONG_DB_NAME, MYF(0), db->str);
> - break;
> - }
> - if (check_access(thd, ALTER_ACL, db->str, NULL, NULL, 1, 0) ||
> - check_access(thd, DROP_ACL, db->str, NULL, NULL, 1, 0) ||
> - check_access(thd, CREATE_ACL, db->str, NULL, NULL, 1, 0))
> - {
> - res= 1;
> - break;
> - }
> - WSREP_TO_ISOLATION_BEGIN(db->str, NULL, NULL)
> - res= mysql_upgrade_db(thd, db);
> - if (!res)
> - my_ok(thd);
> + if ((res= SQL::Statement::Schema::Upgrade(lex).exec_direct(thd)) < 0)
> + goto error;
> break;
> - }
> case SQLCOM_ALTER_DB:
> - {
> - LEX_STRING *db= &lex->name;
> - HA_CREATE_INFO create_info(lex->create_info);
> - if (check_db_name(db))
> - {
> - my_error(ER_WRONG_DB_NAME, MYF(0), db->str);
> - break;
> - }
> - /*
> - If in a slave thread :
> - ALTER DATABASE DB may not be preceded by USE DB.
> - For that reason, maybe db_ok() in sql/slave.cc did not check the
> - do_db/ignore_db. And as this query involves no tables, tables_ok()
> - above was not called. So we have to check rules again here.
> - */
> -#ifdef HAVE_REPLICATION
> - if (thd->slave_thread)
> - {
> - rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter;
> - if (!rpl_filter->db_ok(db->str) ||
> - !rpl_filter->db_ok_with_wild_table(db->str))
> - {
> - my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> - break;
> - }
> - }
> -#endif
> - if (check_access(thd, ALTER_ACL, db->str, NULL, NULL, 1, 0))
> - break;
> - WSREP_TO_ISOLATION_BEGIN(db->str, NULL, NULL)
> - res= mysql_alter_db(thd, db->str, &create_info);
> + if ((res= SQL::Statement::Schema::Alter(lex).exec_direct(thd)) < 0)
> + goto error;
> break;
> - }
> case SQLCOM_SHOW_CREATE_DB:
> {
> char db_name_buff[NAME_LEN+1];
> diff --git a/sql/sql_show.cc b/sql/sql_show.cc
> index 0541cbc..2536255 100644
> --- a/sql/sql_show.cc
> +++ b/sql/sql_show.cc
> @@ -1696,14 +1695,14 @@ int show_create_table(THD *thd, TABLE_LIST *table_list, String *packet,
>
> packet->append(STRING_WITH_LEN("CREATE "));
> if (create_info_arg &&
> - (create_info_arg->org_options & HA_LEX_CREATE_REPLACE ||
> + ((create_info_arg->or_replace() &&
> + !create_info_arg->or_replace_slave_generated()) ||
that's a bit strange. Could you explain this?
why org_options & HA_LEX_CREATE_REPLACE is translated to
or_replace() && !or_replace_slave_generated()
> create_info_arg->table_was_deleted))
> packet->append(STRING_WITH_LEN("OR REPLACE "));
> if (share->tmp_table)
> packet->append(STRING_WITH_LEN("TEMPORARY "));
> packet->append(STRING_WITH_LEN("TABLE "));
> - if (create_info_arg &&
> - (create_info_arg->options & HA_LEX_CREATE_IF_NOT_EXISTS))
> + if (create_info_arg && create_info_arg->if_not_exists())
> packet->append(STRING_WITH_LEN("IF NOT EXISTS "));
> if (table_list->schema_table)
> alias= table_list->schema_table->table_name;
> diff --git a/sql/sql_table.cc b/sql/sql_table.cc
> index b991215..7bb293b 100644
> --- a/sql/sql_table.cc
> +++ b/sql/sql_table.cc
> @@ -4628,7 +4628,8 @@ int create_table_impl(THD *thd,
> const char *orig_db, const char *orig_table_name,
> const char *db, const char *table_name,
> const char *path,
> - HA_CREATE_INFO *create_info,
> + const DDL_options_st options,
> + HA_CREATE_INFO &create_info,
why? I generally prefer pointers over C++ references -
one can immediately see what variables can be modified by the function.
is there a compelling reason to use references here?
> Alter_info *alter_info,
> int create_table_mode,
> bool *is_trans,
> @@ -5273,9 +5275,10 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table,
> */
>
> /* Copy temporarily the statement flags to thd for lock_table_names() */
> + // QQ: is this really needed???
may be not anymore, as you pass flags separately
> uint save_thd_create_info_options= thd->lex->create_info.options;
> - thd->lex->create_info.options|= create_info->options;
> - res= open_tables(thd, &thd->lex->query_tables, ¬_used, 0);
> + thd->lex->create_info.options|= create_info.options;
> + res= open_tables(thd, create_info, &thd->lex->query_tables, ¬_used, 0);
> thd->lex->create_info.options= save_thd_create_info_options;
>
> if (res)
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index 632bdd2..0f2c0e0 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -1621,7 +1623,8 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize);
> IDENT_sys TEXT_STRING_sys TEXT_STRING_literal
> NCHAR_STRING opt_component key_cache_name
> sp_opt_label BIN_NUM label_ident TEXT_STRING_filesystem ident_or_empty
> - opt_constraint constraint opt_ident opt_if_not_exists_ident
> + opt_constraint constraint opt_ident
> + opt_if_not_exists_opt_table_element_name
can you please try to come up with shorter identifier names?
40 characters - that's a bit too long :)
>
> %type <lex_str_ptr>
> opt_table_alias
> diff --git a/storage/xtradb/handler/ha_innodb.cc b/storage/xtradb/handler/ha_innodb.cc
> index f3b1167..6b1942e 100644
> --- a/storage/xtradb/handler/ha_innodb.cc
> +++ b/storage/xtradb/handler/ha_innodb.cc
> @@ -11349,7 +11349,7 @@ static __attribute__((nonnull, warn_unused_result))
>
> /* Do not use DATA DIRECTORY with TEMPORARY TABLE. */
> if (create_info->data_file_name
> - && create_info->options & HA_LEX_CREATE_TMP_TABLE) {
> + && create_info->tmp_table()) {
nope, you should not change storage engine API here.
tmp_table() was a convenience wrapper, old way should still work
> push_warning(
> thd, Sql_condition::WARN_LEVEL_WARN,
> ER_ILLEGAL_HA_CREATE_OPTION,
Regards,
Sergei
Follow ups
