From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: Re: [tarantool-patches] Re: [PATCH 1/2] schema: add "_vcollation" sysview From: Roman Khabibov In-Reply-To: <20190604163159.5lctqc4kaclxo3rh@esperanza> Date: Wed, 5 Jun 2019 20:08:46 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <5eab44d0494d7622ef5fbb77c45229a09212d9bc.1559219194.git.roman.habibov@tarantool.org> <20190604163159.5lctqc4kaclxo3rh@esperanza> To: tarantool-patches@freelists.org Cc: Vladimir Davydov List-ID: Hi! Thanks for the review. > On Jun 4, 2019, at 7:31 PM, Vladimir Davydov = wrote: >=20 > On Thu, May 30, 2019 at 03:36:02PM +0300, Roman Khabibov wrote: >> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua >> index 89d6e3d52..f2a1be639 100644 >> --- a/src/box/lua/upgrade.lua >> +++ b/src/box/lua/upgrade.lua >> @@ -737,6 +737,30 @@ local function upgrade_to_2_1_3() >> end >> end >>=20 >> +local function create_vcollation_space() >> + local _collation =3D box.space._collation >> + local format =3D _collation:format() >> + create_sysview(box.schema.COLLATION_ID, = box.schema.VCOLLATION_ID) >> + box.space[box.schema.VCOLLATION_ID]:format(format) >> +end >> + >> +local function upgrade_to_2_1_4() >> + local _collation =3D box.space._collation >> + local _index =3D box.space._index >> + >> + -- System space format usually is in order "id, owner, name...". >> + -- The fields "name", "owner" are swapped in "_collation" = format, >> + -- due to the field "owner" was added after the "_collation" = creation. >> + box.space._index:delete{276, 1} >> + log.info("update index name on _collation") >> + box.space._index:insert{_collation.id, 2, 'name', 'tree', = {unique =3D true}, >> + {{1, 'string'}}} >> + log.info("create index owner on _collation") >> + box.space._index:insert{_collation.id, 1, 'owner', 'tree', = {unique =3D false}, >> + {{2, 'unsigned'}}} >=20 > I don't understand this part. Why do you need to create 'owner' index? > Why do you need to update 'name' index? create_sysview() can=E2=80=99t work otherwise. + +-- Fit _collation indexes to be suitable forcreate_sysview(). +local function upgrade_collation_to_2_2_2() + local _collation =3D box.space._collation + local _index =3D box.space._index >=20 >> + create_vcollation_space() >> +end >> + >> local function get_version() >> local version =3D box.space._schema:get{'version'} >> if version =3D=3D nil then >> @@ -768,6 +792,7 @@ local function upgrade(options) >> {version =3D mkversion(2, 1, 1), func =3D upgrade_to_2_1_1, = auto =3D true}, >> {version =3D mkversion(2, 1, 2), func =3D upgrade_to_2_1_2, = auto =3D true}, >> {version =3D mkversion(2, 1, 3), func =3D upgrade_to_2_1_3, = auto =3D true}, >> + {version =3D mkversion(2, 1, 4), func =3D upgrade_to_2_1_4, = auto =3D true} >=20 > Should be in upgrade_to_2_2_1. To 2.2.2 maybe? = +-------------------------------------------------------------------------= ------- +-- Tarantool 2.2.2 = +-------------------------------------------------------------------------= ------- + +-- Fit _collation space to be suitable for create_sysview(). +local function upgrade_collation_to_2_2_2() + local _collation =3D box.space._collation + local _index =3D box.space._index + + -- System space format usually is in order "id, owner, + -- name...". The fields "name", "owner" are swapped in + -- "_collation" format, due to the field "owner" was added + -- after the "_collation" creation. + box.space._index:delete{276, 1} + log.info("update index name on _collation") + box.space._index:insert{_collation.id, 2, 'name', 'tree', {unique =3D= true}, + {{1, 'string'}}} + log.info("create index owner on _collation") + box.space._index:insert{_collation.id, 1, 'owner', 'tree', {unique = =3D false}, + {{2, 'unsigned'}}} +end + +local function create_vcollation_space() + local _collation =3D box.space._collation + local format =3D _collation:format() + create_sysview(box.schema.COLLATION_ID, box.schema.VCOLLATION_ID) + box.space[box.schema.VCOLLATION_ID]:format(format) +end + +local function upgrade_to_2_2_2() + upgrade_collation_to_2_2_2() + create_vcollation_space() +end + = --------------------------------------------------------------------------= ------ >> } >>=20 >> for _, handler in ipairs(handlers) do >> diff --git a/src/box/schema_def.h b/src/box/schema_def.h >> index eeeeb950b..77c004690 100644 >> --- a/src/box/schema_def.h >> +++ b/src/box/schema_def.h >> @@ -72,6 +72,8 @@ enum { >> BOX_SCHEMA_ID =3D 272, >> /** Space id of _collation. */ >> BOX_COLLATION_ID =3D 276, >> + /** Space id of _vcollation. */ >> + BOX_VCOLLATION_ID =3D 277, >> /** Space id of _space. */ >> BOX_SPACE_ID =3D 280, >> /** Space id of _vspace view. */ >> diff --git a/src/box/sysview.c b/src/box/sysview.c >> index 96c5e78ca..0d1259af0 100644 >> --- a/src/box/sysview.c >> +++ b/src/box/sysview.c >> @@ -402,6 +402,14 @@ vsequence_filter(struct space *source, struct = tuple *tuple) >> ((PRIV_WRDA | PRIV_X) & effective); >> } >>=20 >> +static bool >> + vcollation_filter(struct space *source, struct tuple *tuple) >> + { >> + (void) source; >> + (void) tuple; >> + return true; >> + } >> + >=20 > Bad indentation. +static bool +vcollation_filter(struct space *source, struct tuple *tuple) +{ + (void) source; + (void) tuple; + return true; +} + commit e67cd253a41f5e010c00b5cb38059224850e86c4 Author: Roman Khabibov Date: Thu May 16 02:06:39 2019 +0300 schema: add "_vcollation" sysview =20 Add "_vcollation" sysview to read it from net.box. This sysview is always readable, except when a user doesn't have "public" role. =20 Needed for #3941 =20 @TarantoolBot document Title: box.space._vcollation =20 _vcollation is a system space that represents a virtual view. The structure of its tuples is identical to that of _collation. Tuples of this sysview is always readable, except when the user doesn't have "public" role. diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index bb8fbeba1..f01a34670 100644 Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap = differ diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index 87adaeb16..5d837eda6 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -540,6 +540,8 @@ box_lua_space_init(struct lua_State *L) lua_setfield(L, -2, "FUNC_ID"); lua_pushnumber(L, BOX_COLLATION_ID); lua_setfield(L, -2, "COLLATION_ID"); + lua_pushnumber(L, BOX_VCOLLATION_ID); + lua_setfield(L, -2, "VCOLLATION_ID"); lua_pushnumber(L, BOX_VFUNC_ID); lua_setfield(L, -2, "VFUNC_ID"); lua_pushnumber(L, BOX_PRIV_ID); diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index 070662698..b7fb08014 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -778,6 +778,40 @@ local function upgrade_to_2_2_1() upgrade_sequence_to_2_2_1() end =20 = +-------------------------------------------------------------------------= ------- +-- Tarantool 2.2.2 = +-------------------------------------------------------------------------= ------- + +-- Fit _collation indexes to be suitable forcreate_sysview(). +local function upgrade_collation_to_2_2_2() + local _collation =3D box.space._collation + local _index =3D box.space._index + + -- System space format usually is in order "id, owner, + -- name...". The fields "name", "owner" are swapped in + -- "_collation" format, due to the field "owner" was added + -- after the "_collation" creation. + box.space._index:delete{276, 1} + log.info("update index name on _collation") + box.space._index:insert{_collation.id, 2, 'name', 'tree', {unique =3D= true}, + {{1, 'string'}}} + log.info("create index owner on _collation") + box.space._index:insert{_collation.id, 1, 'owner', 'tree', {unique = =3D false}, + {{2, 'unsigned'}}} +end + +local function create_vcollation_space() + local _collation =3D box.space._collation + local format =3D _collation:format() + create_sysview(box.schema.COLLATION_ID, box.schema.VCOLLATION_ID) + box.space[box.schema.VCOLLATION_ID]:format(format) +end + +local function upgrade_to_2_2_2() + upgrade_collation_to_2_2_2() + create_vcollation_space() +end + = --------------------------------------------------------------------------= ------ =20 local function get_version() @@ -812,6 +846,7 @@ local function upgrade(options) {version =3D mkversion(2, 1, 2), func =3D upgrade_to_2_1_2, = auto =3D true}, {version =3D mkversion(2, 1, 3), func =3D upgrade_to_2_1_3, = auto =3D true}, {version =3D mkversion(2, 2, 1), func =3D upgrade_to_2_2_1, = auto =3D true}, + {version =3D mkversion(2, 2, 2), func =3D upgrade_to_2_2_2, = auto =3D true} } =20 for _, handler in ipairs(handlers) do diff --git a/src/box/schema_def.h b/src/box/schema_def.h index b817b49f6..e0febb640 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -72,6 +72,8 @@ enum { BOX_SCHEMA_ID =3D 272, /** Space id of _collation. */ BOX_COLLATION_ID =3D 276, + /** Space id of _vcollation. */ + BOX_VCOLLATION_ID =3D 277, /** Space id of _space. */ BOX_SPACE_ID =3D 280, /** Space id of _vspace view. */ diff --git a/src/box/sysview.c b/src/box/sysview.c index 96c5e78ca..46cf1e13f 100644 --- a/src/box/sysview.c +++ b/src/box/sysview.c @@ -402,6 +402,14 @@ vsequence_filter(struct space *source, struct tuple = *tuple) ((PRIV_WRDA | PRIV_X) & effective); } =20 +static bool +vcollation_filter(struct space *source, struct tuple *tuple) +{ + (void) source; + (void) tuple; + return true; +} + static struct index * sysview_space_create_index(struct space *space, struct index_def *def) { @@ -448,6 +456,11 @@ sysview_space_create_index(struct space *space, = struct index_def *def) source_index_id =3D def->iid; filter =3D vsequence_filter; break; + case BOX_VCOLLATION_ID: + source_space_id =3D BOX_COLLATION_ID; + source_index_id =3D def->iid; + filter =3D vcollation_filter; + break; default: diag_set(ClientError, ER_MODIFY_INDEX, def->name, space_name(space), diff --git a/test/app-tap/tarantoolctl.test.lua = b/test/app-tap/tarantoolctl.test.lua index c1e1490ca..ac1efa0ba 100755 --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -405,8 +405,8 @@ do check_ctlcat_xlog(test_i, dir, "--from=3D3 --to=3D6 = --format=3Djson --show-system --replica 1", "\n", 3) check_ctlcat_xlog(test_i, dir, "--from=3D3 --to=3D6 = --format=3Djson --show-system --replica 1 --replica 2", "\n", 3) check_ctlcat_xlog(test_i, dir, "--from=3D3 --to=3D6 = --format=3Djson --show-system --replica 2", "\n", 0) - check_ctlcat_snap(test_i, dir, "--space=3D280", "---\n", = 21) - check_ctlcat_snap(test_i, dir, "--space=3D288", "---\n", = 47) + check_ctlcat_snap(test_i, dir, "--space=3D280", "---\n", = 22) + check_ctlcat_snap(test_i, dir, "--space=3D288", "---\n", = 51) end) end) =20 diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index 0684914c0..c8c37eb06 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -4,7 +4,7 @@ box.internal.bootstrap() box.space._schema:select{} --- - - ['max_id', 511] - - ['version', 2, 2, 1] + - ['version', 2, 2, 2] ... box.space._cluster:select{} --- @@ -21,6 +21,10 @@ box.space._space:select{} 'name': 'name', 'type': 'string'}, {'name': 'owner', 'type': = 'unsigned'}, {'name': 'type', 'type': 'string'}, {'name': 'locale', 'type': = 'string'}, { 'name': 'opts', 'type': 'map'}]] + - [277, 1, '_vcollation', 'sysview', 0, {}, [{'name': 'id', 'type': = 'unsigned'}, + {'name': 'name', 'type': 'string'}, {'name': 'owner', 'type': = 'unsigned'}, { + 'name': 'type', 'type': 'string'}, {'name': 'locale', 'type': = 'string'}, { + 'name': 'opts', 'type': 'map'}]] - [280, 1, '_space', 'memtx', 0, {}, [{'name': 'id', 'type': = 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, = {'name': 'engine', 'type': 'string'}, {'name': 'field_count', 'type': 'unsigned'}, = {'name': 'flags', @@ -84,7 +88,11 @@ box.space._index:select{} --- - - [272, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]] - [276, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]] - - [276, 1, 'name', 'tree', {'unique': true}, [[1, 'string']]] + - [276, 1, 'owner', 'tree', {'unique': false}, [[2, 'unsigned']]] + - [276, 2, 'name', 'tree', {'unique': true}, [[1, 'string']]] + - [277, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]] + - [277, 1, 'owner', 'tree', {'unique': false}, [[2, 'unsigned']]] + - [277, 2, 'name', 'tree', {'unique': true}, [[1, 'string']]] - [280, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]] - [280, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]] - [280, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]] @@ -151,6 +159,7 @@ box.space._priv:select{} - [1, 1, 'universe', 0, 4294967295] - [1, 2, 'function', 1, 4] - [1, 2, 'space', 276, 2] + - [1, 2, 'space', 277, 1] - [1, 2, 'space', 281, 1] - [1, 2, 'space', 286, 1] - [1, 2, 'space', 289, 1] diff --git a/test/box/access_misc.result b/test/box/access_misc.result index 24bdd9d63..d5bb8b208 100644 --- a/test/box/access_misc.result +++ b/test/box/access_misc.result @@ -761,6 +761,10 @@ box.space._space:select() 'name': 'name', 'type': 'string'}, {'name': 'owner', 'type': = 'unsigned'}, {'name': 'type', 'type': 'string'}, {'name': 'locale', 'type': = 'string'}, { 'name': 'opts', 'type': 'map'}]] + - [277, 1, '_vcollation', 'sysview', 0, {}, [{'name': 'id', 'type': = 'unsigned'}, + {'name': 'name', 'type': 'string'}, {'name': 'owner', 'type': = 'unsigned'}, { + 'name': 'type', 'type': 'string'}, {'name': 'locale', 'type': = 'string'}, { + 'name': 'opts', 'type': 'map'}]] - [280, 1, '_space', 'memtx', 0, {}, [{'name': 'id', 'type': = 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, = {'name': 'engine', 'type': 'string'}, {'name': 'field_count', 'type': 'unsigned'}, = {'name': 'flags', diff --git a/test/box/access_sysview.result = b/test/box/access_sysview.result index c6a2b22ed..96976babb 100644 --- a/test/box/access_sysview.result +++ b/test/box/access_sysview.result @@ -74,6 +74,10 @@ session.su('guest') --- - true ... +#box.space._vspace.index[2]:select('_vcollation') ~=3D 0 +--- +- true +... #box.space._vindex:select(box.space._vspace.id) > 0 --- - true @@ -94,6 +98,10 @@ session.su('guest') --- - true ... +#box.space._vindex:select(box.space._vcollation.id) > 0 +--- +- true +... box.session.su('admin') --- ... @@ -127,6 +135,10 @@ box.session.su('guest') --- - error: Read access to space '_vsequence' is denied for user 'guest' ... +#box.space._vcollation:select{} +--- +- error: Read access to space '_vcollation' is denied for user 'guest' +... box.session.su('admin') --- ... @@ -138,11 +150,15 @@ box.session.su('guest') ... #box.space._vspace:select{} --- -- 8 +- 9 ... #box.space._vindex:select{} --- -- 20 +- 24 +... +#box.space._vcollation:select{} +--- +- 277 ... box.session.su('admin') --- @@ -230,11 +246,11 @@ box.session.su('guest') ... #box.space._vspace:select{} --- -- 22 +- 23 ... #box.space._vindex:select{} --- -- 48 +- 52 ... #box.space._vuser:select{} --- @@ -242,12 +258,16 @@ box.session.su('guest') ... #box.space._vpriv:select{} --- -- 15 +- 16 ... #box.space._vfunc:select{} --- - 1 ... +#box.space._vcollation:select{} +--- +- 277 +... box.session.su('admin') --- ... @@ -262,7 +282,7 @@ box.session.su('guest') ... #box.space._vindex:select{} --- -- 48 +- 52 ... #box.space._vuser:select{} --- @@ -270,7 +290,7 @@ box.session.su('guest') ... #box.space._vpriv:select{} --- -- 15 +- 16 ... #box.space._vfunc:select{} --- @@ -280,6 +300,10 @@ box.session.su('guest') --- - 0 ... +#box.space._vcollation:select{} +--- +- 277 +... box.session.su('admin') --- ... @@ -631,6 +655,60 @@ seq:drop() --- ... -- +-- _vcollation +-- +box.session.su('admin') +--- +... +box.internal.collation.create('test', 'ICU', 'ru-RU') +--- +... +-- Only admin can create collation. +coll_cnt =3D #box.space._collation:select{} +--- +... +box.schema.user.grant("guest", "read, write, alter, execute", "space", = "_collation") +--- +... +box.session.su("guest") +--- +... +box.internal.collation.create('guest0', 'ICU', 'ru-RU') +--- +- error: Create access to collation 'guest0' is denied for user 'guest' +... +box.space._vcollation:select{0} +--- +- - [0, 'none', 1, 'BINARY', '', {}] +... +#box.space._vcollation:select{} =3D=3D coll_cnt +--- +- true +... +box.session.su('admin') +--- +... +-- _vcollation is readable anyway. +box.schema.user.revoke("guest", "read", "space", "_collation") +--- +... +box.session.su("guest") +--- +... +#box.space._vcollation:select{} +--- +- 278 +... +session.su('admin') +--- +... +box.internal.collation.drop('test') +--- +... +box.internal.collation.drop('guest0') +--- +... +-- -- view:alter() tests -- box.space._vspace.index[1]:alter({parts =3D { 2, 'string' }}) diff --git a/test/box/access_sysview.test.lua = b/test/box/access_sysview.test.lua index c62458407..031df28aa 100644 --- a/test/box/access_sysview.test.lua +++ b/test/box/access_sysview.test.lua @@ -31,12 +31,14 @@ session.su('guest') #box.space._vspace.index[2]:select('_vuser') ~=3D 0 #box.space._vspace.index[2]:select('_vfunc') ~=3D 0 #box.space._vspace.index[2]:select('_vpriv') ~=3D 0 +#box.space._vspace.index[2]:select('_vcollation') ~=3D 0 =20 #box.space._vindex:select(box.space._vspace.id) > 0 #box.space._vindex:select(box.space._vindex.id) > 0 #box.space._vindex:select(box.space._vuser.id) > 0 #box.space._vindex:select(box.space._vfunc.id) > 0 #box.space._vindex:select(box.space._vpriv.id) > 0 +#box.space._vindex:select(box.space._vcollation.id) > 0 =20 box.session.su('admin') box.schema.user.revoke('guest', 'public') @@ -48,6 +50,7 @@ box.session.su('guest') #box.space._vpriv:select{} #box.space._vfunc:select{} #box.space._vsequence:select{} +#box.space._vcollation:select{} =20 box.session.su('admin') box.schema.user.grant('guest', 'public') @@ -55,6 +58,7 @@ box.session.su('guest') =20 #box.space._vspace:select{} #box.space._vindex:select{} +#box.space._vcollation:select{} =20 box.session.su('admin') s =3D box.schema.space.create('test') @@ -96,6 +100,7 @@ box.session.su('guest') #box.space._vuser:select{} #box.space._vpriv:select{} #box.space._vfunc:select{} +#box.space._vcollation:select{} =20 box.session.su('admin') box.schema.user.revoke('guest', 'read', 'universe') @@ -107,6 +112,7 @@ box.session.su('guest') #box.space._vpriv:select{} #box.space._vfunc:select{} #box.space._vsequence:select{} +#box.space._vcollation:select{} =20 box.session.su('admin') box.schema.user.revoke('guest', 'write', 'universe') @@ -264,6 +270,30 @@ box.session.su("guest") session.su('admin') seq:drop() =20 +-- +-- _vcollation +-- + +box.session.su('admin') +box.internal.collation.create('test', 'ICU', 'ru-RU') + +-- Only admin can create collation. +coll_cnt =3D #box.space._collation:select{} +box.schema.user.grant("guest", "read, write, alter, execute", "space", = "_collation") +box.session.su("guest") +box.internal.collation.create('guest0', 'ICU', 'ru-RU') +box.space._vcollation:select{0} +#box.space._vcollation:select{} =3D=3D coll_cnt +box.session.su('admin') + +-- _vcollation is readable anyway. +box.schema.user.revoke("guest", "read", "space", "_collation") +box.session.su("guest") +#box.space._vcollation:select{} +session.su('admin') +box.internal.collation.drop('test') +box.internal.collation.drop('guest0') + -- -- view:alter() tests -- diff --git a/test/box/alter.result b/test/box/alter.result index e83c0b7ef..ba1405cf1 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -168,7 +168,11 @@ _index:select{} --- - - [272, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]] - [276, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]] - - [276, 1, 'name', 'tree', {'unique': true}, [[1, 'string']]] + - [276, 1, 'owner', 'tree', {'unique': false}, [[2, 'unsigned']]] + - [276, 2, 'name', 'tree', {'unique': true}, [[1, 'string']]] + - [277, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]] + - [277, 1, 'owner', 'tree', {'unique': false}, [[2, 'unsigned']]] + - [277, 2, 'name', 'tree', {'unique': true}, [[1, 'string']]] - [280, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]] - [280, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]] - [280, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]] diff --git a/test/wal_off/alter.result b/test/wal_off/alter.result index ee280fcbb..8040efa1a 100644 --- a/test/wal_off/alter.result +++ b/test/wal_off/alter.result @@ -28,7 +28,7 @@ end; ... #spaces; --- -- 65505 +- 65504 ... -- cleanup for k, v in pairs(spaces) do