<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
Hello! Thank you for review! After discussion decided to left as<br>
it is now.<br>
<br>
Nikita, can you do next review? Links and diff below.<br>
<br>
<div class="moz-cite-prefix">On 10/13/2018 03:17 PM, Vladislav
Shpilevoy wrote:<br>
</div>
<blockquote type="cite"
cite="mid:97830699-35df-f312-fe87-c0a8816eb3e5@tarantool.org">Hi!
<br>
<br>
<blockquote type="cite"> Replace couldn't be used in space
box.space._collation:
<br>
tarantool> function update_collation_strength_field()
<br>
> local _collation =
<br>
box.space[box.schema.COLLATION_ID]
<br>
> for _, collation in
ipairs(_collation:select()) do
<br>
> if (collation.opts.strength == nil)
then
<br>
> local new_collation =
<br>
_collation:get{collation.id}:totable()
<br>
> new_collation[6].strength =
'identical'
<br>
> _collation:replace(new_collation)
<br>
> end
<br>
> end
<br>
> end
<br>
---
<br>
...
<br>
tarantool> update_collation_strength_field()
<br>
---
<br>
- error: collation does not support alter
<br>
...
<br>
</blockquote>
<br>
I just found that it is not normal. Triggers on system
<br>
spaces are turned off before upgrade. But for _collation
<br>
space they are not. Please, turn off the triggers and use
<br>
replace. See function set_system_triggers in upgrade.lua.
<br>
<br>
<blockquote type="cite"> >>+ end
<br>
>> + end
<br>
>> +end
<br>
>> +
<br>
>> +local function upgrade_to_2_1_1()
<br>
>> + update_collation_strength_field()
<br>
>> +end
<br>
>> +
<br>
>> +
<br>
<br>
</blockquote>
</blockquote>
<b>Issue:</b> <a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/3573">https://github.com/tarantool/tarantool/issues/3573</a><br>
<b>Branch:</b>
<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/tree/imeevma/gh-3573-fix-collation-strength">https://github.com/tarantool/tarantool/tree/imeevma/gh-3573-fix-collation-strength</a><br>
<b><br>
Patch:</b><br>
<br>
commit e55064f796d9320035db5fb2e3d5030601ee7333<br>
Author: Mergen Imeev <a class="moz-txt-link-rfc2396E" href="mailto:imeevma@gmail.com"><imeevma@gmail.com></a><br>
Date: Tue Sep 25 21:30:36 2018 +0300<br>
<br>
box: update collation strength option.<br>
<br>
At the moment all collations that don't have their "strength"<br>
option set works the same way as ones with this option set to<br>
"identical". It is not efficient, according to ICU Comparison<br>
Levels. This patch updates this option of old collations and set<br>
its default value to "primary" for new ones.<br>
<br>
Closes #3573<br>
<br>
diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap<br>
index 6573938..3d3232f 100644<br>
Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap
differ<br>
diff --git a/src/box/coll_id_def.c b/src/box/coll_id_def.c<br>
index 9fe0cda..43f3b63 100644<br>
--- a/src/box/coll_id_def.c<br>
+++ b/src/box/coll_id_def.c<br>
@@ -55,8 +55,8 @@ icu_case_first_from_str(const char *str, uint32_t
len)<br>
static int64_t<br>
icu_strength_from_str(const char *str, uint32_t len)<br>
{<br>
- return strnindex(coll_icu_strength_strs + 1, str, len,<br>
- coll_icu_strength_MAX - 1) + 1;<br>
+ return strnindex(coll_icu_strength_strs, str, len,<br>
+ coll_icu_strength_MAX);<br>
}<br>
<br>
const struct opt_def coll_icu_opts_reg[] = {<br>
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua<br>
index d9c2ae4..66dcef4 100644<br>
--- a/src/box/lua/upgrade.lua<br>
+++ b/src/box/lua/upgrade.lua<br>
@@ -578,6 +578,27 @@ local function upgrade_to_2_1_0()<br>
box.space._schema:format(format)<br>
end<br>
<br>
+--------------------------------------------------------------------------------<br>
+-- Tarantool 2.1.1<br>
+--------------------------------------------------------------------------------<br>
+<br>
+function update_collation_strength_field()<br>
+ local _collation = box.space[box.schema.COLLATION_ID]<br>
+ for _, collation in ipairs(_collation:select()) do<br>
+ if (collation.opts.strength == nil) then<br>
+ local new_collation =
_collation:get{collation.id}:totable()<br>
+ new_collation[6].strength = 'identical'<br>
+ _collation:delete{collation.id}<br>
+ _collation:insert(new_collation)<br>
+ end<br>
+ end<br>
+end<br>
+<br>
+local function upgrade_to_2_1_1()<br>
+ update_collation_strength_field()<br>
+end<br>
+<br>
+<br>
local function get_version()<br>
local version = box.space._schema:get{'version'}<br>
if version == nil then<br>
@@ -605,7 +626,8 @@ local function upgrade(options)<br>
{version = mkversion(1, 7, 7), func = upgrade_to_1_7_7,
auto = true},<br>
{version = mkversion(1, 10, 0), func = upgrade_to_1_10_0,
auto = true},<br>
{version = mkversion(1, 10, 2), func = upgrade_to_1_10_2,
auto = true},<br>
- {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0,
auto = true}<br>
+ {version = mkversion(2, 1, 0), func = upgrade_to_2_1_0,
auto = true},<br>
+ {version = mkversion(2, 1, 1), func = upgrade_to_2_1_1,
auto = true}<br>
}<br>
<br>
for _, handler in ipairs(handlers) do<br>
diff --git a/src/coll.c b/src/coll.c<br>
index 6a76f1f..9b719f2 100644<br>
--- a/src/coll.c<br>
+++ b/src/coll.c<br>
@@ -193,22 +193,25 @@ coll_icu_init_cmp(struct coll *coll, const
struct coll_def *def)<br>
return -1;<br>
}<br>
}<br>
- if (def->icu.strength != COLL_ICU_STRENGTH_DEFAULT) {<br>
- enum coll_icu_strength w = def->icu.strength;<br>
- UColAttributeValue v =<br>
- w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY :<br>
- w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY :<br>
- w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY :<br>
- w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY :<br>
- w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL :<br>
- UCOL_DEFAULT;<br>
- ucol_setAttribute(collator, UCOL_STRENGTH, v, &status);<br>
- if (U_FAILURE(status)) {<br>
- diag_set(CollationError, "failed to set strength: %s",<br>
- u_errorName(status));<br>
- return -1;<br>
- }<br>
+ /*<br>
+ * Set "strength" option of collation. Default values is<br>
+ * "primary".<br>
+ */<br>
+ enum coll_icu_strength w = def->icu.strength;<br>
+ UColAttributeValue v =<br>
+ w == COLL_ICU_STRENGTH_PRIMARY ? UCOL_PRIMARY :<br>
+ w == COLL_ICU_STRENGTH_SECONDARY ? UCOL_SECONDARY :<br>
+ w == COLL_ICU_STRENGTH_TERTIARY ? UCOL_TERTIARY :<br>
+ w == COLL_ICU_STRENGTH_QUATERNARY ? UCOL_QUATERNARY :<br>
+ w == COLL_ICU_STRENGTH_IDENTICAL ? UCOL_IDENTICAL :<br>
+ UCOL_DEFAULT;<br>
+ ucol_setAttribute(collator, UCOL_STRENGTH, v, &status);<br>
+ if (U_FAILURE(status)) {<br>
+ diag_set(CollationError, "failed to set strength: %s",<br>
+ u_errorName(status));<br>
+ return -1;<br>
}<br>
+<br>
if (def->icu.numeric_collation != COLL_ICU_DEFAULT) {<br>
enum coll_icu_on_off w = def->icu.numeric_collation;<br>
UColAttributeValue v = w == COLL_ICU_ON ? UCOL_ON :<br>
diff --git a/src/coll_def.c b/src/coll_def.c<br>
index df58cac..1323643 100644<br>
--- a/src/coll_def.c<br>
+++ b/src/coll_def.c<br>
@@ -54,7 +54,6 @@ const char *coll_icu_case_first_strs[] = {<br>
};<br>
<br>
const char *coll_icu_strength_strs[] = {<br>
- "DEFAULT",<br>
"PRIMARY",<br>
"SECONDARY",<br>
"TERTIARY",<br>
diff --git a/src/coll_def.h b/src/coll_def.h<br>
index 7c20abf..2a1cb68 100644<br>
--- a/src/coll_def.h<br>
+++ b/src/coll_def.h<br>
@@ -84,8 +84,7 @@ extern const char *coll_icu_case_first_strs[];<br>
<br>
/** Strength ICU settings */<br>
enum coll_icu_strength {<br>
- COLL_ICU_STRENGTH_DEFAULT = 0,<br>
- COLL_ICU_STRENGTH_PRIMARY,<br>
+ COLL_ICU_STRENGTH_PRIMARY = 0,<br>
COLL_ICU_STRENGTH_SECONDARY,<br>
COLL_ICU_STRENGTH_TERTIARY,<br>
COLL_ICU_STRENGTH_QUATERNARY,<br>
diff --git a/test/box-py/bootstrap.result
b/test/box-py/bootstrap.result<br>
index 506aca3..2532b70 100644<br>
--- a/test/box-py/bootstrap.result<br>
+++ b/test/box-py/bootstrap.result<br>
@@ -5,7 +5,7 @@ box.space._schema:select{}<br>
---<br>
- - ['cluster', '<cluster uuid>']<br>
- ['max_id', 511]<br>
- - ['version', 2, 1, 0]<br>
+ - ['version', 2, 1, 1]<br>
...<br>
box.space._cluster:select{}<br>
---<br>
diff --git a/test/box/ddl.result b/test/box/ddl.result<br>
index c9a8e96..396318f 100644<br>
--- a/test/box/ddl.result<br>
+++ b/test/box/ddl.result<br>
@@ -484,14 +484,14 @@ box.space._collation:auto_increment{'test', 0,
'ICU', 'ru_RU', setmap{}}<br>
...<br>
box.space._collation:select{}<br>
---<br>
-- - [1, 'unicode', 1, 'ICU', '', {}]<br>
+- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}]<br>
- [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}]<br>
- [3, 'test', 0, 'ICU', 'ru_RU', {}]<br>
...<br>
test_run:cmd('restart server default')<br>
box.space._collation:select{}<br>
---<br>
-- - [1, 'unicode', 1, 'ICU', '', {}]<br>
+- - [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}]<br>
- [2, 'unicode_ci', 1, 'ICU', '', {'strength': 'primary'}]<br>
- [3, 'test', 0, 'ICU', 'ru_RU', {}]<br>
...<br>
@@ -512,7 +512,7 @@ utf8.cmp('abc', 'def')<br>
...<br>
box.space._collation:replace(t)<br>
---<br>
-- [1, 'unicode', 1, 'ICU', '', {}]<br>
+- [1, 'unicode', 1, 'ICU', '', {'strength': 'identical'}]<br>
...<br>
--<br>
-- gh-2839: allow to store custom fields in field definition.<br>
diff --git a/test/box/tree_pk.result b/test/box/tree_pk.result<br>
index df3c78b..7366ed8 100644<br>
--- a/test/box/tree_pk.result<br>
+++ b/test/box/tree_pk.result<br>
@@ -690,7 +690,7 @@ s:drop()<br>
...<br>
--https://github.com/tarantool/tarantool/issues/2649<br>
-- create standart index and alter it to collation index<br>
-box.internal.collation.create('test', 'ICU', 'ru-RU')<br>
+box.internal.collation.create('test', 'ICU', 'ru-RU', {strength =
'identical'})<br>
---<br>
...<br>
box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength
= 'secondary'})<br>
diff --git a/test/box/tree_pk.test.lua b/test/box/tree_pk.test.lua<br>
index 1190ab4..b4ee65c 100644<br>
--- a/test/box/tree_pk.test.lua<br>
+++ b/test/box/tree_pk.test.lua<br>
@@ -238,7 +238,7 @@ s:drop()<br>
<br>
--https://github.com/tarantool/tarantool/issues/2649<br>
-- create standart index and alter it to collation index<br>
-box.internal.collation.create('test', 'ICU', 'ru-RU')<br>
+box.internal.collation.create('test', 'ICU', 'ru-RU', {strength =
'identical'})<br>
box.internal.collation.create('test-ci', 'ICU', 'ru-RU', {strength
= 'secondary'})<br>
s = box.schema.space.create('test')<br>
i1 = s:create_index('i1', { type = 'tree', parts = {{1, 'str'}},
unique = true })<br>
diff --git a/test/sql/collation.result b/test/sql/collation.result<br>
index 79ba9ab..5fdb8ee 100644<br>
--- a/test/sql/collation.result<br>
+++ b/test/sql/collation.result<br>
@@ -110,3 +110,77 @@ cn:close()<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
---<br>
...<br>
+--<br>
+-- gh-3573: Strength in the _collation space<br>
+-- Collation without 'strength' option set now works as one with<br>
+-- 'strength' set to 'primary'.<br>
+--<br>
+box.internal.collation.create('c0', 'ICU', 'unicode')<br>
+---<br>
+...<br>
+box.internal.collation.create('c1', 'ICU', 'unicode',
{strength='primary'})<br>
+---<br>
+...<br>
+box.internal.collation.create('c2', 'ICU', 'unicode',
{strength='secondary'})<br>
+---<br>
+...<br>
+box.internal.collation.create('c5', 'ICU', 'unicode',
{strength='identical'})<br>
+---<br>
+...<br>
+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")]])<br>
+---<br>
+...<br>
+box.sql.execute([[insert into tc values (null, 'a', 'a', 'a',
'a')]])<br>
+---<br>
+...<br>
+box.sql.execute([[insert into tc values (null, 'A', 'A', 'A',
'A')]])<br>
+---<br>
+...<br>
+box.sql.execute([[insert into tc values (null, 'á', 'á', 'á',
'á')]])<br>
+---<br>
+...<br>
+box.sql.execute([[insert into tc values (null, 'â', 'â', 'â',
'â')]])<br>
+---<br>
+...<br>
+-- Should be same as the next one.<br>
+box.sql.execute([[select * from tc where s0 = 'a']])<br>
+---<br>
+- - [1, 'a', 'a', 'a', 'a']<br>
+ - [2, 'A', 'A', 'A', 'A']<br>
+ - [3, 'á', 'á', 'á', 'á']<br>
+ - [4, 'â', 'â', 'â', 'â']<br>
+...<br>
+-- Should return all records.<br>
+box.sql.execute([[select * from tc where s1 = 'a']])<br>
+---<br>
+- - [1, 'a', 'a', 'a', 'a']<br>
+ - [2, 'A', 'A', 'A', 'A']<br>
+ - [3, 'á', 'á', 'á', 'á']<br>
+ - [4, 'â', 'â', 'â', 'â']<br>
+...<br>
+-- Should return two records.<br>
+box.sql.execute([[select * from tc where s2 = 'a']])<br>
+---<br>
+- - [1, 'a', 'a', 'a', 'a']<br>
+ - [2, 'A', 'A', 'A', 'A']<br>
+...<br>
+-- Should return one record.<br>
+box.sql.execute([[select * from tc where s5 = 'a']])<br>
+---<br>
+- - [1, 'a', 'a', 'a', 'a']<br>
+...<br>
+box.sql.execute([[drop table tc]])<br>
+---<br>
+...<br>
+box.internal.collation.drop('c0')<br>
+---<br>
+...<br>
+box.internal.collation.drop('c1')<br>
+---<br>
+...<br>
+box.internal.collation.drop('c2')<br>
+---<br>
+...<br>
+box.internal.collation.drop('c5')<br>
+---<br>
+...<br>
diff --git a/test/sql/collation.test.lua
b/test/sql/collation.test.lua<br>
index 935dea8..130245b 100644<br>
--- a/test/sql/collation.test.lua<br>
+++ b/test/sql/collation.test.lua<br>
@@ -43,3 +43,32 @@ cn:execute('select 1 limit ? collate not_exist',
{1})<br>
<br>
cn:close()<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
+<br>
+--<br>
+-- gh-3573: Strength in the _collation space<br>
+-- Collation without 'strength' option set now works as one with<br>
+-- 'strength' set to 'primary'.<br>
+--<br>
+box.internal.collation.create('c0', 'ICU', 'unicode')<br>
+box.internal.collation.create('c1', 'ICU', 'unicode',
{strength='primary'})<br>
+box.internal.collation.create('c2', 'ICU', 'unicode',
{strength='secondary'})<br>
+box.internal.collation.create('c5', 'ICU', 'unicode',
{strength='identical'})<br>
+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")]])<br>
+box.sql.execute([[insert into tc values (null, 'a', 'a', 'a',
'a')]])<br>
+box.sql.execute([[insert into tc values (null, 'A', 'A', 'A',
'A')]])<br>
+box.sql.execute([[insert into tc values (null, 'á', 'á', 'á',
'á')]])<br>
+box.sql.execute([[insert into tc values (null, 'â', 'â', 'â',
'â')]])<br>
+-- Should be same as the next one.<br>
+box.sql.execute([[select * from tc where s0 = 'a']])<br>
+-- Should return all records.<br>
+box.sql.execute([[select * from tc where s1 = 'a']])<br>
+-- Should return two records.<br>
+box.sql.execute([[select * from tc where s2 = 'a']])<br>
+-- Should return one record.<br>
+box.sql.execute([[select * from tc where s5 = 'a']])<br>
+<br>
+box.sql.execute([[drop table tc]])<br>
+box.internal.collation.drop('c0')<br>
+box.internal.collation.drop('c1')<br>
+box.internal.collation.drop('c2')<br>
+box.internal.collation.drop('c5')<br>
diff --git a/test/unit/coll.cpp b/test/unit/coll.cpp<br>
index 94374a7..acbc7f4 100644<br>
--- a/test/unit/coll.cpp<br>
+++ b/test/unit/coll.cpp<br>
@@ -51,6 +51,7 @@ manual_test()<br>
memset(&def, 0, sizeof(def));<br>
snprintf(def.locale, sizeof(def.locale), "%s", "ru_RU");<br>
def.type = COLL_TYPE_ICU;<br>
+ def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL;<br>
struct coll *coll;<br>
<br>
cout << " -- default ru_RU -- " << endl;<br>
@@ -134,6 +135,7 @@ hash_test()<br>
struct coll *coll;<br>
<br>
/* Case sensitive */<br>
+ def.icu.strength = COLL_ICU_STRENGTH_IDENTICAL;<br>
coll = coll_new(&def);<br>
assert(coll != NULL);<br>
cout << "Case sensitive" << endl;<br>
<br>
</body>
</html>