Hi,

Sergey, thanks for the review, I’ve made the changes as you suggested.
 
>Вторник, 17 марта 2020, 18:00 +03:00 от Sergey Bronnikov < sergeyb@tarantool.org >:
> 
>Hi,
>
>comments inline
>
>On 19:19 Mon 16 Mar , Alexander V. Tikhonov wrote:
>> Divided into tests:
>> - box/ddl_alter.test.lua
>> - box/ddl_collation.test.lua
>> - box/ddl_collation_types.test.lua
>> - box/ddl_collation_wrong_id.test.lua
>> - box/ddl_no_collation.test.lua
>> - box/ddl_parallel.test.lua
>> - box/ddl_tuple.test.lua
>> - box/gh-2336-ddl_call_twice.test.lua
>> - box/gh-2783-ddl_lock.test.lua
>> - box/gh-2839-ddl_custom_fields.test.lua
>> - box/gh-2937-ddl_collation_field_def.test.lua
>> - box/gh-3290-ddl_collation_deleted.test.lua
>> - box/gh-928-ddl_truncate.test.lua
>> ---
>>
>> Github:  https://github.com/tarantool/tarantool/tree/avtikhon/divide_tests
>
>> diff --git a/test/box/ddl_alter.test.lua b/test/box/ddl_alter.test.lua
>> new file mode 100644
>> index 000000000..84c25abdb
>> --- /dev/null
>> +++ b/test/box/ddl_alter.test.lua
>> @@ -0,0 +1,12 @@
>> +env = require('test_run')
>> +test_run = env.new()
>
>unused variables
Removed.
>
>> +-- index should not crash after alter
>> +space = box.schema.space.create('test_swap')
>> +index = space:create_index('pk')
>> +space:replace({1, 2, 3})
>> +index:rename('primary')
>> +index2 = space:create_index('sec')
>> +space:replace({2, 3, 1})
>> +space:select()
>> +space:drop()
>
>> diff --git a/test/box/ddl_collation_types.test.lua b/test/box/ddl_collation_types.test.lua
>> new file mode 100644
>> index 000000000..478530537
>> --- /dev/null
>> +++ b/test/box/ddl_collation_types.test.lua
>> @@ -0,0 +1,9 @@
>> +env = require('test_run')
>> +test_run = env.new()
>
>unused variables
Removed.
>> +-- Check that collation is allowed only for strings, scalar and any types.
>> +format = {}
>> +format[1] = {'field1', 'unsigned', collation = 'unicode'}
>> +s = box.schema.create_space('test', {format = format})
>> +format[1] = {'field2', 'array', collation = 'unicode_ci'}
>> +s = box.schema.create_space('test', {format = format})
>
>missed cleanup here
Not needed due to non of the spaces created.
>
>> diff --git a/test/box/ddl_collation_wrong_id.test.lua b/test/box/ddl_collation_wrong_id.test.lua
>> new file mode 100644
>> index 000000000..0aaab322d
>> --- /dev/null
>> +++ b/test/box/ddl_collation_wrong_id.test.lua
>> @@ -0,0 +1,10 @@
>> +env = require('test_run')
>> +test_run = env.new()
>
>unused variables
Removed.
>> +-- Check that error is raised when collation with wrong id is used.
>> +_space = box.space[box.schema.SPACE_ID]
>> +utils = require('utils')
>> +EMPTY_MAP = utils.setmap({})
>> +format = {{name = 'field1', type = 'string', collation = 666}}
>> +surrogate_space = {12345, 1, 'test', 'memtx', 0, EMPTY_MAP, format}
>> +_space:insert(surrogate_space)
>
>missed cleanup
Neither the original code had cleanup neither it could be added here, because of the error:
 - error: Can't drop the primary key in a system space, space '_space'
