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 DF6D92AF23 for ; Wed, 27 Mar 2019 10:08:49 -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 0bsloLTyfWOt for ; Wed, 27 Mar 2019 10:08:49 -0400 (EDT) Received: from smtp46.i.mail.ru (smtp46.i.mail.ru [94.100.177.106]) (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 93CFD28217 for ; Wed, 27 Mar 2019 10:08:49 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] [PATCH] sql: set explicit default collation's strength From: i.koptelov In-Reply-To: <8ab84a8f-213e-4bcd-217d-e5f694b70693@tarantool.org> Date: Wed, 27 Mar 2019 17:08:47 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <17D31B00-F24C-48C8-BE9D-CA4D32D35484@tarantool.org> References: <20190326203456.76617-1-ivan.koptelov@tarantool.org> <8ab84a8f-213e-4bcd-217d-e5f694b70693@tarantool.org> 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: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy Thank you for the review. > On 27 Mar 2019, at 14:59, Vladislav Shpilevoy = wrote: >=20 > Hi! Thanks for the patch! See 5 comments below. >=20 > On 26/03/2019 23:34, Ivan Koptelov wrote: >> Before the patch, collations with no strength set used >> tertiary strength. But it was not easy to understand it, >> because box.space._collation:select{} would return >> ... [1, 'unicode', 1, 'ICU', '', {}] ... for such collations. >> After the patch default value is set explicitly, so >> user would observe : ... [1, 'unicode', 1, 'ICU', '', >> {strength=3D'tertiary'}] ... >>=20 >> Note that box/stat.test.lua is temporary disabled with this >> patch. It is done so because the patch is meant for the 2.1.2 >> release. Current tarantool version is 2.1.2, so upgrade is done >> (using upgrade.lua) and because of it box/stat is broken (it >> does not expect changes in upgrade) But after the release would >> be made, box/stat would work again, because no changing would be >> done in upgrade.lua. To resume, after we set tarantool >> version to =3D> 2.1.2 box/stat should be enabled again. >=20 > 1. Why so complex? Just update box stat test output. We have > upgrade_to_2_1_2 on some other branches in review, but they do not > have these problems. >=20 > What is more, I enabled that test back, and it passes. So what > a problem? Seems like this test works fine if bootstrap.snap is updated. So I just enable the test and fix commit message. >=20 >>=20 >> Closes #3573 >>=20 >> @TarantoolBot document >> Title: default collation strength is explicit tertiary now >> Before the patch we already have tertiary strength is default >> strength for collations, but it was explicit: >=20 > 2. 'It was explicit', 'it's just become explicit'. A guess, > the first one was 'implicit=E2=80=99. You are right. Sorry, fixed now. >=20 >> [1, 'unicode', 1, 'ICU', '', {}] >> After the patch it's just become explicit: >> 1, 'unicode', 1, 'ICU', '', {'strength' =3D 'tertiary'}] >>=20 >> Also please fix this = https://tarantool.io/en/doc/2.1/book/box/data_model/#collations >> There is line saying: "unicode collation observes all weights, >> from L1 to Ln (identical)" It was not true and now this fact >> would just become obvious. >> --- >> Branch = https://github.com/tarantool/tarantool/tree/sudobobo/gh-3573-add-explicit-= default-coll-strength >> Issue https://github.com/tarantool/tarantool/issues/3573 >>=20 >> src/box/bootstrap.snap | Bin 1834 -> 1840 bytes >> src/box/lua/schema.lua | 4 ++++ >> src/box/lua/upgrade.lua | 26 ++++++++++++++++++++++++-- >> src/lua/utf8.c | 1 + >> test/app-tap/tarantoolctl.test.lua | 10 +++++----- >> test/box-py/bootstrap.result | 2 +- >> test/box/ddl.result | 6 +++--- >> test/box/suite.ini | 2 +- >> test/sql/collation.result | 14 ++++++++++++++ >> test/sql/collation.test.lua | 8 ++++++++ >> test/unit/coll.cpp | 2 ++ >> 11 files changed, 63 insertions(+), 12 deletions(-) >>=20 >> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua >> index dc7328714..440061558 100644 >> --- a/src/box/lua/upgrade.lua >> +++ b/src/box/lua/upgrade.lua >> @@ -400,7 +400,7 @@ local function create_collation_space() >> box.space._index:insert{_collation.id, 1, 'name', 'tree', {unique = =3D true}, {{1, 'string'}}} >>=20 >> log.info("create predefined collations") >> - box.space._collation:replace{1, "unicode", ADMIN, "ICU", "", = setmap{}} >> + box.space._collation:replace{1, "unicode", ADMIN, "ICU", "", = {strength=3D'tertiary'}} >=20 > 3. Please, do not touch old upgrade scripts. They are 'read-only=E2=80=99= . Moreover, this change does not really do anything. Removed. >=20 >> box.space._collation:replace{2, "unicode_ci", ADMIN, "ICU", "", = {strength=3D'primary'}} >>=20 >> local _priv =3D box.space[box.schema.PRIV_ID] >> @@ -632,6 +632,27 @@ local function upgrade_to_2_1_1() >> end >> end >>=20 >> = +-------------------------------------------------------------------------= ------- >> +-- Tarantool 2.1.2 >> = +-------------------------------------------------------------------------= ------- >> + >> +local function update_collation_strength_field() >> + local _collation =3D box.space[box.schema.COLLATION_ID] >> + for _, collation in ipairs(_collation:select()) do >> + if collation.opts.strength =3D=3D nil and collation.name ~=3D = 'none' and >> + collation.name ~=3D 'binary' then >> + local new_collation =3D = _collation:get{collation.id}:totable() >> + new_collation[6].strength =3D 'tertiary' >> + _collation:delete{collation.id} >> + _collation:insert(new_collation) >=20 > 4. 'replace=E2=80=99 ? Replaces are prohibited for _collation space. >> + end >> + end >> +end >> + >> +local function upgrade_to_2_1_2() >> + update_collation_strength_field() >> +end >> + >> local function get_version() >> local version =3D box.space._schema:get{'version'} >> if version =3D=3D nil then >> diff --git a/test/sql/collation.result b/test/sql/collation.result >> index 3794990dc..9994baca9 100644 >> --- a/test/sql/collation.result >> +++ b/test/sql/collation.result >> @@ -785,3 +785,17 @@ box.sql.execute("SELECT DISTINCT substr(s2, 1, = 1) FROM jj;") >> box.space.JJ:drop() >> --- >> ... >> +-- gh-3573: Strength in the _collation space >> +-- Collation without 'strength' option set now has explicit >> +-- 'strength' =3D 'tertiary'. >> +-- >> +box.internal.collation.create('c', 'ICU', 'unicode') >> +--- >> +... >> +id =3D box.internal.collation.id_by_name('c') >> +--- >> +... >> +box.space._collation:select(id) >=20 > 5. id_by_name + select can be replaced with one > box.space._collation.index.name:get({'c'}). Ok, done. >=20 >> +--- >> +- - [4, 'c', 1, 'ICU', 'unicode', {'strength': 'tertiary'}] >> +...