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
_______________________________________________
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
