From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp60.i.mail.ru (smtp60.i.mail.ru [217.69.128.40]) (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 2009B469719 for ; Fri, 18 Sep 2020 15:28:11 +0300 (MSK) Date: Fri, 18 Sep 2020 12:28:10 +0000 From: Nikita Pettik Message-ID: <20200918122809.GJ10599@tarantool.org> References: <2446ef040a7f9d235cfc14bccf4dcaa7b6254e73.1598000242.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2446ef040a7f9d235cfc14bccf4dcaa7b6254e73.1598000242.git.imeevma@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v5 6/6] sql: remove implicit cast from MakeRecord opcode List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: imeevma@tarantool.org Cc: tarantool-patches@dev.tarantool.org On 21 Aug 12:19, imeevma@tarantool.org wrote: > This patch removes implicit casting from MakeRecord opcode. > > Closes #4230 > > @TarantoolBot document > Title: remove implicit cast for comparison > > After this patch-set, there will be no implicit casts for comparison. > This means that the values ​of the field types STRING, BOOLEAN and Again these strange characters in commit msg.. > VARBINARY can be compared with the values of the same field type. Only. > Any numerical value can be compared with any other numerical value. > > Example: > > ``` > tarantool> box.execute([[SELECT '1' > 0;]]) > --- > - null > - 'Type mismatch: can not convert 1 to numeric' > ... > > tarantool> box.execute([[SELECT true > X'33';]]) > --- > - null > - 'Type mismatch: can not convert boolean to varbinary' > ... > > tarantool> box.execute([[SELECT 1.23 > 123;]]) > --- > - metadata: > - name: 1.23 > 123 > type: boolean > rows: > - [false] > ... > ``` > --- > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index bc2182446..d553b80db 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -2886,11 +2886,18 @@ sqlCodeSubselect(Parse * pParse, /* Parsing context */ > jmpIfDynamic = -1; > } > r3 = sqlExprCodeTarget(pParse, pE2, r1); > - enum field_type types[2] = > - { lhs_type, field_type_MAX }; > - sqlVdbeAddOp4(v, OP_MakeRecord, r3, > - 1, r2, (char *)types, > - sizeof(types)); > + uint32_t size = > + 2 * sizeof(enum field_type); > + enum field_type *types= > + sqlDbMallocZero(pParse->db, Why do you have manually alloc types? It is done in sqlVdbeAddOp4() if pass n > 0 (like it was before your refactoring). > + size); > + types[0] = lhs_type; > + types[1] = field_type_MAX; > + sqlVdbeAddOp4(v, OP_ApplyType, r3, 1, 0, > + (char *)types, P4_DYNAMIC); > + sqlVdbeChangeP5(v, OPFLAG_DO_NOT_CONVERT_NUMBERS); > + sqlVdbeAddOp3(v, OP_MakeRecord, r3, 1, > + r2); > sql_expr_type_cache_change(pParse, > r3, 1); > sqlVdbeAddOp2(v, OP_IdxInsert, r2, You also can remove now mem_apply_type(). > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index a0ddbaf60..544619b03 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -3145,24 +3145,17 @@ type_mismatch: > break; > } > > diff --git a/test/sql-tap/in3.test.lua b/test/sql-tap/in3.test.lua > index a6d842962..7f3abbae0 100755 > --- a/test/sql-tap/in3.test.lua > +++ b/test/sql-tap/in3.test.lua > @@ -1,6 +1,6 @@ > #!/usr/bin/env tarantool > test = require("sqltester") > -test:plan(28) > +test:plan(26) > > --!./tcltestrunner.lua > -- 2007 November 29 > @@ -334,18 +334,6 @@ test:do_test( > -- > }) > > -test:do_test( > - "in3-3.5", > - function() > - -- Numeric affinity should be applied to each side before the comparison > - -- takes place. Therefore we cannot use index t1_i1, which has no affinity. > - return exec_neph(" SELECT y IN (SELECT a FROM t1) FROM t2 ") > - end, { > - -- > - 1, true > - -- > - }) > - > test:do_test( > "in3-3.6", > function() > @@ -358,18 +346,6 @@ test:do_test( > -- > }) > > -test:do_test( > - "in3-3.7", > - function() > - -- Numeric affinity is applied before the comparison takes place. > - -- Making it impossible to use index t1_i3. > - return exec_neph(" SELECT y IN (SELECT c FROM t1) FROM t2 ") > - end, { > - -- > - 1, true > - -- > - }) > - Why did you remove these tests? > ----------------------------------------------------------------------- > -- > -- Test using a multi-column index.