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 534952A206 for ; Wed, 27 Mar 2019 07:59: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 8bBTbVxFNK40 for ; Wed, 27 Mar 2019 07:59:49 -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 02B152A204 for ; Wed, 27 Mar 2019 07:59:49 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH] sql: set explicit default collation's strength References: <20190326203456.76617-1-ivan.koptelov@tarantool.org> From: Vladislav Shpilevoy Message-ID: <8ab84a8f-213e-4bcd-217d-e5f694b70693@tarantool.org> Date: Wed, 27 Mar 2019 14:59:47 +0300 MIME-Version: 1.0 In-Reply-To: <20190326203456.76617-1-ivan.koptelov@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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, Ivan Koptelov Hi! Thanks for the patch! See 5 comments below. 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='tertiary'}] ... > > 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 => 2.1.2 box/stat should be enabled again. 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. What is more, I enabled that test back, and it passes. So what a problem? > > Closes #3573 > > @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: 2. 'It was explicit', 'it's just become explicit'. A guess, the first one was 'implicit'. > [1, 'unicode', 1, 'ICU', '', {}] > After the patch it's just become explicit: > 1, 'unicode', 1, 'ICU', '', {'strength' = 'tertiary'}] > > 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 > > 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(-) > > 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 = true}, {{1, 'string'}}} > > log.info("create predefined collations") > - box.space._collation:replace{1, "unicode", ADMIN, "ICU", "", setmap{}} > + box.space._collation:replace{1, "unicode", ADMIN, "ICU", "", {strength='tertiary'}} 3. Please, do not touch old upgrade scripts. They are 'read-only'. > box.space._collation:replace{2, "unicode_ci", ADMIN, "ICU", "", {strength='primary'}} > > local _priv = box.space[box.schema.PRIV_ID] > @@ -632,6 +632,27 @@ local function upgrade_to_2_1_1() > end > end > > +-------------------------------------------------------------------------------- > +-- Tarantool 2.1.2 > +-------------------------------------------------------------------------------- > + > +local 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 and collation.name ~= 'none' and > + collation.name ~= 'binary' then > + local new_collation = _collation:get{collation.id}:totable() > + new_collation[6].strength = 'tertiary' > + _collation:delete{collation.id} > + _collation:insert(new_collation) 4. 'replace' ? > + end > + end > +end > + > +local function upgrade_to_2_1_2() > + update_collation_strength_field() > +end > + > local function get_version() > local version = box.space._schema:get{'version'} > if version == 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' = 'tertiary'. > +-- > +box.internal.collation.create('c', 'ICU', 'unicode') > +--- > +... > +id = box.internal.collation.id_by_name('c') > +--- > +... > +box.space._collation:select(id) 5. id_by_name + select can be replaced with one box.space._collation.index.name:get({'c'}). > +--- > +- - [4, 'c', 1, 'ICU', 'unicode', {'strength': 'tertiary'}] > +...