From cb237e7da9fad938362aad8c2e3171279e97b06d Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Thu, 11 Apr 2019 20:34:43 +0300
Subject: [PATCH] sql: make predicates accept and return boolean

This patch make following predicates accept and return only values
of type boolean: IN, EXISTS, OR, AND, NOT, BETWEEN, IS (NULL).
In terms of approach, it is enough to patch opcodes implementing these
predicates.

Part of #3723
---
 src/box/sql/expr.c              | 26 +++++++++++-------------
 src/box/sql/parse.y             |  4 +++-
 src/box/sql/select.c            |  2 +-
 src/box/sql/vdbe.c              | 35 +++++++++++----------------------
 test/sql-tap/cse.test.lua       |  6 +++---
 test/sql-tap/e_select1.test.lua |  2 +-
 test/sql-tap/in1.test.lua       |  6 +++---
 test/sql-tap/misc3.test.lua     |  2 +-
 test/sql-tap/select4.test.lua   |  2 +-
 test/sql-tap/tkt3541.test.lua   |  2 +-
 test/sql/types.result           | 22 ++++++++++++---------
 11 files changed, 51 insertions(+), 58 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ad84bdcdf0..ab43999fdb 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -102,20 +102,16 @@ sql_expr_type(struct Expr *pExpr)
 	case TK_EQ:
 	case TK_LE:
 	case TK_NE:
-		return FIELD_TYPE_BOOLEAN;
 	case TK_NOT:
 	case TK_AND:
 	case TK_OR:
 	case TK_ISNULL:
 	case TK_NOTNULL:
 	case TK_BETWEEN:
+	case TK_EXISTS:
 	case TK_IN:
-		/*
-		 * FIXME: should be changed to BOOL type
-		 * when it is implemented. Now simply
-		 * return INTEGER.
-		 */
-		return FIELD_TYPE_INTEGER;
+	case TK_IS:
+		return FIELD_TYPE_BOOLEAN;
 	case TK_UMINUS:
 	case TK_UPLUS:
 	case TK_NO:
@@ -1131,7 +1127,10 @@ sql_and_expr_new(struct sql *db, struct Expr *left_expr,
 	} else if (exprAlwaysFalse(left_expr) || exprAlwaysFalse(right_expr)) {
 		sql_expr_delete(db, left_expr, false);
 		sql_expr_delete(db, right_expr, false);
-		return sql_expr_new(db, TK_INTEGER, &sqlIntTokens[0]);
+		struct Expr *f = sql_expr_new_anon(db, TK_FALSE);
+		if (f != NULL)
+			f->type = FIELD_TYPE_BOOLEAN;
+		return f;
 	} else {
 		struct Expr *new_expr = sql_expr_new_anon(db, TK_AND);
 		sqlExprAttachSubtrees(db, new_expr, left_expr, right_expr);
@@ -2925,8 +2924,7 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
 				VdbeComment((v, "Init subquery result"));
 			} else {
 				dest.eDest = SRT_Exists;
-				sqlVdbeAddOp2(v, OP_Integer, 0,
-						  dest.iSDParm);
+				sqlVdbeAddOp2(v, OP_Bool, false, dest.iSDParm);
 				VdbeComment((v, "Init EXISTS result"));
 			}
 			if (pSel->pLimit == NULL) {
@@ -3958,14 +3956,14 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			testcase(op == TK_ISNULL);
 			assert(TK_NOTNULL == OP_NotNull);
 			testcase(op == TK_NOTNULL);
-			sqlVdbeAddOp2(v, OP_Integer, 1, target);
+			sqlVdbeAddOp2(v, OP_Bool, true, target);
 			r1 = sqlExprCodeTemp(pParse, pExpr->pLeft,
 						 &regFree1);
 			testcase(regFree1 == 0);
 			addr = sqlVdbeAddOp1(v, op, r1);
 			VdbeCoverageIf(v, op == TK_ISNULL);
 			VdbeCoverageIf(v, op == TK_NOTNULL);
-			sqlVdbeAddOp2(v, OP_Integer, 0, target);
+			sqlVdbeAddOp2(v, OP_Bool, false, target);
 			sqlVdbeJumpHere(v, addr);
 			break;
 		}