>
>> diff --git a/test/box/ddl_no_collation.test.lua b/test/box/ddl_no_collation.test.lua
>> new file mode 100644
>> index 000000000..a5fdedd02
>> --- /dev/null
>> +++ b/test/box/ddl_no_collation.test.lua
>> @@ -0,0 +1,7 @@
>> +env = require('test_run')
>> +test_run = env.new()
>> +
>> +-- Check that error is raised when collation doesn't exists.
>> +format = {}
>> +format[1] = {'field1', 'unsigend', collation = 'test_coll'}
>> +s = box.schema.create_space('test', {format = format})
>
>> diff --git a/test/box/ddl_parallel.test.lua b/test/box/ddl_parallel.test.lua
>> new file mode 100644
>> index 000000000..99c3bfdcc
>> --- /dev/null
>> +++ b/test/box/ddl_parallel.test.lua
>> @@ -0,0 +1,30 @@
>> +env = require('test_run')
>> +test_run = env.new()
>> +
>> +fiber = require'fiber'
>> +
>> +-- simple test for parallel ddl execution
>> +_ = box.schema.space.create('test'):create_index('pk')
>> +
>> +ch = fiber.channel(2)
>> +
>> +test_run:cmd("setopt delimiter ';'")
>> +
>> +function f1()
>> + box.space.test:create_index('sec', {parts = {2, 'num'}})
>> + ch:put(true)
>> +end;
>> +
>> +function f2()
>> + box.space.test:create_index('third', {parts = {3, 'string'}})
>> + ch:put(true)
>> +end;
>> +
>> +test_run:cmd("setopt delimiter ''");
>> +
>> +_ = {fiber.create(f1), fiber.create(f2)}
>> +
>> +ch:get()
>> +ch:get()
>> +
>> +_ = box.space.test:drop()
>> diff --git a/test/box/ddl_tuple.result b/test/box/ddl_tuple.result
>> new file mode 100644
>> index 000000000..6a024a833
>> --- /dev/null
>> +++ b/test/box/ddl_tuple.result
>> @@ -0,0 +1,67 @@
>> +-- test-run result file version 2
>> +env = require('test_run')
>> + | ---
>> + | ...
>> +test_run = env.new()
>> + | ---
>> + | ...
>> +
>> +fiber = require'fiber'
>> + | ---
>> + | ...
>> +ch = fiber.channel(3)
>> + | ---
>> + | ...
>> +
>> +_ = box.schema.space.create('test'):create_index('pk')
>> + | ---
>> + | ...
>> +
>> +test_run:cmd("setopt delimiter ';'")
>> + | ---
>> + | - true
>> + | ...
>> +function add_index()
>> + box.space.test:create_index('sec', {parts = {2, 'num'}})
>> + ch:put(true)
>> +end;
>> + | ---
>> + | ...
>> +
>> +function insert_tuple(tuple)
>> + ch:put({pcall(box.space.test.replace, box.space.test, tuple)})
>> +end;
>> + | ---
>> + | ...
>> +test_run:cmd("setopt delimiter ''");
>> + | ---
>> + | - true
>> + | ...
>> +
>> +_ = {fiber.create(insert_tuple, {1, 2, 'a'}), fiber.create(add_index), fiber.create(insert_tuple, {2, '3', 'b'})}
>> + | ---
>> + | ...
>> +{ch:get(), ch:get(), ch:get()}
>> + | ---
>> + | - - - false
>> + | - 'Tuple field 2 type does not match one required by operation: expected unsigned'
>> + | - - true
>> + | - [1, 2, 'a']
>> + | - true
>> + | ...
>> +
>> +box.space.test:select()
>> + | ---
>> + | - - [1, 2, 'a']
>> + | ...
>> +
>> +test_run:cmd('restart server default')
>> + |
>> +
>> +box.space.test:select()
>> + | ---
>> + | - - [1, 2, 'a']
>> + | ...
>> +box.space.test:drop()
>> + | ---
>> + | ...
>> diff --git a/test/box/ddl_tuple.test.lua b/test/box/ddl_tuple.test.lua
>> new file mode 100644
>> index 000000000..1c78a00e4
>> --- /dev/null
>> +++ b/test/box/ddl_tuple.test.lua
>> @@ -0,0 +1,28 @@
>> +env = require('test_run')
>> +test_run = env.new()
>> +
>> +fiber = require'fiber'
>> +ch = fiber.channel(3)
>> +
>> +_ = box.schema.space.create('test'):create_index('pk')
>> +
>> +test_run:cmd("setopt delimiter ';'")
>> +function add_index()
>> + box.space.test:create_index('sec', {parts = {2, 'num'}})
>> + ch:put(true)
>> +end;
>> +
>> +function insert_tuple(tuple)
>> + ch:put({pcall(box.space.test.replace, box.space.test, tuple)})
>> +end;
>> +test_run:cmd("setopt delimiter ''");
>> +
>> +_ = {fiber.create(insert_tuple, {1, 2, 'a'}), fiber.create(add_index), fiber.create(insert_tuple, {2, '3', 'b'})}
>> +{ch:get(), ch:get(), ch:get()}
>> +
>> +box.space.test:select()
>> +
>> +test_run:cmd('restart server default')
>> +
>> +box.space.test:select()
>> +box.space.test:drop()
>> diff --git a/test/box/gh-2336-ddl_call_twice.result b/test/box/gh-2336-ddl_call_twice.result
>> new file mode 100644
>> index 000000000..aa4dee54b
>> --- /dev/null
>> +++ b/test/box/gh-2336-ddl_call_twice.result
>> @@ -0,0 +1,51 @@
>> +-- test-run result file version 2
>> +env = require('test_run')
>> + | ---
>> + | ...
>> +test_run = env.new()
>> + | ---
>> + | ...
>> +
>> +-- gh-2336 crash if format called twice during snapshot
>> +fiber = require'fiber'
>> + | ---
>> + | ...
>> +
>> +space = box.schema.space.create('test_format')
>> + | ---
>> + | ...
>> +_ = space:create_index('pk', { parts = { 1,'str' }})
>> + | ---
>> + | ...
>> +space:format({{ name ="key"; type = "string" }, { name ="dataAB"; type = "string" }})
>> + | ---
>> + | ...
>> +str = string.rep("t",1024)
>> + | ---
>> + | ...
>> +for i = 1, 10000 do space:insert{tostring(i), str} end
>> + | ---
>> + | ...
>> +ch = fiber.channel(3)
>> + | ---
>> + | ...
>> +_ = fiber.create(function() fiber.yield() box.snapshot() ch:put(true) end)
>> + | ---
>> + | ...
>> +format = {{name ="key"; type = "string"}, {name ="data"; type = "string"}}
>> + | ---
>> + | ...
>> +for i = 1, 2 do fiber.create(function() fiber.yield() space:format(format) ch:put(true) end) end
>> + | ---
>> + | ...
>> +
>> +{ch:get(), ch:get(), ch:get()}
>> + | ---
>> + | - - true
>> + | - true
>> + | - true
>> + | ...
>> +
>> +space:drop()
>> + | ---
>> + | ...
>> diff --git a/test/box/gh-2336-ddl_call_twice.test.lua b/test/box/gh-2336-ddl_call_twice.test.lua
>> new file mode 100644
>> index 000000000..cf187c1bd
>> --- /dev/null
>> +++ b/test/box/gh-2336-ddl_call_twice.test.lua
>> @@ -0,0 +1,19 @@
>> +env = require('test_run')
>> +test_run = env.new()
>> +
>> +-- gh-2336 crash if format called twice during snapshot
>> +fiber = require'fiber'
>> +
>> +space = box.schema.space.create('test_format')
>> +_ = space:create_index('pk', { parts = { 1,'str' }})
>> +space:format({{ name ="key"; type = "string" }, { name ="dataAB"; type = "string" }})
>> +str = string.rep("t",1024)
>> +for i = 1, 10000 do space:insert{tostring(i), str} end
>> +ch = fiber.channel(3)
>> +_ = fiber.create(function() fiber.yield() box.snapshot() ch:put(true) end)
>> +format = {{name ="key"; type = "string"}, {name ="data"; type = "string"}}
>> +for i = 1, 2 do fiber.create(function() fiber.yield() space:format(format) ch:put(true) end) end
>> +
>> +{ch:get(), ch:get(), ch:get()}
>> +
>> +space:drop()
>
>
>> diff --git a/test/box/gh-2783-ddl_lock.test.lua b/test/box/gh-2783-ddl_lock.test.lua
>> new file mode 100644
>> index 000000000..953e177a7
>> --- /dev/null
>> +++ b/test/box/gh-2783-ddl_lock.test.lua
>> @@ -0,0 +1,33 @@
>> +env = require('test_run')
>> +test_run = env.new()
>> +
>> +--
>> +-- gh-2783
>> +-- A ddl operation shoud fail before trying to lock a ddl latch
>> +-- in a multi-statement transaction.
>> +-- If operation tries to lock already an locked latch then the
>> +-- current transaction will be silently rolled back under our feet.
>> +-- This is confusing. So check for multi-statement transaction
>> +-- before locking the latch.
>> +--
>> +test_latch = box.schema.space.create('test_latch')
>> +_ = test_latch:create_index('primary', {unique = true, parts = {1, 'unsigned'}})
>> +fiber = require('fiber')
>> +c = fiber.channel(1)
>> +test_run:cmd("setopt delimiter ';'")
>> +_ = fiber.create(function()
>> + test_latch:create_index("sec", {unique = true, parts = {2, 'unsigned'}})
>> + c:put(true)
>> +end);
>> +
>> +-- Should be Ok for now
>> +box.begin()
>> + test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}})
>> +box.commit();
>> +test_run:cmd("setopt delimiter ''");
>> +-- Explicitly roll back the transaction in multi-statement,
>> +-- which hasn't finished due to DDL error
>> +box.rollback()
>> +
>> +_ = c:get()
>> +test_latch:drop() -- this is where everything stops
>
>
>> diff --git a/test/box/gh-2839-ddl_custom_fields.test.lua b/test/box/gh-2839-ddl_custom_fields.test.lua
>> new file mode 100644
>> index 000000000..5f0cb243f
>> --- /dev/null
>> +++ b/test/box/gh-2839-ddl_custom_fields.test.lua
>> @@ -0,0 +1,13 @@
>> +env = require('test_run')
>> +test_run = env.new()
>
>unused variables
Removed.
>
>> +-- gh-2839: allow to store custom fields in field definition.
>> +--
>> +format = {}
>> +format[1] = {name = 'field1', type = 'unsigned'}
>> +format[2] = {'field2', 'unsigned'}
>> +format[3] = {'field3', 'unsigned', custom_field = 'custom_value'}
>> +s = box.schema.create_space('test', {format = format})
>> +s:format()[3].custom_field
>> +s:drop()
>
>
>> diff --git a/test/box/gh-2937-ddl_collation_field_def.test.lua b/test/box/gh-2937-ddl_collation_field_def.test.lua
>> new file mode 100644
>> index 000000000..0906afebd
>> --- /dev/null
>> +++ b/test/box/gh-2937-ddl_collation_field_def.test.lua
>> @@ -0,0 +1,13 @@
>> +env = require('test_run')
>> +test_run = env.new()
>
>unused variables
Removed.
>> +
>> +--
>> +-- gh-2937: allow to specify collation in field definition.
>> +--
>> +format = {}
>> +format[1] = {name = 'field1', type = 'string', collation = 'unicode'}
>> +format[2] = {'field2', 'any', collation = 'unicode_ci'}
>> +format[3] = {type = 'scalar', name = 'field3', collation = 'unicode'}
>> +s = box.schema.create_space('test', {format = format})
>> +s:format()
>> +s:drop()
>
>> diff --git a/test/box/gh-3290-ddl_collation_deleted.test.lua b/test/box/gh-3290-ddl_collation_deleted.test.lua
>> new file mode 100644
>> index 000000000..f36838e7a
>> --- /dev/null
>> +++ b/test/box/gh-3290-ddl_collation_deleted.test.lua
>> @@ -0,0 +1,10 @@
>> +env = require('test_run')
>> +test_run = env.new()
>
>unused variables
Removed.
>> +--
>> +-- gh-3290: expose ICU into Lua. It uses built-in collations, that
>> +-- must work even if a collation is deleted from _collation.
>> +--
>> +t = box.space._collation:delete{1}
>> +utf8.cmp('abc', 'def')
>> +box.space._collation:replace(t)
>
>missed cleanup
Seems that no need in cleanup here.
>
>
>> diff --git a/test/box/gh-928-ddl_truncate.test.lua b/test/box/gh-928-ddl_truncate.test.lua
>> new file mode 100644
>> index 000000000..304cc2cfc
>> --- /dev/null
>> +++ b/test/box/gh-928-ddl_truncate.test.lua
>> @@ -0,0 +1,17 @@
>> +env = require('test_run')
>> +test_run = env.new()
>
>unused variables
Removed.
>> +fiber = require'fiber'
>> +ch = fiber.channel(2)
>> +
>> +--issue #928
>> +space = box.schema.space.create('test_trunc')
>> +_ = space:create_index('pk')
>> +_ = box.space.test_trunc:create_index('i1', {type = 'hash', parts = {2, 'STR'}})
>> +_ = box.space.test_trunc:create_index('i2', {type = 'hash', parts = {2, 'STR'}})
>> +
>> +function test_trunc() space:truncate() ch:put(true) end
>> +
>> +_ = {fiber.create(test_trunc), fiber.create(test_trunc)}
>> +_ = {ch:get(), ch:get()}
>> +space:drop()
>> --
>> 2.17.1
>>
>
>--
>sergeyb@ 
 
 
--
Alexander Tikhonov