Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: v.shpilevoy@tarantool.org, kostja@tarantool.org,
	Nikita Pettik <korablev@tarantool.org>
Subject: [tarantool-patches] [PATCH 5/9] sql: improve type determination for column meta
Date: Sun, 14 Apr 2019 18:04:03 +0300	[thread overview]
Message-ID: <c19cef87e6d8e54a527600a0c424346c15c12ce8.1555252410.git.korablev@tarantool.org> (raw)
In-Reply-To: <cover.1555252410.git.korablev@tarantool.org>
In-Reply-To: <cover.1555252410.git.korablev@tarantool.org>

In most cases we don't assign and store type of node of expression AST
(except for constant literals).  To determine type of node  we use
sql_expr_type() function, which implements logic of quite simple
recursive tree traversal. Before this patch we set type of node after
code generation in sqlExprCodeTarget() without any traversal. This
approach is way worse even then sql_expr_type(). So, to improve accuracy
of type determination, let's always call that method and remove type
assignment in sqlExprCodeTarget().
---
 src/box/sql/expr.c                         | 19 ++++---------------
 src/box/sql/select.c                       |  2 +-
 test/sql/check-clear-ephemeral.result      |  2 +-
 test/sql/gh-2347-max-int-literals.result   |  2 +-
 test/sql/gh-3199-no-mem-leaks.result       | 18 +++++++++---------
 test/sql/gh-3888-values-blob-assert.result |  2 +-
 test/sql/persistency.result                |  8 ++++----
 test/sql/transition.result                 |  4 ++--
 test/sql/types.result                      | 10 +++++-----
 9 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 6b38e8e66..09ebdba66 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -105,6 +105,10 @@ sql_expr_type(struct Expr *pExpr)
 	case TK_NOT:
 	case TK_AND:
 	case TK_OR:
