[tarantool-patches] [PATCH 5/9] sql: improve type determination for column meta

Nikita Pettik korablev at tarantool.org
Sun Apr 14 18:04:03 MSK 2019


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





More information about the Tarantool-patches mailing list