@@ -4200,10 +4198,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			sqlVdbeAddOp2(v, OP_Null, 0, target);
 			sqlExprCodeIN(pParse, pExpr, destIfFalse,
 					  destIfNull);
-			sqlVdbeAddOp2(v, OP_Integer, 1, target);
+			sqlVdbeAddOp2(v, OP_Bool, true, target);
 			sqlVdbeGoto(v, destIfNull);
 			sqlVdbeResolveLabel(v, destIfFalse);
-			sqlVdbeAddOp2(v, OP_Integer, 0, target);
+			sqlVdbeAddOp2(v, OP_Bool, false, target);
 			sqlVdbeResolveLabel(v, destIfNull);
 			return target;
 		}
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 421e182eaa..3a443a068a 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1287,11 +1287,13 @@ expr(A) ::= expr(A) in_op(N) LP exprlist(Y) RP(E). [IN] {
     ** regardless of the value of expr1.
     */
     sql_expr_delete(pParse->db, A.pExpr, false);
-    A.pExpr = sql_expr_new_dequoted(pParse->db, TK_INTEGER, &sqlIntTokens[N]);
+    int tk = N == 0 ? TK_FALSE : TK_TRUE;
+    A.pExpr = sql_expr_new_anon(pParse->db, tk);
     if (A.pExpr == NULL) {
       pParse->is_aborted = true;
       return;
     }
+    A.pExpr->type = FIELD_TYPE_BOOLEAN;
   }else if( Y->nExpr==1 ){
     /* Expressions of the form:
     **
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 181c051e01..563f660e5b 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -1251,7 +1251,7 @@ selectInnerLoop(Parse * pParse,		/* The parser context */
 		/* If any row exist in the result set, record that fact and abort.
 		 */
 	case SRT_Exists:{
-			sqlVdbeAddOp2(v, OP_Integer, 1, iParm);
+			sqlVdbeAddOp2(v, OP_Bool, true, iParm);
 			/* The LIMIT clause will terminate the loop for us */
 			break;
 		}
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 016ad11431..1f65e991eb 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2415,14 +2415,10 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 	} else if ((pIn1->flags & MEM_Bool) != 0) {
 		v1 = pIn1->u.b;
 	} else {
-		int64_t i;
-		if (sqlVdbeIntValue(pIn1, &i) != 0) {
-			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn1), "integer");
-			rc = SQL_TARANTOOL_ERROR;
-			goto abort_due_to_error;
-		}
-		v1 = i != 0;
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 sql_value_text(pIn1), "boolean");
+		rc = SQL_TARANTOOL_ERROR;
+		goto abort_due_to_error;
 	}
 	pIn2 = &aMem[pOp->p2];
 	if (pIn2->flags & MEM_Null) {
@@ -2430,14 +2426,10 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 	} else if ((pIn2->flags & MEM_Bool) != 0) {
 		v2 = pIn2->u.b;
 	} else {
-		int64_t i;
-		if (sqlVdbeIntValue(pIn2, &i) != 0) {
-			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn2), "integer");
-			rc = SQL_TARANTOOL_ERROR;
-			goto abort_due_to_error;
-		}
-		v2 = i != 0;
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 sql_value_text(pIn2), "boolean");
+		rc = SQL_TARANTOOL_ERROR;
+		goto abort_due_to_error;
 	}
 	if (pOp->opcode==OP_And) {
 		static const unsigned char and_logic[] = { 0, 0, 0, 0, 1, 2, 0, 2, 2 };
@@ -2450,8 +2442,7 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 	if (v1==2) {
 		MemSetTypeFlag(pOut, MEM_Null);
 	} else {
-		pOut->u.i = v1;
-		MemSetTypeFlag(pOut, MEM_Int);
+		mem_set_bool(pOut, v1);
 	}
 	break;
 }
