Hi Sergei!

On Wed, Mar 27, 2019 at 6:08 PM Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Aleksey!

On Mar 27, Aleksey Midenkov wrote:
> revision-id: 9976699033c (mariadb-10.3.7-30-g9976699033c)
> parent(s): c0f97710582
> author: Aleksey Midenkov <midenok@gmail.com>
> committer: Aleksey Midenkov <midenok@gmail.com>
> timestamp: 2018-05-30 13:12:47 +0300
> message:
>
> Test fixes (versioning.replace, versioning.foreign)
>
> * replace.test: unwrapped create_table procedure;
> * foreign.test: use key_type combinations.
>
> diff --git a/mysql-test/suite/versioning/key_type.combinations b/mysql-test/suite/versioning/key_type.combinations
> index 1929aee9a84..fb3888d3d0f 100644
> --- a/mysql-test/suite/versioning/key_type.combinations
> +++ b/mysql-test/suite/versioning/key_type.combinations
> @@ -1,2 +1,3 @@
>  [unique]
> -[pk]
> +[primary]
> +[secondary]

generally prefer shorter combination names, otherwise mtr output becomes
garbled and difficult to read.

Fixed.
 

> diff --git a/mysql-test/suite/versioning/primary_or_unique.inc b/mysql-test/suite/versioning/primary_or_unique.inc
> new file mode 100644
> index 00000000000..199d8bab942
> --- /dev/null
> +++ b/mysql-test/suite/versioning/primary_or_unique.inc
> @@ -0,0 +1,5 @@
> +--source suite/versioning/key_type.inc
> +--require suite/versioning/primary_or_unique.require

don't do .require, please, it's a technique from 1998,
before mysqltest had the "skip" command. Use

  if (...) { skip; }

Fixed.
 

> +disable_query_log;
> +eval select "$KEY_TYPE" != "key" as pk_or_unique;
> +enable_query_log;
> diff --git a/mysql-test/suite/versioning/t/engines.combinations b/mysql-test/suite/versioning/t/engines.combinations
> deleted file mode 100644
> index 561c5656929..00000000000
> --- a/mysql-test/suite/versioning/t/engines.combinations
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -[timestamp]
> -default-storage-engine=innodb
> -
> -[trx_id]
> -default-storage-engine=innodb
> -
> -[myisam]
> -default-storage-engine=myisam

Is that intentional? I thought many tests use it to run in different
versioning modes.

Yes, it's redundant t/engines.combinations, while the used one resides in versioning/.
 

> diff --git a/mysql-test/suite/versioning/t/foreign.test b/mysql-test/suite/versioning/t/foreign.test
> index 566d481c2a8..72e5b047cbb 100644
> --- a/mysql-test/suite/versioning/t/foreign.test
> +++ b/mysql-test/suite/versioning/t/foreign.test
> @@ -1,11 +1,14 @@
> +--source suite/versioning/primary_only.inc
>  --source suite/versioning/common.inc

what's the point of this? if you're only allowing primary key here, you
can just as well write manually "primary key" in all tests, without
$KEY_TYPE and replace_result.

It's a preparation for the following commit. Made a bit different.
 

Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org


--
All the best,

Aleksey Midenkov
@midenok