[patches] Re: [PATCH v2 1/2] format: add collation to filed_def and tuple_field
v.shpilevoy at tarantool.org
v.shpilevoy at tarantool.org
Sat Mar 10 14:40:55 MSK 2018
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.
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 6b9c76e6d..5ed08cc52 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -346,6 +346,13 @@ function update_format(format)
> end
> elseif k == 2 and not given.type and not given.name then
> field.type = v
> + elseif k == 'collation' then
> + local coll = box.space._collation.index.name:get{v}
> + if not coll then
> + box.error(box.error.ILLEGAL_PARAMS,
> + "format[" .. i .. "]: collation was not found by name '" .. v .. "'")
> + end
> + field[k] = coll[1]
1. Since tuple field names were introduced, you can use coll.id <http://coll.id/> instead of coll[1]. But it is up to you, field names usage is not forced now.
> diff --git a/test/box/ddl.result b/test/box/ddl.result
> index 0eef37992..335f55500 100644
> --- a/test/box/ddl.result
> +++ b/test/box/ddl.result
> @@ -514,3 +514,49 @@ s:format()[3].custom_field
> s:drop()
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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20180310/97810608/attachment.html>
More information about the Tarantool-patches
mailing list