From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41]) (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 92E8C4696C3 for ; Fri, 27 Mar 2020 19:46:05 +0300 (MSK) Date: Fri, 27 Mar 2020 16:46:04 +0000 From: Nikita Pettik Message-ID: <20200327164604.GC9287@tarantool.org> References: <01024a2279b5c030f272d2a97ce8bd83f8ababc3.1585308644.git.imeevma@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <01024a2279b5c030f272d2a97ce8bd83f8ababc3.1585308644.git.imeevma@gmail.com> Subject: Re: [Tarantool-patches] [PATCH v4 1/3] sql: fix CAST() from STRING to INTEGER 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 27 Mar 14:33, imeevma@tarantool.org wrote: Could you please find Peter's table containing current/expected cast behaviours and verify that this patch doesn't contradict it? > diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c > index aad030d..de1d9c3 100644 > --- a/src/box/sql/vdbemem.c > +++ b/src/box/sql/vdbemem.c > @@ -696,7 +696,7 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type) > return -1; > case FIELD_TYPE_INTEGER: > case FIELD_TYPE_UNSIGNED: > - if ((pMem->flags & MEM_Blob) != 0) { > + if ((pMem->flags & (MEM_Blob | MEM_Str)) != 0) { > bool is_neg; > int64_t val; > if (sql_atoi64(pMem->z, &val, &is_neg, pMem->n) != 0) > @@ -711,8 +711,20 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type) > MemSetTypeFlag(pMem, MEM_UInt); > return 0; > } > - if (sqlVdbeMemIntegerify(pMem) != 0) > + if ((pMem->flags & MEM_Real) != 0) { > + double d; > + if (sqlVdbeRealValue(pMem, &d) != 0) > + return -1; > + if (d < INT64_MAX && d >= INT64_MIN) { > + mem_set_int(pMem, d, d <= -1); > + return 0; > + } > + if (d >= INT64_MAX && d < UINT64_MAX) { > + mem_set_u64(pMem, d); > + return 0; > + } > return -1; Instead of keeping inlining code into sqlVdbeMemCast() I'd better split it into separate functions. Good refactoring task tho - just keep in mind. > + } > if (type == FIELD_TYPE_UNSIGNED && > (pMem->flags & MEM_UInt) == 0) > return -1; > diff --git a/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua b/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua > new file mode 100755 > index 0000000..0865c4e > --- /dev/null > +++ b/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua Strictly speaking this change is not related to 4766, so I'd not create separate test file for current patch. Moreover, tests in sql/types.test.lua seem to cover it. > +#!/usr/bin/env tarantool > +test = require("sqltester") > +test:plan(6) > + > +-- > +-- Make sure that STRING or BLOB that contains DOUBLE value cannot > +-- be cast to INTEGER. > +-- > +test:do_catchsql_test( > + "gh-4766-1", > + [[ > + SELECT CAST('1.1' AS INTEGER) > + ]], { > + 1, "Type mismatch: can not convert 1.1 to integer" > + }) > + > +test:do_catchsql_test( > + "gh-4766-2", > + [[ > + SELECT CAST(x'312e31' AS INTEGER) > + ]], { > + 1, "Type mismatch: can not convert varbinary to integer" > + }) > + > diff --git a/test/sql/types.result b/test/sql/types.result > index 38e4385..54aff46 100644 > --- a/test/sql/types.result > +++ b/test/sql/types.result > @@ -269,11 +269,8 @@ box.space.T1:drop() > -- > box.execute("SELECT CAST('1.123' AS INTEGER);") > --- > -- metadata: > - - name: CAST('1.123' AS INTEGER) > - type: integer > - rows: > - - [1] > +- null > +- 'Type mismatch: can not convert 1.123 to integer' > ... > box.execute("CREATE TABLE t1 (f TEXT PRIMARY KEY);") > --- > @@ -285,13 +282,8 @@ box.execute("INSERT INTO t1 VALUES('0.0'), ('1.5'), ('3.9312453');") > ... > box.execute("SELECT CAST(f AS INTEGER) FROM t1;") > --- > -- metadata: > - - name: CAST(f AS INTEGER) > - type: integer > - rows: > - - [0] > - - [1] > - - [3] > +- null > +- 'Type mismatch: can not convert 0.0 to integer' > ... > box.space.T1:drop() > --- > @@ -1105,8 +1097,11 @@ box.execute("SELECT CAST(1.5 AS UNSIGNED);") > ... > box.execute("SELECT CAST(-1.5 AS UNSIGNED);") > --- > -- null > -- 'Type mismatch: can not convert -1 to unsigned' > +- metadata: > + - name: CAST(-1.5 AS UNSIGNED) > + type: unsigned > + rows: > + - [-1] > ... > box.execute("SELECT CAST(true AS UNSIGNED);") > --- > -- > 2.7.4 >