@@ -2468,15 +2459,13 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
 	pOut = &aMem[pOp->p2];
 	sqlVdbeMemSetNull(pOut);
 	if ((pIn1->flags & MEM_Null)==0) {
-		int64_t i;
-		if (sqlVdbeIntValue(pIn1, &i) != 0) {
+		if ((pIn1->flags & MEM_Bool) == 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn1), "integer");
+				 sql_value_text(pIn1), "boolean");
 			rc = SQL_TARANTOOL_ERROR;
 			goto abort_due_to_error;
 		}
-		pOut->flags = MEM_Int;
-		pOut->u.i = !i;
+		mem_set_bool(pOut, ! pIn1->u.b);
 	}
 	break;
 }
diff --git a/test/sql-tap/cse.test.lua b/test/sql-tap/cse.test.lua
index 4b25f936da..39c1cc4ca5 100755
--- a/test/sql-tap/cse.test.lua
+++ b/test/sql-tap/cse.test.lua
@@ -31,7 +31,7 @@ test:do_test(
             INSERT INTO t1 VALUES(2,21,22,23,24,25);
         ]]
         return test:execsql [[
-            SELECT b, -b, ~b, NOT b, NOT NOT b, b-b, b+b, b*b, b/b, b FROM t1
+            SELECT b, -b, ~b, NOT CAST(b AS BOOLEAN), NOT NOT CAST(b AS BOOLEAN), b-b, b+b, b*b, b/b, b FROM t1
         ]]
     end, {
         -- <cse-1.1>
@@ -132,7 +132,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cse-1.7",
     [[
-        SELECT a, -a, ~a, NOT a, NOT NOT a, a-a, a+a, a*a, a/a, a FROM t1
+        SELECT a, -a, ~a, NOT CAST(a AS BOOLEAN), NOT NOT CAST(a AS BOOLEAN), a-a, a+a, a*a, a/a, a FROM t1
     ]], {
         -- <cse-1.7>
         1, -1, -2, 0, 1, 0, 2, 1, 1, 1, 2, -2, -3, 0, 1, 0, 4, 4, 1, 2
@@ -152,7 +152,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cse-1.9",
     [[
-        SELECT NOT b, ~b, NOT NOT b, b FROM t1
+        SELECT NOT CAST(b AS BOOLEAN), ~b, NOT NOT CAST(b AS BOOLEAN), b FROM t1
     ]], {
         -- <cse-1.9>
         0, -12, 1, 11, 0, -22, 1, 21
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index 8e9a2bb237..970eeeed9e 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -897,7 +897,7 @@ test:do_select_tests(
 
         {"3", "SELECT sum(b+1) FROM z1 NATURAL LEFT JOIN z3", {-43.06}},
         {"4", "SELECT sum(b+2) FROM z1 NATURAL LEFT JOIN z3", {-38.06}},
-        {"5", "SELECT sum(b IS NOT NULL) FROM z1 NATURAL LEFT JOIN z3", {5}},
+        {"5", "SELECT sum(CAST(b IS NOT NULL AS INTEGER)) FROM z1 NATURAL LEFT JOIN z3", {5}},
     })
 
 -- EVIDENCE-OF: R-26684-40576 Each non-aggregate expression in the
diff --git a/test/sql-tap/in1.test.lua b/test/sql-tap/in1.test.lua
index 835c10dd56..08a7c36287 100755
--- a/test/sql-tap/in1.test.lua
+++ b/test/sql-tap/in1.test.lua
@@ -100,7 +100,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "in-1.7",
     [[
-        SELECT a+ 100*(a BETWEEN 1 and 3) FROM t1 ORDER BY b
+        SELECT a+ 100*CAST((a BETWEEN 1 and 3) AS INTEGER) FROM t1 ORDER BY b
     ]], {
         -- <in-1.7>
         101, 102, 103, 4, 5, 6, 7, 8, 9, 10
@@ -157,7 +157,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "in-2.5",
     [[
-        SELECT a+100*(b IN (8,16,24)) FROM t1 ORDER BY b
+        SELECT a+100*(CAST(b IN (8,16,24) AS INTEGER)) FROM t1 ORDER BY b
     ]], {
         -- <in-2.5>
         1, 2, 103, 104, 5, 6, 7, 8, 9, 10
@@ -253,7 +253,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "in-3.3",
     [[
-        SELECT a + 100*(b IN (SELECT b FROM t1 WHERE a<5)) FROM t1 ORDER BY b
+        SELECT a + 100*(CAST(b IN (SELECT b FROM t1 WHERE a<5) AS INTEGER)) FROM t1 ORDER BY b
     ]], {
         -- <in-3.3>
         101, 102, 103, 104, 5, 6, 7, 8, 9, 10
diff --git a/test/sql-tap/misc3.test.lua b/test/sql-tap/misc3.test.lua
index eed40f2cd8..d0e45e872d 100755
--- a/test/sql-tap/misc3.test.lua
+++ b/test/sql-tap/misc3.test.lua
@@ -511,7 +511,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "misc-8.2",
     [[
-        SELECT count(*) FROM t3 WHERE 1+(b IN ('abc','xyz'))==2
+        SELECT count(*) FROM t3 WHERE 1+CAST((b IN ('abc','xyz')) AS INTEGER)==2
     ]], {
         -- <misc-8.2>
         2
diff --git a/test/sql-tap/select4.test.lua b/test/sql-tap/select4.test.lua
index 3aafedb4ce..b78091b28b 100755
--- a/test/sql-tap/select4.test.lua
+++ b/test/sql-tap/select4.test.lua
@@ -1449,7 +1449,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "select4-14.15",
     [[
-        SELECT * FROM (SELECT 123), (SELECT 456) ON likely(0 OR 1) OR 0;
+        SELECT * FROM (SELECT 123), (SELECT 456) ON likely(false OR true) OR false;
     ]], {
         -- <select4-14.15>
         123, 456
diff --git a/test/sql-tap/tkt3541.test.lua b/test/sql-tap/tkt3541.test.lua
index 08a9be5574..0d76e77449 100755
--- a/test/sql-tap/tkt3541.test.lua
+++ b/test/sql-tap/tkt3541.test.lua
@@ -39,7 +39,7 @@ test:do_test(
     "tkt3541-1.2",
     function()
         return test:execsql [[
-            SELECT CASE NOT max(x) WHEN min(x) THEN 1 ELSE max(x) END FROM t1;
+            SELECT CASE max(x) = 0 WHEN min(x) <> 0 THEN 1 ELSE max(x) END FROM t1;
         ]]
     end, {
         -- <tkt3541-1.2>
diff --git a/test/sql/types.result b/test/sql/types.result
index 07f98a702b..de17bbb781 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -337,17 +337,17 @@ box.execute("SELECT true IN (1, 'abc', true)")
 ---
 - metadata:
   - name: true IN (1, 'abc', true)
-    type: integer
+    type: boolean
   rows:
-  - [1]
+  - [true]
 ...
 box.execute("SELECT true IN (1, 'abc', false)")
 ---
 - metadata:
   - name: true IN (1, 'abc', false)
-    type: integer
+    type: boolean
   rows:
-  - [0]
+  - [false]
 ...
 box.execute("SELECT 1 LIMIT true;")
 ---
@@ -377,7 +377,11 @@ box.execute("SELECT false / 0;")
 ...
 box.execute("SELECT not true;")
 ---
-- error: 'Type mismatch: can not convert true to integer'
+- metadata:
+  - name: not true
+    type: boolean
+  rows:
+  - [false]
 ...
 box.execute("SELECT ~true;")
 ---
@@ -399,17 +403,17 @@ box.execute("SELECT true and false;")
 ---
 - metadata:
   - name: true and false
-    type: integer
+    type: boolean
   rows:
-  - [0]
+  - [false]
 ...
 box.execute("SELECT true or unknown;")
 ---
 - metadata:
   - name: true or unknown
-    type: integer
+    type: boolean
   rows:
-  - [1]
+  - [true]
 ...
 box.execute("CREATE TABLE t (id INT PRIMARY KEY, b BOOLEAN);")
 ---
-- 
GitLab