<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On 10 Mar 2018, at 14:40, <a href="mailto:v.shpilevoy@tarantool.org" class="">v.shpilevoy@tarantool.org</a> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html; charset=us-ascii" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">See below 2 minor remarks. In the totally the patch seems to be ok. If you will decide to fix these remarks, then send just their diff to the patches, please.</div><br class=""><div class=""><blockquote type="cite" class=""><div class=""><div class="">diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua<br class="">index 6b9c76e6d..5ed08cc52 100644<br class="">--- a/src/box/lua/schema.lua<br class="">+++ b/src/box/lua/schema.lua<br class="">@@ -346,6 +346,13 @@ function update_format(format)<br class=""> end<br class=""> elseif k == 2 and not given.type and not given.name then<br class=""> field.type = v<br class="">+ elseif k == 'collation' then<br class="">+ local coll = box.space._collation.index.name:get{v}<br class="">+ if not coll then<br class="">+ box.error(box.error.ILLEGAL_PARAMS,<br class="">+ "format[" .. i .. "]: collation was not found by name '" .. v .. "'")<br class="">+ end<br class="">+ field[k] = coll[1]<br class=""></div></div></blockquote><div class=""><br class=""></div><div class="">1. Since tuple field names were introduced, you can use <a href="http://coll.id/" class="">coll.id</a> instead of coll[1]. But it is up to you, field names usage is not forced now.</div></div></div></div></blockquote><div><br class=""></div><div>Done:</div><div><br class=""></div><div><div>- field[k] = coll[1]</div><div>+ field[k] = coll.id</div></div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class=""><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div class="">diff --git a/test/box/ddl.result b/test/box/ddl.result<br class="">index 0eef37992..335f55500 100644<br class="">--- a/test/box/ddl.result<br class="">+++ b/test/box/ddl.result<br class="">@@ -514,3 +514,49 @@ s:format()[3].custom_field<br class=""> s:drop()<br class=""></div></div></blockquote><div class=""><br class=""></div><div class="">2. How about a test, when you insert directly into _space with identifier of non-existing collation? To check that "collation was not found by ID" works. It is just advice too.</div></div></div></div></blockquote><br class=""></div><div>Added following test cases:</div><div><br class=""></div><div><div>+</div><div>+-- Check that error is raised when collation doesn't exists.</div><div>+format = {}</div><div>+format[1] = {'field1', 'unsigend', collation = 'test_coll'}</div><div>+s = box.schema.create_space('test', {format = format})</div><div>+</div><div>+-- Check that error is raised when collation with wrong id is used.</div><div>+_space = box.space[box.schema.SPACE_ID]</div><div>+utils = require('utils')</div><div>+EMPTY_MAP = utils.setmap({})</div><div>+format = {{name = 'field1', type = 'string', collation = 666}}</div><div>+surrogate_space = {12345, 1, 'test', 'memtx', 0, EMPTY_MAP, format}</div><div>+_space:insert(surrogate_space)</div><div class=""><br class=""></div></div><br class=""></body></html>