← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 828a7b6: MDEV-6858: enforce_storage_engine option

 

Hi, Jan!

This looks good, thanks!

There's just one strange thing I've noticed, see
sysvars_server_notembedded test below. I don't understand why it's
happening, could you investigate, please?

On Mar 10, Jan Lindström wrote:
> revision-id: 828a7b67768c7e7a72c2a687f5a3ff77bc61536b
> parent(s): f7d196bb2e605766668895027c48728f03803734
> committer: Jan Lindström
> branch nick: 10.1-innodb
> timestamp: 2015-03-10 08:28:51 +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/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result
> index 83c0196..bf8f868 100644
> --- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result
> +++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result
> @@ -721,6 +721,20 @@ NUMERIC_BLOCK_SIZE	NULL
>  ENUM_VALUE_LIST	OFF,ON
>  READ_ONLY	NO
>  COMMAND_LINE_ARGUMENT	OPTIONAL
> +VARIABLE_NAME	ENFORCE_STORAGE_ENGINE
> +SESSION_VALUE	
> +GLOBAL_VALUE	NULL

This looks strange. Why session value is empty string, but global value
is NULL? I believe they should be identical, shouldn't they?

Preferrably, both should be empty string - I don't think we have system
variables that can be NULL.

> +GLOBAL_VALUE_ORIGIN	COMPILE-TIME
> +DEFAULT_VALUE	NULL
> +VARIABLE_SCOPE	SESSION ONLY
> +VARIABLE_TYPE	VARCHAR
> +VARIABLE_COMMENT	Force the use of a storage engine for new tables
> +NUMERIC_MIN_VALUE	NULL
> +NUMERIC_MAX_VALUE	NULL
> +NUMERIC_BLOCK_SIZE	NULL
> +ENUM_VALUE_LIST	NULL
> +READ_ONLY	NO
> +COMMAND_LINE_ARGUMENT	NULL
>  VARIABLE_NAME	ERROR_COUNT
>  SESSION_VALUE	0
>  GLOBAL_VALUE	NULL
> diff --git a/mysql-test/t/enforce_storage_engine.test b/mysql-test/t/enforce_storage_engine.test
> new file mode 100644
> index 0000000..80839e2
> --- /dev/null
> +++ b/mysql-test/t/enforce_storage_engine.test
> @@ -0,0 +1,64 @@
> +-- source include/have_innodb.inc

You can keep it this way, but I'd recommend to change to either
have_xtradb.inc or (better) not use innodb at all and test with myisam
and memory engines.

with have_innodb.inc the test will be run twice - for innodb and for
xtradb, which is a waste of time, because it doesn't do anything
engine-specific here.

> +-- source include/not_embedded.inc
> +
> +--disable_warnings
> +drop table if exists t1;
> +--enable_warnings

Regards,
Sergei


Follow ups