From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 059636EC55; Fri, 30 Jul 2021 10:14:45 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 059636EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1627629285; bh=IOGOqsI2WCTalZpEaoT8lvsh1RXbMZfebrFICO+Ue5o=; h=To:Cc:References:In-Reply-To:Date:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=Zv2li0iekgIyvx7AwxZEQiPQmdwUizkmfM/G8bP0Sv0LE66dhEoAlIEspqegVSWyW RSTtW7ntggL6XGk2J0eyiNNX9nHL0SJXV1G0HD0asy/UURQH6u7cjWrjHjcE9/IAek ccrEVpdc0Vjbt3IRoB5riWduedM2xBYI4Lt8V+H0= Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (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 A8F316EC55 for ; Fri, 30 Jul 2021 10:14:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A8F316EC55 Received: by smtp29.i.mail.ru with esmtpa (envelope-from ) id 1m9MjC-0003SL-44; Fri, 30 Jul 2021 10:14:42 +0300 To: , "Kirill Yukhin" Cc: References: In-Reply-To: Date: Fri, 30 Jul 2021 10:14:36 +0300 Message-ID: <005f01d78512$8defd690$a9cf83b0$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQMAKZt1jrnIdNdNlSci0cjxkip/6qkJz1dw Content-Language: ru X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD941C43E597735A9C351B198F4576AC7B20CA14D9DFB46B94A182A05F53808504064D4A4D04EF8994BCC921568DEE1F6C2C945AC7676EC8F6561C4BD8434935A39 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE704BA85F3D5A9F85BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006377A06FA5CCF78F9BC8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8699D5EC44635E6E0750CB14DFF0CCB86117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735209ECD01F8117BC8BEA471835C12D1D977C4224003CC836476EB9C4185024447017B076A6E789B0E975F5C1EE8F4F765FCFCB87305045D83103AA81AA40904B5D9CF19DD082D7633A078D18283394535A93AA81AA40904B5D98AA50765F79006372763B6983292DCA9D81D268191BDAD3D698AB9A7B718F8C4D1B931868CE1C5781A620F70A64A45A98AA50765F79006372E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F83C798A30B85E16BA91E23F1B6B78B78B5C8C57E37DE458BEDA766A37F9254B7 X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A213B5FB47DCBC3458834459D11680B505AEBCF5B81AC7EAE47CCABEAA5A141FD5 X-C1DE0DAB: 0D63561A33F958A5A74523E495509B0854C88202B4ECE786B65DBAA05816C37DD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7517A45118377F5F9E8E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34315359784EE451039A79B982867100BAA78EB1D8B08D66549B21FEB699186AB347023DC60A550A7C1D7E09C32AA3244CA3FD77125EB8BAF1DFBDB3E703B7B6037101BF96129E4011FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojWBddABnKmoI7kv9rxmDIhA== X-Mailru-Sender: B5B6A6EBBD94DAD8DF9DB9650A8885C8C1C1334795945499886D60FF4C137A20F32926FFFE6152661EC9E4A2C82A33BC8C24925A86E657CE0C70AEE3C9A96FBAB3D7EE8ED63280BE112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: remove OP_Realify X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Timur Safin via Tarantool-patches Reply-To: Timur Safin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sorry. But this is part of a changes, which we have not agreed upon = (reminder, that the=20 last agreement with PMs was to get rid of scalar, but keep number = arithmetic), and which=20 [I believe] we would have to revert eventually. Please ask Kirill for = ack, if he believes that we have to do it for "consistent types" task. Let me clarify how I see this - in this particular case forcing real = type semantics for=20 NUMBER field was pretty much consistent behavior. And using integer = division on some=20 rows, but double on others is actually making things _much less_ = consistent.=20 Regards, : From: imeevma@tarantool.org : Subject: [PATCH v1 1/1] sql: remove OP_Realify :=20 : This opcode was used to convert INTEGER values to REAL. It is not : necessary in Tarantool and causes errors. :=20 : Due to OP_Realify two type of errors appeared: : 1) In some cases in trigger INTEGER may be converted to DOUBLE. : For example: : box.execute("CREATE TABLE t (i NUMBER PRIMARY KEY, n NUMBER);") : box.execute("CREATE TRIGGER t AFTER INSERT ON t FOR EACH ROW BEGIN = UPDATE t : SET n =3D new.n; END;") : box.execute("INSERT INTO t VALUES (1, 1);") : box.execute("SELECT i / 2, n / 2 FROM t;") :=20 : Result: : tarantool> box.execute("SELECT i / 2, n / 2 FROM t;") : --- : - metadata: : - name: COLUMN_1 : type: number : - name: COLUMN_2 : type: number : rows: : - [0, 0.5] : ... :=20 : 2) If SELECT uses GROUP BY then it may return DOUBLE instead of = INTEGER. : For example: : box.execute("CREATE TABLE t (i NUMBER PRIMARY KEY, n NUMBER);") : box.execute("INSERT INTO t VALUES (1,1);") : box.execute("SELECT i / 2, n / 2 FROM t GROUP BY n;") :=20 : Result: : tarantool> box.execute("SELECT i / 2, n / 2 FROM t GROUP BY n;") : --- : - metadata: : - name: COLUMN_1 : type: number : - name: COLUMN_2 : type: number : rows: : - [0.5, 0.5] : ... :=20 : This patch removes OP_Realify, after which these errors disappear. :=20 : Closes #5335 : --- : https://github.com/tarantool/tarantool/issues/5335 : https://github.com/tarantool/tarantool/tree/imeevma/gh-5335-remove-op- : realify :=20 : ...gh-5335-remove-wrong-double-to-int-cast.md | 4 ++ : src/box/sql/expr.c | 13 ------- : src/box/sql/vdbe.c | 17 -------- : .../gh-5335-wrong-int-to-double-cast.test.lua | 39 = +++++++++++++++++++ : 4 files changed, 43 insertions(+), 30 deletions(-) : create mode 100644 = changelogs/unreleased/gh-5335-remove-wrong-double-to- : int-cast.md : create mode 100755 = test/sql-tap/gh-5335-wrong-int-to-double-cast.test.lua :=20 : diff --git a/changelogs/unreleased/gh-5335-remove-wrong-double-to-int- : cast.md = b/changelogs/unreleased/gh-5335-remove-wrong-double-to-int-cast.md : new file mode 100644 : index 000000000..b06805a7f : --- /dev/null : +++ b/changelogs/unreleased/gh-5335-remove-wrong-double-to-int-cast.md : @@ -0,0 +1,4 @@ : +## bugfix/sql : + : +* Removed spontaneous conversion from INTEGER to DOUBLE in a field of = type : + NUMBER (gh-5335). : diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c : index 3772596d6..d2624516c 100644 : --- a/src/box/sql/expr.c : +++ b/src/box/sql/expr.c : @@ -3700,11 +3700,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, = int : target) : sqlVdbeAddOp3(v, OP_Column, : pAggInfo->sortingIdxPTab, : pCol->iSorterColumn, target); : - if (pCol->space_def->fields[pExpr->iAgg].type =3D=3D : - FIELD_TYPE_NUMBER) { : - sqlVdbeAddOp1(v, OP_Realify, : - target); : - } : return target; : } : /* : @@ -4260,14 +4255,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, = int : target) : (pExpr->iTable ? "new" : "old"), : pExpr->space_def->fields[ : pExpr->iColumn].name, target)); : - : - /* If the column has type NUMBER, it may currently be : stored as an : - * integer. Use OP_Realify to make sure it is really real. : - */ : - if (pExpr->iColumn >=3D 0 && def->fields[ : - pExpr->iColumn].type =3D=3D FIELD_TYPE_NUMBER) { : - sqlVdbeAddOp1(v, OP_Realify, target); : - } : break; : } :=20 : diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c : index 9e763ed85..0a3c904b1 100644 : --- a/src/box/sql/vdbe.c : +++ b/src/box/sql/vdbe.c : @@ -1445,23 +1445,6 @@ case OP_MustBeInt: { /* jump, in1 */ : break; : } :=20 : -/* Opcode: Realify P1 * * * * : - * : - * If register P1 holds an integer convert it to a real value. : - * : - * This opcode is used when extracting information from a column that : - * has float type. Such column values may still be stored as : - * integers, for space efficiency, but after extraction we want them : - * to have only a real value. : - */ : -case OP_Realify: { /* in1 */ : - pIn1 =3D &aMem[pOp->p1]; : - if (mem_is_int(pIn1)) { : - mem_to_double(pIn1); : - } : - break; : -} : - : /* Opcode: Cast P1 P2 * * * : * Synopsis: type(r[P1]) : * : diff --git a/test/sql-tap/gh-5335-wrong-int-to-double-cast.test.lua : b/test/sql-tap/gh-5335-wrong-int-to-double-cast.test.lua : new file mode 100755 : index 000000000..d29324a28 : --- /dev/null : +++ b/test/sql-tap/gh-5335-wrong-int-to-double-cast.test.lua : @@ -0,0 +1,39 @@ : +#!/usr/bin/env tarantool : +local test =3D require("sqltester") : +test:plan(2) : + : +test:execsql([[ : + CREATE TABLE t1 (i NUMBER PRIMARY KEY, n NUMBER); : + CREATE TABLE t2 (i NUMBER PRIMARY KEY, n NUMBER); : + CREATE TRIGGER r AFTER INSERT ON t1 FOR EACH ROW BEGIN UPDATE t1 = SET n : =3D new.n; END; : + INSERT INTO t1 VALUES (1, 1); : + INSERT INTO t2 VALUES (1, 1); : +]]) : + : +-- : +-- Make sure that there are no unnecesary INTEGER to DOUBLE implicit = cast : in : +-- field of type NUMBER. : +-- : +test:do_execsql_test( : + "gh-5335-1", : + [[ : + SELECT i / 2, n / 2 FROM t1; : + ]], { : + 0, 0 : + }) : + : +test:do_execsql_test( : + "gh-5335-2", : + [[ : + SELECT i / 2, n / 2 FROM t2 GROUP BY n; : + ]], { : + 0, 0 : + }) : + : +test:execsql([[ : + DROP TRIGGER r; : + DROP TABLE t1; : + DROP TABLE t2; : +]]) : + : +test:finish_test() : -- : 2.25.1