From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 74C512936C for ; Fri, 5 Apr 2019 05:51:23 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id r2X4m0V_cRdD for ; Fri, 5 Apr 2019 05:51:23 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 8CD9E2934F for ; Fri, 5 Apr 2019 05:51:22 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH] sql: fix extra type calculation before bytecode generation References: <20190405000627.58693-1-korablev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <66d88fd9-b2fc-3263-2bb4-962205a275d7@tarantool.org> Date: Fri, 5 Apr 2019 12:51:20 +0300 MIME-Version: 1.0 In-Reply-To: <20190405000627.58693-1-korablev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Nikita Pettik , tarantool-patches@freelists.org LGTM On 05/04/2019 03:06, Nikita Pettik wrote: > In SQL type of constant literal (e.g. 1, 2.5, 'abc') is assigned right > after parsing and saving into struct Expr. Occasionally, type is > re-assigned before emitting opcodes to store literal into VDBE memory. > What is more, for floating point number type is changed to "integer". > This patch fixes this obvious misbehaviour. > --- > Branch: https://github.com/tarantool/tarantool/tree/np/fix-float-type-meta > > src/box/sql/expr.c | 4 --- > test/sql/gh-3888-values-blob-assert.result | 2 +- > test/sql/misc.result | 43 ++++++++++++++++++++++++++++++ > test/sql/misc.test.lua | 9 +++++++ > 4 files changed, 53 insertions(+), 5 deletions(-) > > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c > index 4b98bd175..0b5fe788c 100644 > --- a/src/box/sql/expr.c > +++ b/src/box/sql/expr.c > @@ -3760,20 +3760,17 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > pExpr->op2); > } > case TK_INTEGER:{ > - pExpr->type = FIELD_TYPE_INTEGER; > expr_code_int(pParse, pExpr, false, target); > return target; > } > #ifndef SQL_OMIT_FLOATING_POINT > case TK_FLOAT:{ > - pExpr->type = FIELD_TYPE_INTEGER; > assert(!ExprHasProperty(pExpr, EP_IntValue)); > codeReal(v, pExpr->u.zToken, 0, target); > return target; > } > #endif > case TK_STRING:{ > - pExpr->type = FIELD_TYPE_STRING; > assert(!ExprHasProperty(pExpr, EP_IntValue)); > sqlVdbeLoadString(v, target, pExpr->u.zToken); > return target; > @@ -3791,7 +3788,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) > assert(pExpr->u.zToken[0] == 'x' > || pExpr->u.zToken[0] == 'X'); > assert(pExpr->u.zToken[1] == '\''); > - pExpr->type = FIELD_TYPE_SCALAR; > z = &pExpr->u.zToken[2]; > n = sqlStrlen30(z) - 1; > assert(z[n] == '\''); > diff --git a/test/sql/gh-3888-values-blob-assert.result b/test/sql/gh-3888-values-blob-assert.result > index 5275b0fc7..0d4a27c1f 100644 > --- a/test/sql/gh-3888-values-blob-assert.result > +++ b/test/sql/gh-3888-values-blob-assert.result > @@ -64,7 +64,7 @@ box.execute('SELECT 3.14') > --- > - metadata: > - name: '3.14' > - type: integer > + type: number > rows: > - [3.14] > ... > diff --git a/test/sql/misc.result b/test/sql/misc.result > index 6454015f5..b5bb1fb4c 100644 > --- a/test/sql/misc.result > +++ b/test/sql/misc.result > @@ -63,3 +63,46 @@ box.execute('CREATE TABLE test (id INTEGER PRIMARY KEY, b TEXT CONSTRAINT c1 COL > - error: Keyword 'COLLATE' is reserved. Please use double quotes if 'COLLATE' is an > identifier. > ... > +-- Make sure that type of literals in meta complies with its real > +-- type. For instance, typeof(0.5) is number, not integer. > +-- > +box.execute('SELECT 1;') > +--- > +- metadata: > + - name: '1' > + type: integer > + rows: > + - [1] > +... > +box.execute('SELECT 1.5;') > +--- > +- metadata: > + - name: '1.5' > + type: number > + rows: > + - [1.5] > +... > +box.execute('SELECT 1.0;') > +--- > +- metadata: > + - name: '1.0' > + type: number > + rows: > + - [1] > +... > +box.execute('SELECT \'abc\';') > +--- > +- metadata: > + - name: '''abc''' > + type: string > + rows: > + - ['abc'] > +... > +box.execute('SELECT X\'4D6564766564\'') > +--- > +- metadata: > + - name: X'4D6564766564' > + type: scalar > + rows: > + - ['Medved'] > +... > diff --git a/test/sql/misc.test.lua b/test/sql/misc.test.lua > index 79ca8da21..0b1c34d1b 100644 > --- a/test/sql/misc.test.lua > +++ b/test/sql/misc.test.lua > @@ -17,3 +17,12 @@ box.execute('\n\n\n\t\t\t ') > box.execute('CREATE TABLE test (id INTEGER PRIMARY KEY, b INTEGER CONSTRAINT c1 NULL)') > box.execute('CREATE TABLE test (id INTEGER PRIMARY KEY, b INTEGER CONSTRAINT c1 DEFAULT 300)') > box.execute('CREATE TABLE test (id INTEGER PRIMARY KEY, b TEXT CONSTRAINT c1 COLLATE "binary")') > + > +-- Make sure that type of literals in meta complies with its real > +-- type. For instance, typeof(0.5) is number, not integer. > +-- > +box.execute('SELECT 1;') > +box.execute('SELECT 1.5;') > +box.execute('SELECT 1.0;') > +box.execute('SELECT \'abc\';') > +box.execute('SELECT X\'4D6564766564\'') > -- > 2.15.1 >