+	case TK_ISNULL:
+	case TK_NOTNULL:
+	case TK_BETWEEN:
+	case TK_IN:
 		/*
 		 * FIXME: should be changed to BOOL type
 		 * when it is implemented. Now simply
@@ -3769,7 +3773,6 @@ 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;
 		}
@@ -3780,14 +3783,12 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int 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;
@@ -3805,7 +3806,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] == '\'');
@@ -3887,7 +3887,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 				testcase(regFree1 == 0);
 				testcase(regFree2 == 0);
 			}
-			pExpr->type = FIELD_TYPE_INTEGER;
 			break;
 		}
 	case TK_AND:
@@ -3931,10 +3930,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			sqlVdbeAddOp3(v, op, r2, r1, target);
 			testcase(regFree1 == 0);
 			testcase(regFree2 == 0);
-			if (op != TK_CONCAT)
-				pExpr->type = FIELD_TYPE_NUMBER;
-			else
-				pExpr->type = FIELD_TYPE_STRING;
 			break;
 		}
 	case TK_UMINUS:{
@@ -3966,7 +3961,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 		}
 	case TK_BITNOT:
 	case TK_NOT:{
-			pExpr->type = FIELD_TYPE_INTEGER;
 			assert(TK_BITNOT == OP_BitNot);
 			testcase(op == TK_BITNOT);
 			assert(TK_NOT == OP_Not);
@@ -3980,7 +3974,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 	case TK_ISNULL:
 	case TK_NOTNULL:{
 			int addr;
-			pExpr->type = FIELD_TYPE_INTEGER;
 			assert(TK_ISNULL == OP_IsNull);
 			testcase(op == TK_ISNULL);
 			assert(TK_NOTNULL == OP_NotNull);
@@ -4225,7 +4218,6 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 	case TK_IN:{
 			int destIfFalse = sqlVdbeMakeLabel(v);
 			int destIfNull = sqlVdbeMakeLabel(v);
-			pExpr->type = FIELD_TYPE_INTEGER;
 			sqlVdbeAddOp2(v, OP_Null, 0, target);
 			sqlExprCodeIN(pParse, pExpr, destIfFalse,
 					  destIfNull);
@@ -4249,18 +4241,15 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 		 * Z is stored in pExpr->pList->a[1].pExpr.
 		 */
 	case TK_BETWEEN:{
-			pExpr->type = FIELD_TYPE_INTEGER;
 			exprCodeBetween(pParse, pExpr, target, 0, 0);
 			return target;
 		}
 	case TK_SPAN:
 	case TK_COLLATE:{
-			pExpr->type = FIELD_TYPE_STRING;
 			return sqlExprCodeTarget(pParse, pExpr->pLeft,
 						     target);
 		}
 	case TK_UPLUS:{
-			pExpr->type = FIELD_TYPE_NUMBER;
 			return sqlExprCodeTarget(pParse, pExpr->pLeft,
 						     target);
 		}
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index b1ec8c758..59f22140c 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1760,7 +1760,7 @@ generateColumnNames(Parse * pParse,	/* Parser context */
 		if (p->op == TK_VARIABLE)
 			var_pos[var_count++] = i;
 		sqlVdbeSetColName(v, i, COLNAME_DECLTYPE,
-				  field_type_strs[p->type], SQL_TRANSIENT);
+				  field_type_strs[sql_expr_type(p)], SQL_TRANSIENT);
 		if (pEList->a[i].zName) {
 			char *zName = pEList->a[i].zName;
 			sqlVdbeSetColName(v, i, COLNAME_NAME, zName,
diff --git a/test/sql/check-clear-ephemeral.result b/test/sql/check-clear-ephemeral.result
index 8fbea982c..84dc7af3d 100644
--- a/test/sql/check-clear-ephemeral.result
+++ b/test/sql/check-clear-ephemeral.result
@@ -26,7 +26,7 @@ box.execute("SELECT a FROM t1 ORDER BY b, a LIMIT 10 OFFSET 20;");
 ---
 - metadata:
   - name: A
-    type: scalar
+    type: integer
   rows:
   - [840]
   - [880]
diff --git a/test/sql/gh-2347-max-int-literals.result b/test/sql/gh-2347-max-int-literals.result
index 35bedd0c9..2c2773e8c 100644
--- a/test/sql/gh-2347-max-int-literals.result
+++ b/test/sql/gh-2347-max-int-literals.result
@@ -23,7 +23,7 @@ box.execute("select (-9223372036854775808)")
 ---
 - metadata:
   - name: (-9223372036854775808)
-    type: number
+    type: integer
   rows:
   - [-9223372036854775808]
 ...
diff --git a/test/sql/gh-3199-no-mem-leaks.result b/test/sql/gh-3199-no-mem-leaks.result
index a873c5019..49ed9775d 100644
--- a/test/sql/gh-3199-no-mem-leaks.result
+++ b/test/sql/gh-3199-no-mem-leaks.result
@@ -29,7 +29,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y')
   - name: X
     type: integer
   - name: Y
-    type: scalar
+    type: integer
   - name: x + y
     type: number
   rows:
@@ -46,7 +46,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y')
   - name: X
     type: integer
   - name: Y
-    type: scalar
+    type: integer
   - name: x + y
     type: number
   rows:
@@ -59,7 +59,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y')
   - name: X
     type: integer
   - name: Y
-    type: scalar
+    type: integer
   - name: x + y
     type: number
   rows:
@@ -72,7 +72,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y')
   - name: X
     type: integer
   - name: Y
-    type: scalar
+    type: integer
   - name: x + y
     type: number
   rows:
@@ -85,7 +85,7 @@ box.execute('SELECT x, y, x + y FROM test ORDER BY y')
   - name: X
     type: integer
   - name: Y
-    type: scalar
+    type: integer
   - name: x + y
     type: number
   rows:
@@ -112,7 +112,7 @@ box.execute('SELECT a, id + 2, b FROM test2 WHERE b < id * 2 ORDER BY a ')
 ---
 - metadata:
   - name: A
-    type: scalar
+    type: string
   - name: id + 2
     type: number
   - name: B
@@ -135,7 +135,7 @@ box.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
   - name: id + 2 * b
     type: number
   - name: A
-    type: scalar
+    type: string
   rows:
   - ['abc', 3, 'abc']
   - ['hello', 6, 'hello']
@@ -150,7 +150,7 @@ box.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
   - name: id + 2 * b
     type: number
   - name: A
-    type: scalar
+    type: string
   rows:
   - ['abc', 3, 'abc']
   - ['hello', 6, 'hello']
@@ -165,7 +165,7 @@ box.execute('SELECT a, id + 2 * b, a FROM test2 WHERE b < id * 2 ORDER BY a ')
   - name: id + 2 * b
     type: number
   - name: A
-    type: scalar
+    type: string
   rows:
   - ['abc', 3, 'abc']
   - ['hello', 6, 'hello']
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/persistency.result b/test/sql/persistency.result
index b4217f808..cd5dcae1f 100644
--- a/test/sql/persistency.result
+++ b/test/sql/persistency.result
@@ -228,7 +228,7 @@ box.execute("SELECT bar, foo, 42, 'awesome' FROM foobar ORDER BY bar")
 ---
 - metadata:
   - name: BAR
-    type: scalar
+    type: string
   - name: FOO
     type: integer
   - name: '42'
@@ -244,7 +244,7 @@ box.execute("SELECT bar, foo, 42, 'awesome' FROM foobar ORDER BY bar DESC")
 ---
 - metadata:
   - name: BAR
-    type: scalar
+    type: string
   - name: FOO
     type: integer
   - name: '42'
@@ -341,7 +341,7 @@ box.execute("SELECT a FROM t1 ORDER BY b, a LIMIT 10 OFFSET 20;");
 ---
 - metadata:
   - name: A
-    type: scalar
+    type: integer
   rows:
   - [840]
   - [880]
@@ -446,7 +446,7 @@ box.execute("SELECT a FROM t1 ORDER BY b, a LIMIT 10 OFFSET 20;");
 ---
 - metadata:
   - name: A
-    type: scalar
+    type: integer
   rows:
   - [840]
   - [880]
diff --git a/test/sql/transition.result b/test/sql/transition.result
index adf5d0816..49edf96c6 100644
--- a/test/sql/transition.result
+++ b/test/sql/transition.result
@@ -225,7 +225,7 @@ box.execute("SELECT bar, foo, 42, 'awesome' FROM foobar ORDER BY bar")
 ---
 - metadata:
   - name: BAR
-    type: scalar
+    type: string
   - name: FOO
     type: integer
   - name: '42'
@@ -241,7 +241,7 @@ box.execute("SELECT bar, foo, 42, 'awesome' FROM foobar ORDER BY bar DESC")
 ---
 - metadata:
   - name: BAR
-    type: scalar
+    type: string
   - name: FOO
     type: integer
   - name: '42'
diff --git a/test/sql/types.result b/test/sql/types.result
index 3aa0169e2..6bccd39ef 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -399,7 +399,7 @@ box.execute("SELECT true and false;")
 ---
 - metadata:
   - name: true and false
-    type: number
+    type: integer
   rows:
   - [0]
 ...
@@ -407,7 +407,7 @@ box.execute("SELECT true or unknown;")
 ---
 - metadata:
   - name: true or unknown
-    type: number
+    type: integer
   rows:
   - [1]
 ...
@@ -481,7 +481,7 @@ box.execute("SELECT b FROM t ORDER BY b;")
 ---
 - metadata:
   - name: B
-    type: scalar
+    type: boolean
   rows:
   - [null]
   - [false]
@@ -501,7 +501,7 @@ box.execute("SELECT b FROM t ORDER BY b LIMIT 1;")
 ---
 - metadata:
   - name: B
-    type: scalar
+    type: boolean
   rows:
   - [null]
 ...
@@ -509,7 +509,7 @@ box.execute("SELECT b FROM t GROUP BY b LIMIT 1;")
 ---
 - metadata:
   - name: B
-    type: scalar
+    type: boolean
   rows:
   - [null]
 ...
-- 
2.15.1

  parent reply	other threads:[~2019-04-14 15:04 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-14 15:03 [tarantool-patches] [PATCH 0/9] Introduce type BOOLEAN in SQL Nikita Pettik
2019-04-14 15:03 ` [tarantool-patches] [PATCH 1/9] sql: refactor mem_apply_numeric_type() Nikita Pettik
2019-04-14 15:04 ` [tarantool-patches] [PATCH 2/9] sql: disallow text values participate in sum() aggregate Nikita Pettik
2019-04-16 14:12   ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-18 17:54     ` n.pettik
2019-04-22 18:02       ` Vladislav Shpilevoy
2019-04-23 19:58         ` n.pettik
2019-04-14 15:04 ` [tarantool-patches] [PATCH 3/9] sql: use msgpack types instead of custom ones Nikita Pettik
2019-04-16 14:12   ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-18 17:54     ` n.pettik
2019-04-22 18:02       ` Vladislav Shpilevoy
2019-04-23 19:58         ` n.pettik
2019-04-14 15:04 ` [tarantool-patches] [PATCH 4/9] sql: introduce type boolean Nikita Pettik
2019-04-16 14:12   ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-18 17:54     ` n.pettik
2019-04-22 18:02       ` Vladislav Shpilevoy
2019-04-23 19:58         ` n.pettik
2019-04-23 21:06           ` Vladislav Shpilevoy
2019-04-14 15:04 ` Nikita Pettik [this message]
2019-04-16 14:12   ` [tarantool-patches] Re: [PATCH 5/9] sql: improve type determination for column meta Vladislav Shpilevoy
2019-04-18 17:54     ` n.pettik
2019-04-22 18:02       ` Vladislav Shpilevoy
2019-04-23 19:58         ` n.pettik
2019-04-14 15:04 ` [tarantool-patches] [PATCH 6/9] sql: make comparison predicate return boolean Nikita Pettik
2019-04-16 14:12   ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-18 17:54     ` n.pettik
2019-04-14 15:04 ` [tarantool-patches] [PATCH 7/9] sql: make predicates accept and " Nikita Pettik
2019-04-16 14:12   ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-18 17:55     ` n.pettik
2019-04-14 15:04 ` [tarantool-patches] [PATCH 9/9] sql: make <search condition> accept only boolean Nikita Pettik
2019-04-16 14:12   ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-18 17:55     ` n.pettik
2019-04-22 18:02       ` Vladislav Shpilevoy
2019-04-23 19:59         ` n.pettik
2019-04-23 21:06           ` Vladislav Shpilevoy
2019-04-23 22:01             ` n.pettik
     [not found] ` <b2a84f129c2343d3da3311469cbb7b20488a21c2.1555252410.git.korablev@tarantool.org>
2019-04-16 14:12   ` [tarantool-patches] Re: [PATCH 8/9] sql: make LIKE predicate return boolean result Vladislav Shpilevoy
2019-04-18 17:55     ` n.pettik
2019-04-22 18:02       ` Vladislav Shpilevoy
2019-04-23 19:58         ` n.pettik
2019-04-24 10:28 ` [tarantool-patches] Re: [PATCH 0/9] Introduce type BOOLEAN in SQL Vladislav Shpilevoy
2019-04-25  8:46 ` 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=c19cef87e6d8e54a527600a0c424346c15c12ce8.1555252410.git.korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [tarantool-patches] [PATCH 5/9] sql: improve type determination for column meta' \
    /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