Hi, Jan! On Mar 08, Jan Lindström wrote:

revision-id: a1c4620646d43eaff979c132a57ee2101070fa5e parent(s): 1b74f32b1d94465f6d6e14e4aa7ba1f94c32e39e committer: Jan Lindström branch nick: 10.1-innodb timestamp: 2015-03-08 17:23:05 +0200 message:

MDEV-6858: enforce_storage_engine option

Merge from Percona Server enforced use of a specific storage engine authored by Stewart Smith.

Modified to be session variable and modifiable only by SUPER. Use similar implementation as default_storage_engine.

diff --git a/mysql-test/t/enforce_storage_engine.test b/mysql-test/t/enforce_storage_engine.test new file mode 100644 index 0000000..36231e6 --- /dev/null +++ b/mysql-test/t/enforce_storage_engine.test @@ -0,0 +1,35 @@ ... +--error ER_UNKNOWN_STORAGE_ENGINE +CREATE TABLE t2 (c1 INT PRIMARY KEY AUTO_INCREMENT, c2 VARCHAR(10)) ENGINE=MyISAM; + +SET SESSION enforce_storage_engine=MyISAM; +select @@session.enforce_storage_engine; +select * from t1; +drop table t1; + +--error 1286 +SET SESSION enforce_storage_engine=FooBar; + +select @@session.enforce_storage_engine;

Consider adding tests for "only modifiable by SUPER" (create a new user, connect with it, see that enforce_storage_engine cannot be changed), and for modified enforce_storage_engine (you tested that it can be changed, but didn't do any CREATE TABLE with the new value).

diff --git a/sql/handler.cc b/sql/handler.cc index 12d7ffb..5d5ac30 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -117,6 +117,15 @@ static plugin_ref ha_default_tmp_plugin(THD *thd) return ha_default_plugin(thd); }

+ +static plugin_ref ha_enforced_plugin(THD *thd) +{ + if (thd->variables.enforced_table_plugin) + return thd->variables.enforced_table_plugin; + return NULL; +} + + /** @brief Return the default storage engine handlerton for thread

@@ -148,6 +157,25 @@ handlerton *ha_default_tmp_handlerton(THD *thd)

/** @brief + Return the enforced storage engine handlerton for thread + + SYNOPSIS + ha_enforce_handlerton(thd) + thd current thread + + RETURN + pointer to handlerton OR NULL + +*/ +handlerton *ha_enforced_handlerton(THD *thd) +{ + plugin_ref plugin= ha_enforced_plugin(thd); + if (plugin) + return plugin_hton(plugin); + return NULL; +} + +/** @brief Return the storage engine handlerton for the supplied name

SYNOPSIS diff --git a/sql/mysqld.cc b/sql/mysqld.cc index b97742d..6966fd9 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4738,8 +4738,8 @@ static void add_file_to_crash_report(char *file) #define init_default_storage_engine(X,Y) \ init_default_storage_engine_impl(#X, X, &global_system_variables.Y)

-static int init_default_storage_engine_impl(const char *opt_name, - char *engine_name, plugin_ref *res) +int init_default_storage_engine_impl(const char *opt_name, + char *engine_name, plugin_ref *res)

You've made init_default_storage_engine_impl() extern, but you never use it anywhere. And you've forgot to create a command line option for enforced_table_plugin.

{ if (!engine_name) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index e9a1ae9..184c073 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -9745,12 +9745,24 @@ static bool check_engine(THD *thd, const char *db_name, DBUG_ENTER("check_engine"); handlerton **new_engine= &create_info->db_type; handlerton *req_engine= *new_engine; + handlerton *enf_engine= ha_enforced_handlerton(thd);

I'd write it directly as thd->variables.enforced_table_plugin ? plugin_hton(thd->variables.enforced_table_plugin) : NULL; Or as an inlined function here, in sql_table.cc. Not as two small but non-inlined functions in handler.cc, that are only used once here.

bool no_substitution= thd->variables.sql_mode & MODE_NO_ENGINE_SUBSTITUTION; *new_engine= ha_checktype(thd, req_engine, no_substitution); DBUG_ASSERT(*new_engine); if (!*new_engine) DBUG_RETURN(true);

+ if (enf_engine) + { + if (enf_engine != *new_engine && no_substitution) + { + const char *engine_name= ha_resolve_storage_engine_name(req_engine); + my_error(ER_UNKNOWN_STORAGE_ENGINE, MYF(0), engine_name, engine_name); + DBUG_RETURN(TRUE); + } + *new_engine= enf_engine; + } + if (req_engine && req_engine != *new_engine) { push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 8371df0..cd53714 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -3437,6 +3437,28 @@ static Sys_var_plugin Sys_default_tmp_storage_engine( SESSION_VAR(tmp_table_plugin), NO_CMD_LINE, MYSQL_STORAGE_ENGINE_PLUGIN, DEFAULT(&default_tmp_storage_engine));

+extern int init_default_storage_engine_impl(const char* opt_name, char *engine_name, plugin_ref *plugin);

Not used below.

+ + +static bool check_super_and_not_null(sys_var *self, THD*thd, set_var *var)

I think that NULL value should be allowed, it means "don't enforce a storage engine".

+{ +#ifndef NO_EMBEDDED_ACCESS_CHECKS + if (!(thd->security_ctx->master_access & SUPER_ACL)) + { + my_error(ER_SPECIFIC_ACCESS_DENIED_ERROR, MYF(0), "SUPER"); + return true; + } +#endif + return var->value && var->value->is_null(); +} + +static Sys_var_plugin Sys_enforce_storage_engine( + "enforce_storage_engine", "Force the use of a storage engine for new " + "tables", + SESSION_VAR(enforced_table_plugin), + NO_CMD_LINE, MYSQL_STORAGE_ENGINE_PLUGIN, + DEFAULT(0), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_super_and_not_null)); + #if defined(ENABLED_DEBUG_SYNC) /* Variable can be set for the session only. Regards, Sergei