From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Nikita Pettik <korablev@tarantool.org>, tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH] sql: fix extra type calculation before bytecode generation
Date: Fri, 5 Apr 2019 12:51:20 +0300 [thread overview]
Message-ID: <66d88fd9-b2fc-3263-2bb4-962205a275d7@tarantool.org> (raw)
In-Reply-To: <20190405000627.58693-1-korablev@tarantool.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
>
next prev parent reply other threads:[~2019-04-05 9:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-04-05 0:06 [tarantool-patches] " Nikita Pettik
2019-04-05 5:49 ` [tarantool-patches] " Konstantin Osipov
2019-04-05 9:51 ` Vladislav Shpilevoy [this message]
2019-04-05 15:35 ` Kirill Yukhin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=66d88fd9-b2fc-3263-2bb4-962205a275d7@tarantool.org \
--to=v.shpilevoy@tarantool.org \
--cc=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='[tarantool-patches] Re: [PATCH] sql: fix extra type calculation before bytecode generation' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox