From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 41CC6469710 for ; Mon, 18 May 2020 13:26:51 +0300 (MSK) Date: Mon, 18 May 2020 13:26:49 +0300 From: Mergen Imeev Message-ID: <20200518102648.GA17007@tarantool.org> References: <20200417110925.31741-1-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200417110925.31741-1-roman.habibov@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] sql: display collation in metadata for scalar List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org Hi! Thank you for the fix. I have only one comment here, you can see it below. Please send next version to Nikita. On Fri, Apr 17, 2020 at 02:09:25PM +0300, Roman Khabibov wrote: > Fix bug with the display of collation for scalar fields in > > when sql_full_metadata is enabled. > > src/box/sql/select.c | 7 ++- > .../gh-4755-scalar-collation-metadata.result | 56 +++++++++++++++++++ > ...gh-4755-scalar-collation-metadata.test.lua | 22 ++++++++ > 3 files changed, 82 insertions(+), 3 deletions(-) > create mode 100644 test/sql/gh-4755-scalar-collation-metadata.result > create mode 100755 test/sql/gh-4755-scalar-collation-metadata.test.lua > > diff --git a/src/box/sql/select.c b/src/box/sql/select.c > index f39484013..92d20e992 100644 > --- a/src/box/sql/select.c > +++ b/src/box/sql/select.c > @@ -1792,9 +1792,10 @@ generate_column_metadata(struct Parse *pParse, struct SrcList *pTabList, > continue; > if (p->op == TK_VARIABLE) > var_pos[var_count++] = i; > - vdbe_metadata_set_col_type(v, i, > - field_type_strs[sql_expr_type(p)]); > - if (is_full_meta && sql_expr_type(p) == FIELD_TYPE_STRING) { > + enum field_type type = sql_expr_type(p); > + vdbe_metadata_set_col_type(v, i, field_type_strs[type]); > + if (is_full_meta && (type == FIELD_TYPE_STRING || > + type == FIELD_TYPE_SCALAR)) { > bool unused; > uint32_t id = 0; > struct coll *coll = NULL; > diff --git a/test/sql/gh-4755-scalar-collation-metadata.result b/test/sql/gh-4755-scalar-collation-metadata.result > new file mode 100644 > index 000000000..2ffcc79f3 > --- /dev/null > +++ b/test/sql/gh-4755-scalar-collation-metadata.result > @@ -0,0 +1,56 @@ > +-- test-run result file version 2 > +env = require('test_run') > + | --- > + | ... > +test_run = env.new() > + | --- > + | ... > + > +-- > +-- gh-4755: Collation in metadata must be displayed as for string > +-- filed as for scalar field. > +-- > +test_run:cmd("setopt delimiter ';'"); > + | --- > + | - true > + | ... > +box.execute([[UPDATE "_session_settings" > + SET "value" = true > + WHERE "name" = 'sql_full_metadata';]]); > + | --- > + | - row_count: 1 > + | ... > +box.execute([[CREATE TABLE test (a SCALAR COLLATE "unicode_ci" PRIMARY KEY, > + b STRING COLLATE "unicode_ci");]]); > + | --- > + | - row_count: 1 > + | ... > +box.execute("SELECT * FROM test;"); > + | --- > + | - metadata: > + | - span: A > + | type: scalar > + | is_nullable: false > + | name: A > + | collation: unicode_ci > + | - span: B > + | type: string > + | is_nullable: true > + | name: B > + | collation: unicode_ci > + | rows: [] > + | ... > + > +-- > +-- Cleanup. > +-- > +box.execute([[UPDATE "_session_settings" > + SET "value" = false > + WHERE "name" = 'sql_full_metadata';]]); > + | --- > + | - row_count: 1 > + | ... > +box.execute("DROP TABLE test;"); > + | --- > + | - row_count: 1 > + | ... > diff --git a/test/sql/gh-4755-scalar-collation-metadata.test.lua b/test/sql/gh-4755-scalar-collation-metadata.test.lua > new file mode 100755 > index 000000000..197b06891 > --- /dev/null > +++ b/test/sql/gh-4755-scalar-collation-metadata.test.lua > @@ -0,0 +1,22 @@ > +env = require('test_run') > +test_run = env.new() > + > +-- > +-- gh-4755: Collation in metadata must be displayed as for string > +-- filed as for scalar field. > +-- > +test_run:cmd("setopt delimiter ';'"); > +box.execute([[UPDATE "_session_settings" > + SET "value" = true > + WHERE "name" = 'sql_full_metadata';]]); I think it make sense to use new interface here. > +box.execute([[CREATE TABLE test (a SCALAR COLLATE "unicode_ci" PRIMARY KEY, > + b STRING COLLATE "unicode_ci");]]); > +box.execute("SELECT * FROM test;"); > + > +-- > +-- Cleanup. > +-- > +box.execute([[UPDATE "_session_settings" > + SET "value" = false > + WHERE "name" = 'sql_full_metadata';]]); > +box.execute("DROP TABLE test;"); > -- > 2.21.0 (Apple Git-122) >