* [tarantool-patches] Re: [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option. @ 2018-10-12 18:07 Мерген Имеев 2018-10-13 12:17 ` Vladislav Shpilevoy 0 siblings, 1 reply; 6+ messages in thread From: Мерген Имеев @ 2018-10-12 18:07 UTC (permalink / raw) To: Vladislav Shpilevoy, tarantool-patches [-- Attachment #1: Type: text/plain, Size: 2953 bytes --] Hello! Thank you for review! My answer below. On 10/12/2018 03:05 AM, Vladislav Shpilevoy wrote: >Thanks for the fixes! See 1 comment below. >>diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua >> index d9c2ae4..66dcef4 100644 >> --- a/src/box/lua/upgrade.lua >> +++ b/src/box/lua/upgrade.lua >> @@ -578,6 +578,27 @@ 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] >> + for _, collation in ipairs(_collation:select()) do >> + if (collation.opts.strength == nil) then >> + local new_collation = _collation:get{collation.id}:totable() >> + new_collation[6].strength = 'identical' >> + _collation:delete{collation.id} >> + _collation:insert(new_collation) > Why did you remove replace from my diff and replaced it with > delete + insert? collation.id is a primary index and it is the > same in the new and old collation. It makes no sense to do delete + > insert, when you can do one replace. Replace couldn't be used in space box.space._collation: tarantool> function update_collation_strength_field() > local _collation = box.space[box.schema.COLLATION_ID] > for _, collation in ipairs(_collation:select()) do > if (collation.opts.strength == nil) then > local new_collation = _collation:get{collation.id}:totable() > new_collation[6].strength = 'identical' > _collation:replace(new_collation) > end > end > end --- ... tarantool> update_collation_strength_field() --- - error: collation does not support alter ... >>+ end >> + end >> +end >> + >> +local function upgrade_to_2_1_1() >> + update_collation_strength_field() >> +end >> + >> + [-- Attachment #2: Type: text/html, Size: 5496 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option. 2018-10-12 18:07 [tarantool-patches] Re: [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option Мерген Имеев @ 2018-10-13 12:17 ` Vladislav Shpilevoy 2018-10-15 18:30 ` Imeev Mergen 0 siblings, 1 reply; 6+ messages in thread From: Vladislav Shpilevoy @ 2018-10-13 12:17 UTC (permalink / raw) To: tarantool-patches, Мерген Имеев Hi! > Replace couldn't be used in space box.space._collation: > tarantool> function update_collation_strength_field() > > local _collation = > box.space[box.schema.COLLATION_ID] > > for _, collation in ipairs(_collation:select()) do > > if (collation.opts.strength == nil) then > > local new_collation = > _collation:get{collation.id}:totable() > > new_collation[6].strength = 'identical' > > _collation:replace(new_collation) > > end > > end > > end > --- > ... > tarantool> update_collation_strength_field() > --- > - error: collation does not support alter > ... I just found that it is not normal. Triggers on system spaces are turned off before upgrade. But for _collation space they are not. Please, turn off the triggers and use replace. See function set_system_triggers in upgrade.lua. > >>+ end > >> + end > >> +end > >> + > >> +local function upgrade_to_2_1_1() > >> + update_collation_strength_field() > >> +end > >> + > >> + > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option. 2018-10-13 12:17 ` Vladislav Shpilevoy @ 2018-10-15 18:30 ` Imeev Mergen 0 siblings, 0 replies; 6+ messages in thread From: Imeev Mergen @ 2018-10-15 18:30 UTC (permalink / raw) To: n.pettik; +Cc: Vladislav Shpilevoy, tarantool-patches [-- Attachment #1: Type: text/plain, Size: 15276 bytes --] Hello! Thank you for review! After discussion decided to left as it is now. Nikita, can you do next review? Links and diff below. On 10/13/2018 03:17 PM, Vladislav Shpilevoy wrote: > Hi! > >> Replace couldn't be used in space box.space._collation: >> tarantool> function update_collation_strength_field() >> > local _collation = >> box.space[box.schema.COLLATION_ID] >> > for _, collation in ipairs(_collation:select()) do >> > if (collation.opts.strength == nil) then >> > local new_collation = >> _collation:get{collation.id}:totable() >> > new_collation[6].strength = 'identical' >> > _collation:replace(new_collation) >> > end >> > end >> > end >> --- >> ... >> tarantool> update_collation_strength_field() >> --- >> - error: collation does not support alter >> ... > > I just found that it is not normal. Triggers on system > spaces are turned off before upgrade. But for _collation > space they are not. Please, turn off the triggers and use > replace. See function set_system_triggers in upgrade.lua. > >> >>+ end >> >> + end >> >> +end >> >> + >> >> +local function upgrade_to_2_1_1() >> >> + update_collation_strength_field() >> >> +end >> >> + >> >> + >> *Issue:* https://github.com/tarantool/tarantool/issues/3573 *Branch:* https://github.com/tarantool/tarantool/tree/imeevma/gh-3573-fix-collation-strength * Patch:* commit e55064f796d9320035db5fb2e3d5030601ee7333 Author: Mergen Imeev <imeevma@gmail.com> Date: Tue Sep 25 21:30:36 2018 +0300 box: update collation strength option. 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 diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index 6573938..3d3232f 100644 Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap differ diff --git a/src/box/coll_id_def.c b/src/box/coll_id_def.c index 9fe0cda..43f3b63 100644 --- a/src/box/coll_id_def.c +++ b/src/box/coll_id_def.c @@ -55,8 +55,8 @@ icu_case_first_from_str(const char *str, uint32_t len) static int64_t icu_strength_from_str(const char *str, uint32_t len) { - return strnindex(coll_icu_strength_strs + 1, str, len, - coll_icu_strength_MAX - 1) + 1; + return strnindex(coll_icu_strength_strs, str, len, + coll_icu_strength_MAX); } const struct opt_def coll_icu_opts_reg[] = { diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index d9c2ae4..66dcef4 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -578,6 +578,27 @@ 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] + for _, collation in ipairs(_collation:select()) do + if (collation.opts.strength == nil) then + local new_collation = _collation:get{collation.id}:totable() + new_collation[6].strength = 'identical' + _collation:delete{collation.id} + _collation:insert(new_collation) + end + end +end + +local function upgrade_to_2_1_1() + update_collation_strength_field() +end + + local function get_version() local version = box.space._schema:get{'version'} if version == nil then @@ -605,7 +626,8 @@ local function upgrade(options) {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true}, {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true}, {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true}, - {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true} + {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true}, + {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true} } for _, handler in ipairs(handlers) do diff --git a/src/coll.c b/src/coll.c index 6a76f1f..9b719f2 100644 --- a/src/coll.c +++ b/src/coll.c @@ -193,22 +193,25 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def) return -1; } } - if (def->icu.strength != COLL_ICU_STRENGTH_DEFAULT) { - enum coll_icu_strength w = def->icu.strength; - UColAttributeValue v = - w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY : - w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY : - w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY : - w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY : - w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL : - UCOL_DEFAULT; - ucol_setAttribute(collator, UCOL_STRENGTH, v, &status); - if (U_FAILURE(status)) { - diag_set(CollationError, "failed to set strength: %s", - u_errorName(status)); - return -1; - } + /* + * Set "strength" option of collation. Default values is + * "primary". + */ + enum coll_icu_strength w = def->icu.strength; + UColAttributeValue v = + w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY : + w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY : + w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY : + w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY : + w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL : + UCOL_DEFAULT; + ucol_setAttribute(collator, UCOL_STRENGTH, v, &status); + if (U_FAILURE(status)) { + diag_set(CollationError, "failed to set strength: %s", + u_errorName(status)); + return -1; } + if (def->icu.numeric_collation != COLL_ICU_DEFAULT) { enum coll_icu_on_off w = def->icu.numeric_collation; UColAttributeValue v = w == COLL_ICU_ON ? UCOL_ON : diff --git a/src/coll_def.c b/src/coll_def.c index df58cac..1323643 100644 --- a/src/coll_def.c +++ b/src/coll_def.c @@ -54,7 +54,6 @@ const char *coll_icu_case_first_strs[] = { }; const char *coll_icu_strength_strs[] = { - "DEFAULT", "PRIMARY", "SECONDARY", "TERTIARY", diff --git a/src/coll_def.h b/src/coll_def.h index 7c20abf..2a1cb68 100644 --- a/src/coll_def.h +++ b/src/coll_def.h @@ -84,8 +84,7 @@ extern const char *coll_icu_case_first_strs[]; /** Strength ICU settings */ enum coll_icu_strength { - COLL_ICU_STRENGTH_DEFAULT = 0, - COLL_ICU_STRENGTH_PRIMARY, + COLL_ICU_STRENGTH_PRIMARY = 0, COLL_ICU_STRENGTH_SECONDARY, COLL_ICU_STRENGTH_TERTIARY, COLL_ICU_STRENGTH_QUATERNARY, diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index 506aca3..2532b70 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -5,7 +5,7 @@ box.space._schema:select{} --- - - ['cluster', '<cluster uuid>'] - ['max_id', 511] - - ['version', 2, 1, 0] + - ['version', 2, 1, 1] ... box.space._cluster:select{} --- diff --git a/test/box/ddl.result b/test/box/ddl.result index c9a8e96..396318f 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -484,14 +484,14 @@ box.space._collation:auto_increment{'test', 0, 'ICU', 'ru_RU', setmap{}} ... box.space._collation:select{} --- -- - [1, 'unicode', 1, 'ICU', '', {}] +- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] - [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}] - [3, 'test', 0, 'ICU', 'ru_RU', {}] ... test_run:cmd('restart server default') box.space._collation:select{} --- -- - [1, 'unicode', 1, 'ICU', '', {}] +- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] - [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}] - [3, 'test', 0, 'ICU', 'ru_RU', {}] ... @@ -512,7 +512,7 @@ utf8.cmp('abc', 'def') ... box.space._collation:replace(t) --- -- [1, 'unicode', 1, 'ICU', '', {}] +- [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] ... -- -- gh-2839: allow to store custom fields in field definition. diff --git a/test/box/tree_pk.result b/test/box/tree_pk.result index df3c78b..7366ed8 100644 --- a/test/box/tree_pk.result +++ b/test/box/tree_pk.result @@ -690,7 +690,7 @@ s:drop() ... --https://github.com/tarantool/tarantool/issues/2649 -- create standart index and alter it to collation index -box.internal.collation.create('test', 'ICU', 'ru-RU') +box.internal.collation.create('test', 'ICU', 'ru-RU', {strength = 'identical'}) --- ... box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength = 'secondary'}) diff --git a/test/box/tree_pk.test.lua b/test/box/tree_pk.test.lua index 1190ab4..b4ee65c 100644 --- a/test/box/tree_pk.test.lua +++ b/test/box/tree_pk.test.lua @@ -238,7 +238,7 @@ s:drop() --https://github.com/tarantool/tarantool/issues/2649 -- create standart index and alter it to collation index -box.internal.collation.create('test', 'ICU', 'ru-RU') +box.internal.collation.create('test', 'ICU', 'ru-RU', {strength = 'identical'}) box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength = 'secondary'}) s = box.schema.space.create('test') i1 = s:create_index('i1', { type = 'tree', parts = {{1, 'str'}}, unique = true }) diff --git a/test/sql/collation.result b/test/sql/collation.result index 79ba9ab..5fdb8ee 100644 --- a/test/sql/collation.result +++ b/test/sql/collation.result @@ -110,3 +110,77 @@ 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, 'â', 'â', 'â', 'â')]]) +--- +... +-- Should be same as the next one. +box.sql.execute([[select * from tc where s0 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] + - [2, 'A', 'A', 'A', 'A'] + - [3, 'á', 'á', 'á', 'á'] + - [4, 'â', 'â', 'â', 'â'] +... +-- Should return all records. +box.sql.execute([[select * from tc where s1 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] + - [2, 'A', 'A', 'A', 'A'] + - [3, 'á', 'á', 'á', 'á'] + - [4, 'â', 'â', 'â', 'â'] +... +-- Should return two records. +box.sql.execute([[select * from tc where s2 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] + - [2, 'A', 'A', 'A', 'A'] +... +-- Should return one record. +box.sql.execute([[select * from tc where s5 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] +... +box.sql.execute([[drop table tc]]) +--- +... +box.internal.collation.drop('c0') +--- +... +box.internal.collation.drop('c1') +--- +... +box.internal.collation.drop('c2') +--- +... +box.internal.collation.drop('c5') +--- +... diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua index 935dea8..130245b 100644 --- a/test/sql/collation.test.lua +++ b/test/sql/collation.test.lua @@ -43,3 +43,32 @@ cn:execute('select 1 limit ? collate not_exist', {1}) 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, 'â', 'â', 'â', 'â')]]) +-- Should be same as the next one. +box.sql.execute([[select * from tc where s0 = 'a']]) +-- Should return all records. +box.sql.execute([[select * from tc where s1 = 'a']]) +-- Should return two records. +box.sql.execute([[select * from tc where s2 = 'a']]) +-- Should return one record. +box.sql.execute([[select * from tc where s5 = 'a']]) + +box.sql.execute([[drop table tc]]) +box.internal.collation.drop('c0') +box.internal.collation.drop('c1') +box.internal.collation.drop('c2') +box.internal.collation.drop('c5') diff --git a/test/unit/coll.cpp b/test/unit/coll.cpp index 94374a7..acbc7f4 100644 --- a/test/unit/coll.cpp +++ b/test/unit/coll.cpp @@ -51,6 +51,7 @@ manual_test() memset(&def, 0, sizeof(def)); snprintf(def.locale, sizeof(def.locale), "%s", "ru_RU"); def.type = COLL_TYPE_ICU; + def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL; struct coll *coll; cout << " -- default ru_RU -- " << endl; @@ -134,6 +135,7 @@ hash_test() struct coll *coll; /* Case sensitive */ + def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL; coll = coll_new(&def); assert(coll != NULL); cout << "Case sensitive" << endl; [-- Attachment #2: Type: text/html, Size: 19763 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] [PATCH v2 1/1] box: update collation strength option. @ 2018-10-06 9:04 imeevma 2018-10-10 11:18 ` [tarantool-patches] " Vladislav Shpilevoy 0 siblings, 1 reply; 6+ messages in thread From: imeevma @ 2018-10-06 9:04 UTC (permalink / raw) To: v.shpilevoy, tarantool-patches 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 index 65739384a66d6ba4a538553ccf4677536ba15280..84b27c0d8b9d6d683692e2ef55034ffcc68d11d0 100644 GIT binary patch delta 1884 zcmV-i2c!7l4&Dxs7k@P^HZUz`Ff(N|W@R=CNp5p=VQyn(Iv_J<IA&xwVKpsbHZx@{ 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<YnJQf6Sh@N@2|aY1nNsKg<N)~qVZO0ZrY}vD zy%3s9<K4NfXf{L+b=bqk3;S2c?QOCMxO?C6GZ^+!^Gx$zyD--w?m=0vdDwn;L5_Ra 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><f;VmNfL9 z#@04x5`P$pS*0n1pkL1_nK1_{m7>0%zmJtW*}uBmjeXzF$Dyk+jJDtPUE>@0o!5-$ z4}+fD+=MY8^f<<4{1&sf`BI=?*#25J*Uhc5>+tKWmmjy5G<NB?6|uJILZDJ9f)_^P z9sFvy`?3v`_5E7t8<8aG5BHn%*IA|6GN4i^LVx+px;cm1a=i2bJ3KojW;VJaAy*^n z>}Vw-6(R*9wS!urR4ARAoEjXXnj4xAG^t9ZXhefDd6=1OW=^x24l*2MHptYx_<hCD zz-WNU0E6M>!i$BM3Y5B-=FI5HbeVCXFlUQQrBZB(kuxJG9|mC^+KB#luSu3ECCM}c zQ-7%xm-5#sT@MWPmwR>n!bXml^zRGiyo_I+k*@LGo$If&LRwbI^d?j)McCywPuH@h z1-+&v;Q&eKyf)-vT4r@xs^89S4&qj6`grla-~IYsmg6K#M@yy+mrA8L&c5GHhr8R{ z{E1Mh6pvgkyt~Cs9|)C7F`QZ4<arRo8-LtssZ<84_fk2n9GrWzqjS<}d8<<^)xtUb zjm|ygT$R^awQep{nKKg)i^82ic&ep8KX!mpK5BZHc*yLSFFGofB5j#cMw(;l5BE-o zEgYRlrWKAWSFTK%H!78)i+sL6hINRuQK=LGP&tuIs~Y>kkdKoBjSgq<_tLC0K7W6^ z*5S+wLEEG<DwX08jbYv$=ZsOQ6i2vPFHpvFPYV?thZ#IxNCu;$z!pLud&Qij?u(zm z`|5H;`*~RF{6p6`C6WmRqf#mM+;BmgUrS?~{XDd>8TwdE=0q~lyqKPgN~P$v2rxSW z`kDD)ftx-yR{W@#J}G=qOr8@wCVzH%0)c9%@myr621F150D+(Y(FF%F=!ymsfS^E* zqcDzv5C}sMi~<`+KmZIv3N(mXFf<d)LEV}?7Kj`WELYJcaq2yyZ9DJ*!`4hIO<xQV z+hPzs*W<x31TaJ;=!p0MNtz@}CVPq#9D5v-6<|A&G_N*vre=qS*Xq`V<bSB1#;gg~ zR<q|hOYI5u4JIEEG2weNe*4J57-senpRuOazo2GVmss3f#)S}=A4p;4!+WE*1FJ&R zJ#`Sa`uA6D$wopq@`_XEYyl7$^iQEt)>Rv>`7UJYp1i{(QOM}CqHfDQ31JSX2QkI0 zFz(7DZ02BFK@9^aL6^*vN`D9nV8$&ZPVH@YPR5hTz$&*j00@>q(!xy8L%eS2Ti(}v zCy<OlbudzzipDlApP$XV{oqP4fWQrgsGF=tfcJZrFnqVK$k>($m6x6LF`5PlWr_3U zKo;S)^$NT4|F_kwzw8DqCZ>VsKhs>sR4o$?O?gWxbS|?>T?mj1a({fK4eGD+$}Y&i z^uSiPc@4BZ1;wsj$*T2|KY_(@nu?|$nj8u+jI?b}zbUm~_9sQDs6Q^@$4#t-<)#mv z-wt|>7&wB<$*!t=Q+2TaReg=MD3SiRnlW5~#3HywfCIkPp<Adm%30*dBh*$9=c}>) zV-<q)FC;1*lHJz5*ngqSR>YvAM2k~ImI^wGbJ!hewU))G`{<j|K~mZzhB2MYQ@^jp z;npy4ipFWzHVk&A3+GHM4JgVCawnoAhTN@{`$=TNSwji{_$xkO3Kn0~Ys3Z3BaH0h zh5ys+r=aU+bhy(_>kEzao)T*^uv@*j8^r(ktY8$;OY~MJ=~b(WY)So2?xu8jML$8M zgg42p=<tD<$fI?LvS7%BlF2Lfh|{eRk00k6T}a{y(f;Fx)R|nesi&oDN<bm~L3w}{ W6V80G9f_8&%zKhBZaUQvt?dd;sGvsx delta 1886 zcmV-k2ch`h4&V-u7k@M@I5#mZXEQS}VKFskVq;}xWC}@cb97;DV`VxZGc`3ZV>UB6 zEi*7;Vl6Z_W-~1~FgZ0XI5lH1H#s$9F*ac{3RXjGZ)0mZAbWiZ3e~y`y3GdT0L~dp z)=i}V00000D77#B08rI706I%F5K6#UQ2{85q9}?YD$b&eu79{-3N}O7l2mpR;KZ{2 zn+{@4)QCi;q$nk^F5UY}lDbGJHYRv&HT-RauFbizhI29aQ_<+yD7;K51?d3d0QUge z@{IH&cw?nZUm7X<AvBlHyK|e+jK~D)u!ofw_Akusjj{;1d*AXwaqOk$AnkkY%3P1Q z7iGQnW&8Mw9Dnz)t7}3;^vhNp!|lQa%?HER5PU&a?)!uP+3)xM)_)e1=^V38D5X=3 zcrF3DI!W2Su)M|FtGo2?&T&B*+oKAgtCJ*x<i-gWH2PnNvmS5M$ZP4kI>||^4l~~$ zyA|o`B$McrFI%7sv(c_j^4i?AS~#iHcgndb*|2O@rGL-H=B9Kq-J4QQ3x#n|IW`!q z?C*R8@ona@&^f8F2r)^*5u~qF*^vUEPN7|$r0c;P%UjG{@M3Pu*^XZbD@6JayT03X zi{&lalBygO?dRjEL02dF>f<fi&qrHQ6N4gHkFA)sQ5u%6PIBzei(oyrY)NMp7{%<3 zdJ6Ux%YRHW0{Ty9Yojs=j>PN{jX}`wXO~b42f8{*eIE=%7Kmm4!WWl)-_A>*t22zY z-}PPRoA{mAj_4nKp4%LSG4S&_#%25#vp4!upkLSyTQ=7PTV>Z{i0ub{ZXxOH(r+_j zZPbN8S0^c67>#!^tX&*}Hc{61>z}YpVx+^|FMkMz*dEcA0bQMBlm}TC1jj3#Rf(Cc zsz^-Lh;Bxt5^)M~0&!ZIRHhsg4o!;YW{3tgV>5H=>LiC|Ox}l?S0huJ$#{tA5W^vc z=Ed)e!VJs?7!5EPUNF2|c(G8ci$`ZeP^gQHO9e_>V!ArX77BAV6y-;stVdhW|L(QP zLVu?ic``6vog@+%;-mYCqYiT~+%K%;8A<=XV(tU;3m@q^-`%+mu^rN~OXxSDtCNIX zZu@i%)HI*hG^3jz5<0H|eVGQbI_=bN=N9zg_K5m;`M%%%`dyZ1B}ha}s128{PO_YR zzgUmE+Z^qQ(A7y2xm<a7i=#Xcx;n{mW`A*1=RquQan%y2i*v%MRgMX#RHf9)q$;6W zI5#C0%Y-VeP&n9>N}(`Vp)*g1RpHVgI-RXOKvq9aK4^M)JYsgx7ad)lWNo2S%9?}f zFn6rS7Ov7FPlY2XDTxZb(bY-1%;)=KS&ujyU7aL=I%$!os<B@jc}XkKz!?;OBY)9u z#)ILm;S91y(MD^LXOf{a?7JnUF}ga*5w6xtlyPvUg$mqZ29N)d!RRKj#m~oHu_ved z;wACEa4u*+A8Vce=ql+f@=P$gI?0_IE@(7t>0Gm)k2N(wA*%_cMc&K{`nl-pB)=8` zW<)=FW<FTpD36U5Jt`<q3LX?x=YNEbiPcVlYpC&HWT*s05C8xHNC43V2Qlc11`=Sf zK#Zd>j)4#eLlBBW8%IC@3_=Pth*~go48;SwHJFH2Fd%F?2kk;VXcww=-~-00nO2(4 z7#_C9Kn&JPXB`m05Sd^o^n)x9k}%ou6eKwIauTj!EYZT3wXZg0T(d*RYkzg?LUB|# zW0r(#8`yKErQ-?y^2rxLfPl3%zvXk}o-2LCX{@RGC#bmUq8G=dT?n=Lxht&Ltv5XM zKq^G>sVm1;VNq3wzfvSZ&zbt0ErQxX{}LL1Ts3TVav_oK;iE_r^^ZOib+@*&8Jb6T z{24#RZ0XOiX`b+=wiC=jmw(uke4NxWLj+RR*0wq)@TAqaja#AVtR8b(h~aq%?uI^M z`prRRPT(uuXYNcJ+VJy!Hk<p!R)T;J@vKg_H;wT38<xm^m(NJmmI#uUo&7P|2}oqg z_tGMZ@S$}GTlD{LtGR))1F&c@4JrSb3^FEkG11VJx0FKTGHdEW3V*pE$5+~-{vxmL zf*4EBeRas!LD@4G?DCbYS}*y)SR8><QJU1qp&G+T()NPsQhQ*3fG8CW<4FAYY>n6@ z`rP@=q}MzHN3)#x+LLco2l`9ZcTS5Eseh}}gCB-3f?K0?yRSvk7El%CEOMj@g~-HV zYP|oo3c>daiHd?`w|_NfFDEk*ne8ai;u4WnDMz7QaYtIM<u>XL{bqEKk~Rq!Oegci z?|E^eRScX$<Mg@>shsKmIpci>6g3mx`LKW?cWdQ*5}9z8djSBq#m8;I;y?8oaX|qI z!}@5+->Dg+0MUDNxS*Xd7KijMg=l18w|Yfa#Q*iIdK8gK^jFHBq=_p2AoZQJo5JB~ z{)9_u-DI}n!v|vG5UoYj4MPyhwO7m|j<QA`KTd1ZBMB3t<;Mk&Gr6RupO)4s0af}3 YejQs<I4l@DLYl8<_9QWfbkz{8?Jq=*EC2ui diff --git a/src/box/coll_id_def.c b/src/box/coll_id_def.c index 9fe0cda..43f3b63 100644 --- a/src/box/coll_id_def.c +++ b/src/box/coll_id_def.c @@ -55,8 +55,8 @@ icu_case_first_from_str(const char *str, uint32_t len) static int64_t icu_strength_from_str(const char *str, uint32_t len) { - return strnindex(coll_icu_strength_strs + 1, str, len, - coll_icu_strength_MAX - 1) + 1; + return strnindex(coll_icu_strength_strs, str, len, + coll_icu_strength_MAX); } const struct opt_def coll_icu_opts_reg[] = { 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)) + end + end +end + +local function upgrade_to_2_1_1() + update_collation_strength_field() +end + + local function get_version() local version = box.space._schema:get{'version'} if version == nil then @@ -605,7 +631,8 @@ local function upgrade(options) {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true}, {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true}, {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true}, - {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true} + {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true}, + {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true} } for _, handler in ipairs(handlers) do diff --git a/src/coll.c b/src/coll.c index 6a76f1f..9b719f2 100644 --- a/src/coll.c +++ b/src/coll.c @@ -193,22 +193,25 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def) return -1; } } - if (def->icu.strength != COLL_ICU_STRENGTH_DEFAULT) { - enum coll_icu_strength w = def->icu.strength; - UColAttributeValue v = - w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY : - w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY : - w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY : - w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY : - w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL : - UCOL_DEFAULT; - ucol_setAttribute(collator, UCOL_STRENGTH, v, &status); - if (U_FAILURE(status)) { - diag_set(CollationError, "failed to set strength: %s", - u_errorName(status)); - return -1; - } + /* + * Set "strength" option of collation. Default values is + * "primary". + */ + enum coll_icu_strength w = def->icu.strength; + UColAttributeValue v = + w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY : + w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY : + w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY : + w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY : + w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL : + UCOL_DEFAULT; + ucol_setAttribute(collator, UCOL_STRENGTH, v, &status); + if (U_FAILURE(status)) { + diag_set(CollationError, "failed to set strength: %s", + u_errorName(status)); + return -1; } + if (def->icu.numeric_collation != COLL_ICU_DEFAULT) { enum coll_icu_on_off w = def->icu.numeric_collation; UColAttributeValue v = w == COLL_ICU_ON ? UCOL_ON : diff --git a/src/coll_def.c b/src/coll_def.c index df58cac..1323643 100644 --- a/src/coll_def.c +++ b/src/coll_def.c @@ -54,7 +54,6 @@ const char *coll_icu_case_first_strs[] = { }; const char *coll_icu_strength_strs[] = { - "DEFAULT", "PRIMARY", "SECONDARY", "TERTIARY", diff --git a/src/coll_def.h b/src/coll_def.h index 7c20abf..2a1cb68 100644 --- a/src/coll_def.h +++ b/src/coll_def.h @@ -84,8 +84,7 @@ extern const char *coll_icu_case_first_strs[]; /** Strength ICU settings */ enum coll_icu_strength { - COLL_ICU_STRENGTH_DEFAULT = 0, - COLL_ICU_STRENGTH_PRIMARY, + COLL_ICU_STRENGTH_PRIMARY = 0, COLL_ICU_STRENGTH_SECONDARY, COLL_ICU_STRENGTH_TERTIARY, COLL_ICU_STRENGTH_QUATERNARY, diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index 506aca3..2532b70 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -5,7 +5,7 @@ box.space._schema:select{} --- - - ['cluster', '<cluster uuid>'] - ['max_id', 511] - - ['version', 2, 1, 0] + - ['version', 2, 1, 1] ... box.space._cluster:select{} --- diff --git a/test/box/ddl.result b/test/box/ddl.result index c9a8e96..396318f 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -484,14 +484,14 @@ box.space._collation:auto_increment{'test', 0, 'ICU', 'ru_RU', setmap{}} ... box.space._collation:select{} --- -- - [1, 'unicode', 1, 'ICU', '', {}] +- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] - [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}] - [3, 'test', 0, 'ICU', 'ru_RU', {}] ... test_run:cmd('restart server default') box.space._collation:select{} --- -- - [1, 'unicode', 1, 'ICU', '', {}] +- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] - [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}] - [3, 'test', 0, 'ICU', 'ru_RU', {}] ... @@ -512,7 +512,7 @@ utf8.cmp('abc', 'def') ... box.space._collation:replace(t) --- -- [1, 'unicode', 1, 'ICU', '', {}] +- [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] ... -- -- gh-2839: allow to store custom fields in field definition. diff --git a/test/box/tree_pk.result b/test/box/tree_pk.result index df3c78b..7366ed8 100644 --- a/test/box/tree_pk.result +++ b/test/box/tree_pk.result @@ -690,7 +690,7 @@ s:drop() ... --https://github.com/tarantool/tarantool/issues/2649 -- create standart index and alter it to collation index -box.internal.collation.create('test', 'ICU', 'ru-RU') +box.internal.collation.create('test', 'ICU', 'ru-RU', {strength = 'identical'}) --- ... box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength = 'secondary'}) diff --git a/test/box/tree_pk.test.lua b/test/box/tree_pk.test.lua index 1190ab4..b4ee65c 100644 --- a/test/box/tree_pk.test.lua +++ b/test/box/tree_pk.test.lua @@ -238,7 +238,7 @@ s:drop() --https://github.com/tarantool/tarantool/issues/2649 -- create standart index and alter it to collation index -box.internal.collation.create('test', 'ICU', 'ru-RU') +box.internal.collation.create('test', 'ICU', 'ru-RU', {strength = 'identical'}) box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength = 'secondary'}) s = box.schema.space.create('test') i1 = s:create_index('i1', { type = 'tree', parts = {{1, 'str'}}, unique = true }) 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 +--- +... +-- Should be true as default value is 'primary'. +count == 0 +--- +- true +... +box.sql.execute([[drop table tc]]) +--- +... +box.internal.collation.drop('c0') +--- +... +box.internal.collation.drop('c1') +--- +... +box.internal.collation.drop('c2') +--- +... +box.internal.collation.drop('c5') +--- +... diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua index 935dea8..1923503 100644 --- a/test/sql/collation.test.lua +++ b/test/sql/collation.test.lua @@ -43,3 +43,35 @@ cn:execute('select 1 limit ? collate not_exist', {1}) 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']]) +box.sql.execute([[select * from tc where s1 = 'a']]) +box.sql.execute([[select * from tc where s2 = 'a']]) +box.sql.execute([[select * from tc where s5 = '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 + +-- Should be true as default value is 'primary'. +count == 0 + +box.sql.execute([[drop table tc]]) +box.internal.collation.drop('c0') +box.internal.collation.drop('c1') +box.internal.collation.drop('c2') +box.internal.collation.drop('c5') diff --git a/test/unit/coll.cpp b/test/unit/coll.cpp index 94374a7..acbc7f4 100644 --- a/test/unit/coll.cpp +++ b/test/unit/coll.cpp @@ -51,6 +51,7 @@ manual_test() memset(&def, 0, sizeof(def)); snprintf(def.locale, sizeof(def.locale), "%s", "ru_RU"); def.type = COLL_TYPE_ICU; + def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL; struct coll *coll; cout << " -- default ru_RU -- " << endl; @@ -134,6 +135,7 @@ hash_test() struct coll *coll; /* Case sensitive */ + def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL; coll = coll_new(&def); assert(coll != NULL); cout << "Case sensitive" << endl; -- 2.7.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option. 2018-10-06 9:04 [tarantool-patches] " imeevma @ 2018-10-10 11:18 ` Vladislav Shpilevoy 2018-10-11 18:10 ` Imeev Mergen 0 siblings, 1 reply; 6+ messages in thread From: Vladislav Shpilevoy @ 2018-10-10 11:18 UTC (permalink / raw) To: imeevma, tarantool-patches 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<IA&xwVKpsbHZx@{ > 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<YnJQf6Sh@N@2|aY1nNsKg<N)~qVZO0ZrY}vD > zy%3s9<K4NfXf{L+b=bqk3;S2c?QOCMxO?C6GZ^+!^Gx$zyD--w?m=0vdDwn;L5_Ra > 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><f;VmNfL9 > 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? ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option. 2018-10-10 11:18 ` [tarantool-patches] " Vladislav Shpilevoy @ 2018-10-11 18:10 ` Imeev Mergen 2018-10-12 0:05 ` Vladislav Shpilevoy 0 siblings, 1 reply; 6+ messages in thread From: Imeev Mergen @ 2018-10-11 18:10 UTC (permalink / raw) To: Vladislav Shpilevoy, tarantool-patches [-- Attachment #1: Type: text/plain, Size: 21080 bytes --] Hello! Thank you for review! New patch below. This time I wasn't able to insert diff between two last versions, but I will do it in next patches. On 10/10/2018 02:18 PM, Vladislav Shpilevoy wrote: > > > 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. Ok, understood. > >> index >> 65739384a66d6ba4a538553ccf4677536ba15280..84b27c0d8b9d6d683692e2ef55034ffcc68d11d0 >> 100644 >> GIT binary patch >> delta 1884 >> zcmV-i2c!7l4&Dxs7k@P^HZUz`Ff(N|W@R=CNp5p=VQyn(Iv_J<IA&xwVKpsbHZx@{ >> 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<YnJQf6Sh@N@2|aY1nNsKg<N)~qVZO0ZrY}vD >> zy%3s9<K4NfXf{L+b=bqk3;S2c?QOCMxO?C6GZ^+!^Gx$zyD--w?m=0vdDwn;L5_Ra >> 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><f;VmNfL9 >> 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. Done. >> + 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? Fixed, removed unnecessary checks and added comments. *New patch:* commit e55064f796d9320035db5fb2e3d5030601ee7333 Author: Mergen Imeev <imeevma@gmail.com> Date: Tue Sep 25 21:30:36 2018 +0300 box: update collation strength option. 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 diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index 6573938..3d3232f 100644 Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap differ diff --git a/src/box/coll_id_def.c b/src/box/coll_id_def.c index 9fe0cda..43f3b63 100644 --- a/src/box/coll_id_def.c +++ b/src/box/coll_id_def.c @@ -55,8 +55,8 @@ icu_case_first_from_str(const char *str, uint32_t len) static int64_t icu_strength_from_str(const char *str, uint32_t len) { - return strnindex(coll_icu_strength_strs + 1, str, len, - coll_icu_strength_MAX - 1) + 1; + return strnindex(coll_icu_strength_strs, str, len, + coll_icu_strength_MAX); } const struct opt_def coll_icu_opts_reg[] = { diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index d9c2ae4..66dcef4 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -578,6 +578,27 @@ 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] + for _, collation in ipairs(_collation:select()) do + if (collation.opts.strength == nil) then + local new_collation = _collation:get{collation.id}:totable() + new_collation[6].strength = 'identical' + _collation:delete{collation.id} + _collation:insert(new_collation) + end + end +end + +local function upgrade_to_2_1_1() + update_collation_strength_field() +end + + local function get_version() local version = box.space._schema:get{'version'} if version == nil then @@ -605,7 +626,8 @@ local function upgrade(options) {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true}, {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true}, {version = mkversion(1, 10, 2), func = upgrade_to_1_10_2, auto = true}, - {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true} + {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0, auto = true}, + {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1, auto = true} } for _, handler in ipairs(handlers) do diff --git a/src/coll.c b/src/coll.c index 6a76f1f..9b719f2 100644 --- a/src/coll.c +++ b/src/coll.c @@ -193,22 +193,25 @@ coll_icu_init_cmp(struct coll *coll, const struct coll_def *def) return -1; } } - if (def->icu.strength != COLL_ICU_STRENGTH_DEFAULT) { - enum coll_icu_strength w = def->icu.strength; - UColAttributeValue v = - w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY : - w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY : - w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY : - w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY : - w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL : - UCOL_DEFAULT; - ucol_setAttribute(collator, UCOL_STRENGTH, v, &status); - if (U_FAILURE(status)) { - diag_set(CollationError, "failed to set strength: %s", - u_errorName(status)); - return -1; - } + /* + * Set "strength" option of collation. Default values is + * "primary". + */ + enum coll_icu_strength w = def->icu.strength; + UColAttributeValue v = + w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY : + w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY : + w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY : + w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY : + w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL : + UCOL_DEFAULT; + ucol_setAttribute(collator, UCOL_STRENGTH, v, &status); + if (U_FAILURE(status)) { + diag_set(CollationError, "failed to set strength: %s", + u_errorName(status)); + return -1; } + if (def->icu.numeric_collation != COLL_ICU_DEFAULT) { enum coll_icu_on_off w = def->icu.numeric_collation; UColAttributeValue v = w == COLL_ICU_ON ? UCOL_ON : diff --git a/src/coll_def.c b/src/coll_def.c index df58cac..1323643 100644 --- a/src/coll_def.c +++ b/src/coll_def.c @@ -54,7 +54,6 @@ const char *coll_icu_case_first_strs[] = { }; const char *coll_icu_strength_strs[] = { - "DEFAULT", "PRIMARY", "SECONDARY", "TERTIARY", diff --git a/src/coll_def.h b/src/coll_def.h index 7c20abf..2a1cb68 100644 --- a/src/coll_def.h +++ b/src/coll_def.h @@ -84,8 +84,7 @@ extern const char *coll_icu_case_first_strs[]; /** Strength ICU settings */ enum coll_icu_strength { - COLL_ICU_STRENGTH_DEFAULT = 0, - COLL_ICU_STRENGTH_PRIMARY, + COLL_ICU_STRENGTH_PRIMARY = 0, COLL_ICU_STRENGTH_SECONDARY, COLL_ICU_STRENGTH_TERTIARY, COLL_ICU_STRENGTH_QUATERNARY, diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result index 506aca3..2532b70 100644 --- a/test/box-py/bootstrap.result +++ b/test/box-py/bootstrap.result @@ -5,7 +5,7 @@ box.space._schema:select{} --- - - ['cluster', '<cluster uuid>'] - ['max_id', 511] - - ['version', 2, 1, 0] + - ['version', 2, 1, 1] ... box.space._cluster:select{} --- diff --git a/test/box/ddl.result b/test/box/ddl.result index c9a8e96..396318f 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -484,14 +484,14 @@ box.space._collation:auto_increment{'test', 0, 'ICU', 'ru_RU', setmap{}} ... box.space._collation:select{} --- -- - [1, 'unicode', 1, 'ICU', '', {}] +- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] - [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}] - [3, 'test', 0, 'ICU', 'ru_RU', {}] ... test_run:cmd('restart server default') box.space._collation:select{} --- -- - [1, 'unicode', 1, 'ICU', '', {}] +- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] - [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}] - [3, 'test', 0, 'ICU', 'ru_RU', {}] ... @@ -512,7 +512,7 @@ utf8.cmp('abc', 'def') ... box.space._collation:replace(t) --- -- [1, 'unicode', 1, 'ICU', '', {}] +- [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}] ... -- -- gh-2839: allow to store custom fields in field definition. diff --git a/test/box/tree_pk.result b/test/box/tree_pk.result index df3c78b..7366ed8 100644 --- a/test/box/tree_pk.result +++ b/test/box/tree_pk.result @@ -690,7 +690,7 @@ s:drop() ... --https://github.com/tarantool/tarantool/issues/2649 -- create standart index and alter it to collation index -box.internal.collation.create('test', 'ICU', 'ru-RU') +box.internal.collation.create('test', 'ICU', 'ru-RU', {strength = 'identical'}) --- ... box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength = 'secondary'}) diff --git a/test/box/tree_pk.test.lua b/test/box/tree_pk.test.lua index 1190ab4..b4ee65c 100644 --- a/test/box/tree_pk.test.lua +++ b/test/box/tree_pk.test.lua @@ -238,7 +238,7 @@ s:drop() --https://github.com/tarantool/tarantool/issues/2649 -- create standart index and alter it to collation index -box.internal.collation.create('test', 'ICU', 'ru-RU') +box.internal.collation.create('test', 'ICU', 'ru-RU', {strength = 'identical'}) box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength = 'secondary'}) s = box.schema.space.create('test') i1 = s:create_index('i1', { type = 'tree', parts = {{1, 'str'}}, unique = true }) diff --git a/test/sql/collation.result b/test/sql/collation.result index 79ba9ab..5fdb8ee 100644 --- a/test/sql/collation.result +++ b/test/sql/collation.result @@ -110,3 +110,77 @@ 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, 'â', 'â', 'â', 'â')]]) +--- +... +-- Should be same as the next one. +box.sql.execute([[select * from tc where s0 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] + - [2, 'A', 'A', 'A', 'A'] + - [3, 'á', 'á', 'á', 'á'] + - [4, 'â', 'â', 'â', 'â'] +... +-- Should return all records. +box.sql.execute([[select * from tc where s1 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] + - [2, 'A', 'A', 'A', 'A'] + - [3, 'á', 'á', 'á', 'á'] + - [4, 'â', 'â', 'â', 'â'] +... +-- Should return two records. +box.sql.execute([[select * from tc where s2 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] + - [2, 'A', 'A', 'A', 'A'] +... +-- Should return one record. +box.sql.execute([[select * from tc where s5 = 'a']]) +--- +- - [1, 'a', 'a', 'a', 'a'] +... +box.sql.execute([[drop table tc]]) +--- +... +box.internal.collation.drop('c0') +--- +... +box.internal.collation.drop('c1') +--- +... +box.internal.collation.drop('c2') +--- +... +box.internal.collation.drop('c5') +--- +... diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua index 935dea8..130245b 100644 --- a/test/sql/collation.test.lua +++ b/test/sql/collation.test.lua @@ -43,3 +43,32 @@ cn:execute('select 1 limit ? collate not_exist', {1}) 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, 'â', 'â', 'â', 'â')]]) +-- Should be same as the next one. +box.sql.execute([[select * from tc where s0 = 'a']]) +-- Should return all records. +box.sql.execute([[select * from tc where s1 = 'a']]) +-- Should return two records. +box.sql.execute([[select * from tc where s2 = 'a']]) +-- Should return one record. +box.sql.execute([[select * from tc where s5 = 'a']]) + +box.sql.execute([[drop table tc]]) +box.internal.collation.drop('c0') +box.internal.collation.drop('c1') +box.internal.collation.drop('c2') +box.internal.collation.drop('c5') diff --git a/test/unit/coll.cpp b/test/unit/coll.cpp index 94374a7..acbc7f4 100644 --- a/test/unit/coll.cpp +++ b/test/unit/coll.cpp @@ -51,6 +51,7 @@ manual_test() memset(&def, 0, sizeof(def)); snprintf(def.locale, sizeof(def.locale), "%s", "ru_RU"); def.type = COLL_TYPE_ICU; + def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL; struct coll *coll; cout << " -- default ru_RU -- " << endl; @@ -134,6 +135,7 @@ hash_test() struct coll *coll; /* Case sensitive */ + def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL; coll = coll_new(&def); assert(coll != NULL); cout << "Case sensitive" << endl; [-- Attachment #2: Type: text/html, Size: 27192 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option. 2018-10-11 18:10 ` Imeev Mergen @ 2018-10-12 0:05 ` Vladislav Shpilevoy 0 siblings, 0 replies; 6+ messages in thread From: Vladislav Shpilevoy @ 2018-10-12 0:05 UTC (permalink / raw) To: Imeev Mergen, tarantool-patches Thanks for the fixes! See 1 comment below. > diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua > index d9c2ae4..66dcef4 100644 > --- a/src/box/lua/upgrade.lua > +++ b/src/box/lua/upgrade.lua > @@ -578,6 +578,27 @@ 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] > + for _, collation in ipairs(_collation:select()) do > + if (collation.opts.strength == nil) then > + local new_collation = _collation:get{collation.id}:totable() > + new_collation[6].strength = 'identical' > + _collation:delete{collation.id} > + _collation:insert(new_collation) Why did you remove replace from my diff and replaced it with delete + insert? collation.id is a primary index and it is the same in the new and old collation. It makes no sense to do delete + insert, when you can do one replace. > + end > + end > +end > + > +local function upgrade_to_2_1_1() > + update_collation_strength_field() > +end > + > + ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-15 18:30 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-12 18:07 [tarantool-patches] Re: [tarantool-patches] Re: [PATCH v2 1/1] box: update collation strength option Мерген Имеев 2018-10-13 12:17 ` Vladislav Shpilevoy 2018-10-15 18:30 ` Imeev Mergen -- strict thread matches above, loose matches on Subject: below -- 2018-10-06 9:04 [tarantool-patches] " imeevma 2018-10-10 11:18 ` [tarantool-patches] " Vladislav Shpilevoy 2018-10-11 18:10 ` Imeev Mergen 2018-10-12 0:05 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox