Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: fix extra type calculation before bytecode generation
@ 2019-04-05  0:06 Nikita Pettik
  2019-04-05  5:49 ` [tarantool-patches] " Konstantin Osipov
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Nikita Pettik @ 2019-04-05  0:06 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: fix extra type calculation before bytecode generation
  2019-04-05  0:06 [tarantool-patches] [PATCH] sql: fix extra type calculation before bytecode generation Nikita Pettik
@ 2019-04-05  5:49 ` Konstantin Osipov
  2019-04-05  9:51 ` Vladislav Shpilevoy
  2019-04-05 15:35 ` Kirill Yukhin
  2 siblings, 0 replies; 4+ messages in thread
From: Konstantin Osipov @ 2019-04-05  5:49 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

* Nikita Pettik <korablev@tarantool.org> [19/04/05 06:36]:
> 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.

LGTM (for 2.1)


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: fix extra type calculation before bytecode generation
  2019-04-05  0:06 [tarantool-patches] [PATCH] sql: fix extra type calculation before bytecode generation Nikita Pettik
  2019-04-05  5:49 ` [tarantool-patches] " Konstantin Osipov
@ 2019-04-05  9:51 ` Vladislav Shpilevoy
  2019-04-05 15:35 ` Kirill Yukhin
  2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy @ 2019-04-05  9:51 UTC (permalink / raw)
  To: Nikita Pettik, tarantool-patches

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
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] Re: [PATCH] sql: fix extra type calculation before bytecode generation
  2019-04-05  0:06 [tarantool-patches] [PATCH] sql: fix extra type calculation before bytecode generation Nikita Pettik
  2019-04-05  5:49 ` [tarantool-patches] " Konstantin Osipov
  2019-04-05  9:51 ` Vladislav Shpilevoy
@ 2019-04-05 15:35 ` Kirill Yukhin
  2 siblings, 0 replies; 4+ messages in thread
From: Kirill Yukhin @ 2019-04-05 15:35 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

Hello,

On 05 Apr 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

I've checked the patch into master and 2.1 branch.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-04-05 15:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-05  0:06 [tarantool-patches] [PATCH] sql: fix extra type calculation before bytecode generation Nikita Pettik
2019-04-05  5:49 ` [tarantool-patches] " Konstantin Osipov
2019-04-05  9:51 ` Vladislav Shpilevoy
2019-04-05 15:35 ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox