From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (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 EF2A84696C3 for ; Fri, 10 Apr 2020 21:09:13 +0300 (MSK) Date: Fri, 10 Apr 2020 21:09:11 +0300 From: Mergen Imeev Message-ID: <20200410180911.GA28782@tarantool.org> References: <20200327165417.GD9287@tarantool.org> <20200410104122.GB20019@tarantool.org> <20200410125757.GB9428@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20200410125757.GB9428@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v4 2/3] sql: fix implicit cast from STRING to INTEGER List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org Hi! Thank you for the review. I extended commit-message. You can see it below. On Fri, Apr 10, 2020 at 12:57:57PM +0000, Nikita Pettik wrote: > On 10 Apr 13:41, Mergen Imeev wrote: > > On Fri, Mar 27, 2020 at 04:54:17PM +0000, Nikita Pettik wrote: > > > On 27 Mar 14:33, imeevma@tarantool.org wrote: > > > > Prior to this patch, STRING, which contains the DOUBLE value, > > > > could be implicitly cast to INTEGER. This was done by converting > > > > STRING to DOUBLE and then converting this DOUBLE value to INTEGER. > > > > This may affect the accuracy of CAST(), so it was forbidden. > > > > > > > > Example: > > > > box.execute("CREATE TABLE t(i INT PRIMARY KEY);") > > > > > > > > Before patch: > > > > box.execute("INSERT INTO t VALUES ('111.1');") > > > > box.execute("SELECT * FROM t;") > > > > Result: 111 > > > > > > > > After patch: > > > > box.execute("INSERT INTO t VALUES ('1.1');") > > > > Result: 'Type mismatch: can not convert 1.1 to integer' > > > > > > > > box.execute("INSERT INTO t VALUES ('1.0');") > > > > Result: 'Type mismatch: can not convert 1.0 to integer' > > > > > > > > box.execute("INSERT INTO t VALUES ('1.');") > > > > Result: 'Type mismatch: can not convert 1. to integer' > > > > > > Is comparison predicat affected? > > > > > No, since all implicit casts in case of comparison executed > > inside of comparisons opcodes. > > You should have mentioned this fact in commit message. > Fixed: sql: fix implicit cast from STRING to INTEGER Prior to this patch, STRING, which contains the DOUBLE value, could be implicitly cast to INTEGER. This was done by converting STRING to DOUBLE and then converting this DOUBLE value to INTEGER. This may affect the accuracy of CAST(), so it was forbidden. It is worth noting that these changes will not affect the comparison, since the implicit cast in this case has different mechanics. Example: box.execute("CREATE TABLE t(i INT PRIMARY KEY);") Before patch: box.execute("INSERT INTO t VALUES ('111.1');") box.execute("SELECT * FROM t;") Result: 111 After patch: box.execute("INSERT INTO t VALUES ('1.1');") Result: 'Type mismatch: can not convert 1.1 to integer' box.execute("INSERT INTO t VALUES ('1.0');") Result: 'Type mismatch: can not convert 1.0 to integer' box.execute("INSERT INTO t VALUES ('1.');") Result: 'Type mismatch: can not convert 1. to integer' @TarantoolBot document Title: disallow cast from STRING contains DOUBLE to INTEGER After the last two patches, explicit and implicit casting from the string containing DOUBLE to INTEGER directly will be prohibited. The user must use the explicit cast to DOUBLE before the explicit or implicit cast to INTEGER. The reason for this is that before these patches, such STRINGs were implicitly cast to DOUBLE, and then this DOUBLE was implicitly or explicitly cast to INTEGER. Because of this, the result of such a cast may differ from what the user expects, and the user may not know why. It is worth noting that these changes will not affect the comparison, since the implicit cast in this case has different mechanics. Example for implicit cast: box.execute("CREATE TABLE t(i INT PRIMARY KEY);") -- Does not work anymore: box.execute("INSERT INTO t VALUES ('1.1');") -- Right way: box.execute("INSERT INTO t VALUES (CAST('1.1' AS DOUBLE));") Example for explicit cast: -- Does not work anymore: box.execute("SELECT CAST('1.1' AS INTEGER);") -- Right way: box.execute("SELECT CAST(CAST('1.1' AS DOUBLE) AS INTEGER);") > LGTM otherwise. >