From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B493A236B8 for ; Wed, 10 Oct 2018 07:18:13 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7PzhZIfEUj4i for ; Wed, 10 Oct 2018 07:18:13 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 0099423339 for ; Wed, 10 Oct 2018 07:18:12 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option. References: From: Vladislav Shpilevoy Message-ID: Date: Wed, 10 Oct 2018 14:18:10 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: imeevma@tarantool.org, tarantool-patches@freelists.org On 06/10/2018 12:04, imeevma@tarantool.org wrote: > At the moment all collations that don't have their "strength" > option set works the same way as ones with this option set to > "identical". It is not efficient, according to ICU Comparison > Levels. This patch updates this option of old collations and set > its default value to "primary" for new ones. > > Closes #3573 > --- > Branch: https://github.com/tarantool/tarantool/tree/imeevma/gh-3573-fix-collation-strength > Issue: https://github.com/tarantool/tarantool/issues/3573 > > src/box/bootstrap.snap | Bin 1888 -> 1886 bytes > src/box/coll_id_def.c | 4 +- > src/box/lua/upgrade.lua | 29 ++++++++++++++- > src/coll.c | 33 ++++++++-------- > src/coll_def.c | 1 - > src/coll_def.h | 3 +- > test/box-py/bootstrap.result | 2 +- > test/box/ddl.result | 6 +-- > test/box/tree_pk.result | 2 +- > test/box/tree_pk.test.lua | 2 +- > test/sql/collation.result | 87 +++++++++++++++++++++++++++++++++++++++++++ > test/sql/collation.test.lua | 32 ++++++++++++++++ > test/unit/coll.cpp | 2 + > 13 files changed, 176 insertions(+), 27 deletions(-) > > diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap 1. Please, remove binary diff before sending an email. > index 65739384a66d6ba4a538553ccf4677536ba15280..84b27c0d8b9d6d683692e2ef55034ffcc68d11d0 100644 > GIT binary patch > delta 1884 > zcmV-i2c!7l4&Dxs7k@P^HZUz`Ff(N|W@R=CNp5p=VQyn(Iv_J zG-feoEjVOmHZ3+eGhs9}W;HQnWj6{|Lu_wjYdRo%eF_TIx(m9^2Ic_Hx7lmzr2qf` > z001bpFZ}>e?KS}FOGFS!z*dm~D2k#jilV6AqMxq(Vp{_Az<(~`=k4N^J9QNuj=Z`N > z=~K!63-eO4{Xa?goO8~~I%_^CBZ1 zy%3s9 z)wQCd>0t|o-+y-Dh~($*C8WEVmHYnrJ?s76-+IqxnZ_~OWX>t2h{+{DrBamb3&R_{ > z;m#^i0F_EH-wt>E9?reGOaJa1M|9-I2^O^Z-q&Fr-e!`irB*7%IjR0I-ygdXsZ@$r > zrW6m`p$oIkN~NgMQRS>Um8$bR)M@pqdwLxd>fHSbwLRQvC`i)w)n+f9GNDZZi|h > zlyVA75fPvxtQad)dZGZJvM8eKGmha6=5D?nx8-c_J_zeu`v1DV+jWcK4cc+893buI > zVX8r;QhfFBBJJm)9VdzbkxqxLn6=FsmP(}<_UA-89k%Q^X%-~K>}{G9> z#@04x5`P$pS*0n1pkL1_nK1_{m7>0%zmJtW*}uBmjeXzF$Dyk+jJDtPUE>@0o!5-$ > diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua > index d9c2ae4..07cace0 100644 > --- a/src/box/lua/upgrade.lua > +++ b/src/box/lua/upgrade.lua > @@ -578,6 +578,32 @@ local function upgrade_to_2_1_0() > box.space._schema:format(format) > end > > +-------------------------------------------------------------------------------- > +-- Tarantool 2.1.1 > +-------------------------------------------------------------------------------- > + > +function update_collation_strength_field() > + local _collation = box.space[box.schema.COLLATION_ID] > + local _format = _collation:format() > + for _, collation in ipairs(_collation:select()) do > + if (collation.opts.strength == nil) then > + local old_collation = _collation:get{collation.id} > + local new_collation = {} > + for _,field in ipairs(_format) do > + new_collation[field.name] = old_collation[field.name] > + end > + new_collation.opts.strength = 'identical' > + _collation:delete{old_collation.id} > + _collation:insert(_collation:frommap(new_collation)) 2. This looks much simpler: @@ -584,17 +584,11 @@ end function update_collation_strength_field() local _collation = box.space[box.schema.COLLATION_ID] - local _format = _collation:format() for _, collation in ipairs(_collation:select()) do - if (collation.opts.strength == nil) then - local old_collation = _collation:get{collation.id} - local new_collation = {} - for _,field in ipairs(_format) do - new_collation[field.name] = old_collation[field.name] - end - new_collation.opts.strength = 'identical' - _collation:delete{old_collation.id} - _collation:insert(_collation:frommap(new_collation)) + if collation.opts.strength == nil then + local collation = _collation:get{collation.id}:totable() + collation[6].strength = 'identical' + _collation:replace(collation) end end end I did not test it though. Please, apply and debug if necessary. > + end > + end > +end > diff --git a/test/sql/collation.result b/test/sql/collation.result > index 79ba9ab..3c53be1 100644 > --- a/test/sql/collation.result > +++ b/test/sql/collation.result > @@ -110,3 +110,90 @@ cn:close() > box.schema.user.revoke('guest', 'read,write,execute', 'universe') > --- > ... > +-- > +-- gh-3573: Strength in the _collation space > +-- Collation without 'strength' option set now works as one with > +-- 'strength' set to 'primary'. > +-- > +box.internal.collation.create('c0', 'ICU', 'unicode') > +--- > +... > +box.internal.collation.create('c1', 'ICU', 'unicode', {strength='primary'}) > +--- > +... > +box.internal.collation.create('c2', 'ICU', 'unicode', {strength='secondary'}) > +--- > +... > +box.internal.collation.create('c5', 'ICU', 'unicode', {strength='identical'}) > +--- > +... > +box.sql.execute([[create table tc (id int primary key autoincrement, s0 string collate "c0", s1 string collate "c1", s2 string collate "c2", s5 string collate "c5")]]) > +--- > +... > +box.sql.execute([[insert into tc values (null, 'a', 'a', 'a', 'a')]]) > +--- > +... > +box.sql.execute([[insert into tc values (null, 'A', 'A', 'A', 'A')]]) > +--- > +... > +box.sql.execute([[insert into tc values (null, 'á', 'á', 'á', 'á')]]) > +--- > +... > +box.sql.execute([[insert into tc values (null, 'â', 'â', 'â', 'â')]]) > +--- > +... > +box.sql.execute([[select * from tc where s0 = 'a']]) > +--- > +- - [1, 'a', 'a', 'a', 'a'] > + - [2, 'A', 'A', 'A', 'A'] > + - [3, 'á', 'á', 'á', 'á'] > + - [4, 'â', 'â', 'â', 'â'] > +... > +box.sql.execute([[select * from tc where s1 = 'a']]) > +--- > +- - [1, 'a', 'a', 'a', 'a'] > + - [2, 'A', 'A', 'A', 'A'] > + - [3, 'á', 'á', 'á', 'á'] > + - [4, 'â', 'â', 'â', 'â'] > +... > +box.sql.execute([[select * from tc where s2 = 'a']]) > +--- > +- - [1, 'a', 'a', 'a', 'a'] > + - [2, 'A', 'A', 'A', 'A'] > +... > +box.sql.execute([[select * from tc where s5 = 'a']]) > +--- > +- - [1, 'a', 'a', 'a', 'a'] > +... > +a = box.sql.execute([[select id from tc where s0 = 'a']]) > +--- > +... > +b = box.sql.execute([[select id from tc where s1 = 'a']]) > +--- > +... > +count = 0 > +--- > +... > +for k,v in pairs(a) do if (a[k][1] ~= b[k][1]) then count = count + 1 end end 3. You printed both a and b whole above and they are the same. Why do you need this additional check in a cycle?