[tarantool-patches] [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[tarantool-patches] [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[imeevma]

This patch reworks SQL errors of type "expected column count".

Closes #4204
---
https://github.com/tarantool/tarantool/issues/4204
https://github.com/tarantool/tarantool/tree/imeevma/gh-4204-rework-column-count-errors

 src/box/errcode.h                |   1 +
 src/box/sql/expr.c               |  20 ++---
 src/box/sql/resolve.c            |  11 +--
 src/box/sql/select.c             |   6 +-
 test/box/misc.result             |   1 +
 test/sql-tap/in1.test.lua        |  14 ++--
 test/sql-tap/select7.test.lua    |   8 +-
 test/sql-tap/sql-errors.test.lua | 164 ++++++++++++++++++++++++++++++++++++++-
 test/sql-tap/subselect.test.lua  |   2 +-
 9 files changed, 186 insertions(+), 41 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 9c15f33..789556a 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -247,6 +247,7 @@ struct errcode_record {
 	/*192 */_(ER_INDEX_DEF_UNSUPPORTED,	"%s are prohibited in an index definition") \
 	/*193 */_(ER_CK_DEF_UNSUPPORTED,	"%s are prohibited in a CHECK constraint definition") \
 	/*194 */_(ER_MULTIKEY_INDEX_MISMATCH,	"Field %s is used as multikey in one index and as single key in another") \
+	/*195 */_(ER_SQL_COLUMN_COUNT,		"Left value column count %u does not match the expected column count (%u)") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ba7eea5..8cf4d2e 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
  * Expr pIn is an IN(...) expression. This function checks that the
  * sub-select on the RHS of the IN() operator has the same number of
  * columns as the vector on the LHS. Or, if the RHS of the IN() is not
- * a sub-query, that the LHS is a vector of size 1.
+ * a sub-query, that the LHS must be a vector of size 1.
  */
 int
 sqlExprCheckIN(Parse * pParse, Expr * pIn)
 {
 	int nVector = sqlExprVectorSize(pIn->pLeft);
-	const char *err;
 	if ((pIn->flags & EP_xIsSelect)) {
 		if (nVector != pIn->x.pSelect->pEList->nExpr) {
-			err = "sub-select returns %d columns - expected %d";
 			int expr_count = pIn->x.pSelect->pEList->nExpr;
-			diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-				 tt_sprintf(err, expr_count, nVector));
+			diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector,
+				 expr_count);
 			pParse->is_aborted = true;
 			return 1;
 		}
 	} else if (nVector != 1) {
-		assert((pIn->pLeft->flags & EP_xIsSelect) != 0);
-		int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr;
-		err = tt_sprintf("sub-select returns %d columns - expected 1",
-				 expr_count);
-		diag_set(ClientError, ER_SQL_PARSER_GENERIC, err);
+		diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector, 1);
 		pParse->is_aborted = true;
 		return 1;
 	}
@@ -4153,10 +4147,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			testcase(op == TK_SELECT);
 			if (op == TK_SELECT
 			    && (nCol = pExpr->x.pSelect->pEList->nExpr) != 1) {
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, nCol));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nCol, 1);
 				pParse->is_aborted = true;
 			} else {
 				return sqlCodeSubselect(pParse, pExpr, 0);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 504096e..3d45841 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -781,15 +781,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 				nRight = sqlExprVectorSize(pExpr->pRight);
 			}
 			if (nLeft != nRight) {
-				testcase(pExpr->op == TK_EQ);
-				testcase(pExpr->op == TK_NE);
-				testcase(pExpr->op == TK_LT);
-				testcase(pExpr->op == TK_LE);
-				testcase(pExpr->op == TK_GT);
-				testcase(pExpr->op == TK_GE);
-				testcase(pExpr->op == TK_BETWEEN);
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 "row value misused");
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nLeft, nRight);
 				pParse->is_aborted = true;
 			}
 			break;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index d3472a9..69b888e 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -3616,12 +3616,10 @@ substExpr(Parse * pParse,	/* Report errors here */
 			assert(pExpr->pLeft == 0 && pExpr->pRight == 0);
 			if (sqlExprIsVector(pCopy)) {
 				assert((pCopy->flags & EP_xIsSelect) != 0);
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
 				int expr_count =
 					pCopy->x.pSelect->pEList->nExpr;
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, expr_count));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 expr_count, 1);
 				pParse->is_aborted = true;
 			} else {
 				pNew = sqlExprDup(db, pCopy, 0);
diff --git a/test/box/misc.result b/test/box/misc.result
index 4fcd13a..766bf9c 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -523,6 +523,7 @@ t;
   192: box.error.INDEX_DEF_UNSUPPORTED
   193: box.error.CK_DEF_UNSUPPORTED
   194: box.error.MULTIKEY_INDEX_MISMATCH
+  195: box.error.SQL_COLUMN_COUNT
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql-tap/in1.test.lua b/test/sql-tap/in1.test.lua
index 76112cf..b280ed8 100755
--- a/test/sql-tap/in1.test.lua
+++ b/test/sql-tap/in1.test.lua
@@ -594,7 +594,7 @@ test:do_catchsql_test(
         SELECT b FROM t1 WHERE a NOT IN tb;
     ]], {
         -- <in-9.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </in-9.4>
     })
 
@@ -677,7 +677,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </in-12.2>
     })
 
@@ -689,7 +689,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </in-12.3>
     })
 
@@ -701,7 +701,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </in-12.4>
     })
 
@@ -713,7 +713,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.5>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </in-12.5>
     })
 
@@ -823,7 +823,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.14>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </in-12.14>
     })
 
@@ -1058,7 +1058,7 @@ test:do_catchsql_test(
         SELECT 0 WHERE (SELECT 0,0) OR (0 IN (1,2));
     ]], {
         -- <in-13.15>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 2 does not match the expected column count (1)"
         -- </in-13.15>
     })
 
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index f2a071d..c7b253b 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -130,7 +130,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2);
     ]], {
         -- <select7-5.1>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </select7-5.1>
     })
 
@@ -140,7 +140,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2);
     ]], {
         -- <select7-5.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </select7-5.2>
     })
 
@@ -150,7 +150,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2 UNION SELECT b,a FROM t2);
     ]], {
         -- <select7-5.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </select7-5.3>
     })
 
@@ -160,7 +160,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2 UNION SELECT * FROM t2);
     ]], {
         -- <select7-5.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </select7-5.4>
     })
 
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 9357c40..8a11d90 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(47)
+test:plan(63)
 
 test:execsql([[
 	CREATE TABLE t0 (i INT PRIMARY KEY, a INT);
@@ -462,7 +462,7 @@ test:do_catchsql_test(
 		SELECT (1,2,3) == (1,2,3,4);
 	]], {
 		-- <sql-errors-1.41>
-		1,"row value misused"
+		1,"Left value column count 3 does not match the expected column count (4)"
 		-- </sql-errors-1.41>
 	})
 
@@ -526,4 +526,164 @@ test:do_catchsql_test(
 		-- </sql-errors-1.47>
 	})
 
+test:do_catchsql_test(
+	"sql-errors-1.48",
+	[[
+		SELECT (1,2) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.48>
+		1, "Left value column count 2 does not match the expected column count (1)"
+		-- </sql-errors-1.48>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.49",
+	[[
+		SELECT (1,2) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.49>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.49>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.50",
+	[[
+		SELECT (1,2) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.50>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.50>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.51",
+	[[
+		SELECT (1,2) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.51>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.51>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.52",
+	[[
+		SELECT (1) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.52>
+		0, {true}
+		-- </sql-errors-1.52>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.53",
+	[[
+		SELECT (1,2,3) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.53>
+		0, {true}
+		-- </sql-errors-1.53>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.54",
+	[[
+		SELECT (1,2,3) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.54>
+		0, {true}
+		-- </sql-errors-1.54>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.55",
+	[[
+		SELECT (1,2,3) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.55>
+		0, {true}
+		-- </sql-errors-1.55>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.56",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.56>
+		1, "Left value column count 2 does not match the expected column count (1)"
+		-- </sql-errors-1.56>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.57",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.57>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.57>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.58",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.58>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.58>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.59",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.59>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.59>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.60",
+	[[
+		SELECT (1,2,3) = (1,2,3);
+	]], {
+		-- <sql-errors-1.60>
+		0, {true}
+		-- </sql-errors-1.60>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.61",
+	[[
+		SELECT (1,2) = (1,2,3);
+	]], {
+		-- <sql-errors-1.61>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.61>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.62",
+	[[
+		SELECT (SELECT (1), (2)) = (1,2,3);
+	]], {
+		-- <sql-errors-1.62>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.62>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.63",
+	[[
+		SELECT (VALUES (1,2)) = (1,2,3);
+	]], {
+		-- <sql-errors-1.63>
+		1, "Left value column count 2 does not match the expected column count (3)"
+		-- </sql-errors-1.63>
+	})
+
 test:finish_test()
diff --git a/test/sql-tap/subselect.test.lua b/test/sql-tap/subselect.test.lua
index 5b71390..d7cd600 100755
--- a/test/sql-tap/subselect.test.lua
+++ b/test/sql-tap/subselect.test.lua
@@ -49,7 +49,7 @@ test:do_catchsql_test(
         SELECT * FROM t1 WHERE a = (SELECT * FROM t1)
     ]], {
         -- <subselect-1.2>
-        1, "row value misused"
+        1, "Left value column count 1 does not match the expected column count (2)"
         -- </subselect-1.2>
     })
 
-- 
2.7.4

[tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[n.pettik]

> On 18 May 2019, at 13:53, imeevma@tarantool.org wrote:
> 
> This patch reworks SQL errors of type "expected column count”.

Please, provide decent description of the patch.

> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 9c15f33..789556a 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -247,6 +247,7 @@ struct errcode_record {
> 	/*192 */_(ER_INDEX_DEF_UNSUPPORTED,	"%s are prohibited in an index definition") \
> 	/*193 */_(ER_CK_DEF_UNSUPPORTED,	"%s are prohibited in a CHECK constraint definition") \
> 	/*194 */_(ER_MULTIKEY_INDEX_MISMATCH,	"Field %s is used as multikey in one index and as single key in another") \
> +	/*195 */_(ER_SQL_COLUMN_COUNT,		"Left value column count %u does not match the expected column count (%u)") \

-> ER_SQL_ROW_EXPRESSION

I would re-phrase message to smth like this:

“Misuse of row expression: left side of row value has %u entries, but right side - %u”
“Unequal number of entries in row expression: left side has %u, but right side - %u”

> /*
>  * !IMPORTANT! Please follow instructions at start of the file
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index ba7eea5..8cf4d2e 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
>  * Expr pIn is an IN(...) expression. This function checks that the
>  * sub-select on the RHS of the IN() operator has the same number of
>  * columns as the vector on the LHS. Or, if the RHS of the IN() is not
> - * a sub-query, that the LHS is a vector of size 1.
> + * a sub-query, that the LHS must be a vector of size 1.

What is the point of changing this comment?

>  */
> int
> sqlExprCheckIN(Parse * pParse, Expr * pIn)
> {
> 	int nVector = sqlExprVectorSize(pIn->pLeft);
> -	const char *err;
> 	if ((pIn->flags & EP_xIsSelect)) {
> 		if (nVector != pIn->x.pSelect->pEList->nExpr) {
> -			err = "sub-select returns %d columns - expected %d";
> 			int expr_count = pIn->x.pSelect->pEList->nExpr;
> -			diag_set(ClientError, ER_SQL_PARSER_GENERIC,
> -				 tt_sprintf(err, expr_count, nVector));
> +			diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector,
> +				 expr_count);
> 			pParse->is_aborted = true;
> 			return 1;
> 		}
> 	} else if (nVector != 1) {
> -		assert((pIn->pLeft->flags & EP_xIsSelect) != 0);
> -		int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr;
> -		err = tt_sprintf("sub-select returns %d columns - expected 1",
> -				 expr_count);
> -		diag_set(ClientError, ER_SQL_PARSER_GENERIC, err);
> +		diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector, 1);
> 		pParse->is_aborted = true;
> 		return 1;
> 	}
> 

[tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[Mergen Imeev]

Hi! Thank you for review! My answers and new patch below.

On Sun, May 19, 2019 at 05:38:16PM +0300, n.pettik wrote:
> 
> 
> > On 18 May 2019, at 13:53, imeevma@tarantool.org wrote:
> > 
> > This patch reworks SQL errors of type "expected column count”.
> 
> Please, provide decent description of the patch.
> 
Fixed.

> > diff --git a/src/box/errcode.h b/src/box/errcode.h
> > index 9c15f33..789556a 100644
> > --- a/src/box/errcode.h
> > +++ b/src/box/errcode.h
> > @@ -247,6 +247,7 @@ struct errcode_record {
> > 	/*192 */_(ER_INDEX_DEF_UNSUPPORTED,	"%s are prohibited in an index definition") \
> > 	/*193 */_(ER_CK_DEF_UNSUPPORTED,	"%s are prohibited in a CHECK constraint definition") \
> > 	/*194 */_(ER_MULTIKEY_INDEX_MISMATCH,	"Field %s is used as multikey in one index and as single key in another") \
> > +	/*195 */_(ER_SQL_COLUMN_COUNT,		"Left value column count %u does not match the expected column count (%u)") \
> 
> -> ER_SQL_ROW_EXPRESSION
> 
> I would re-phrase message to smth like this:
> 
> “Misuse of row expression: left side of row value has %u entries, but right side - %u”
> “Unequal number of entries in row expression: left side has %u, but right side - %u”
> 
Thanks. I used the second option.

> > /*
> >  * !IMPORTANT! Please follow instructions at start of the file
> > diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> > index ba7eea5..8cf4d2e 100644
> > --- a/src/box/sql/expr.c
> > +++ b/src/box/sql/expr.c
> > @@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
> >  * Expr pIn is an IN(...) expression. This function checks that the
> >  * sub-select on the RHS of the IN() operator has the same number of
> >  * columns as the vector on the LHS. Or, if the RHS of the IN() is not
> > - * a sub-query, that the LHS is a vector of size 1.
> > + * a sub-query, that the LHS must be a vector of size 1.
> 
> What is the point of changing this comment?
> 
In SQlite it wasn't possible to write something like
"SELECT (1,2) in ..." so if length of the vector were more
than 1 it means it was received using subselect. It isn't
so in our case since we can work with such statements.
To emphasize this, I changed the comment.


New patch:

From 32b665f9ad9dacc16a492c55396dd843f9a355e9 Mon Sep 17 00:00:00 2001
Date: Sat, 18 May 2019 13:15:58 +0300
Subject: [PATCH] sql: rework SQL errors of type "expected column count"

Before this patch, errors of type "expected column count" were
divided into several different errors, and they all had an
ER_SQL_PARSER_GENERIC error code. This patch creates a new error
code for these errors.

In addition, at some point it became possible to use vectors as
the left value in the IN operator. Since this was previously
impossible, it led to a segmentation error.

Closes #4204

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 9c15f33..c0042b5 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -247,6 +247,7 @@ struct errcode_record {
 	/*192 */_(ER_INDEX_DEF_UNSUPPORTED,	"%s are prohibited in an index definition") \
 	/*193 */_(ER_CK_DEF_UNSUPPORTED,	"%s are prohibited in a CHECK constraint definition") \
 	/*194 */_(ER_MULTIKEY_INDEX_MISMATCH,	"Field %s is used as multikey in one index and as single key in another") \
+	/*195 */_(ER_SQL_COLUMN_COUNT,		"Unequal number of entries in row expression: left side has %u, but right side - %u") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index ba7eea5..8cf4d2e 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
  * Expr pIn is an IN(...) expression. This function checks that the
  * sub-select on the RHS of the IN() operator has the same number of
  * columns as the vector on the LHS. Or, if the RHS of the IN() is not
- * a sub-query, that the LHS is a vector of size 1.
+ * a sub-query, that the LHS must be a vector of size 1.
  */
 int
 sqlExprCheckIN(Parse * pParse, Expr * pIn)
 {
 	int nVector = sqlExprVectorSize(pIn->pLeft);
-	const char *err;
 	if ((pIn->flags & EP_xIsSelect)) {
 		if (nVector != pIn->x.pSelect->pEList->nExpr) {
-			err = "sub-select returns %d columns - expected %d";
 			int expr_count = pIn->x.pSelect->pEList->nExpr;
-			diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-				 tt_sprintf(err, expr_count, nVector));
+			diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector,
+				 expr_count);
 			pParse->is_aborted = true;
 			return 1;
 		}
 	} else if (nVector != 1) {
-		assert((pIn->pLeft->flags & EP_xIsSelect) != 0);
-		int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr;
-		err = tt_sprintf("sub-select returns %d columns - expected 1",
-				 expr_count);
-		diag_set(ClientError, ER_SQL_PARSER_GENERIC, err);
+		diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector, 1);
 		pParse->is_aborted = true;
 		return 1;
 	}
@@ -4153,10 +4147,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			testcase(op == TK_SELECT);
 			if (op == TK_SELECT
 			    && (nCol = pExpr->x.pSelect->pEList->nExpr) != 1) {
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, nCol));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nCol, 1);
 				pParse->is_aborted = true;
 			} else {
 				return sqlCodeSubselect(pParse, pExpr, 0);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 504096e..3d45841 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -781,15 +781,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 				nRight = sqlExprVectorSize(pExpr->pRight);
 			}
 			if (nLeft != nRight) {
-				testcase(pExpr->op == TK_EQ);
-				testcase(pExpr->op == TK_NE);
-				testcase(pExpr->op == TK_LT);
-				testcase(pExpr->op == TK_LE);
-				testcase(pExpr->op == TK_GT);
-				testcase(pExpr->op == TK_GE);
-				testcase(pExpr->op == TK_BETWEEN);
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 "row value misused");
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nLeft, nRight);
 				pParse->is_aborted = true;
 			}
 			break;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index d3472a9..69b888e 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -3616,12 +3616,10 @@ substExpr(Parse * pParse,	/* Report errors here */
 			assert(pExpr->pLeft == 0 && pExpr->pRight == 0);
 			if (sqlExprIsVector(pCopy)) {
 				assert((pCopy->flags & EP_xIsSelect) != 0);
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
 				int expr_count =
 					pCopy->x.pSelect->pEList->nExpr;
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, expr_count));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 expr_count, 1);
 				pParse->is_aborted = true;
 			} else {
 				pNew = sqlExprDup(db, pCopy, 0);
diff --git a/test/box/misc.result b/test/box/misc.result
index 4fcd13a..766bf9c 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -523,6 +523,7 @@ t;
   192: box.error.INDEX_DEF_UNSUPPORTED
   193: box.error.CK_DEF_UNSUPPORTED
   194: box.error.MULTIKEY_INDEX_MISMATCH
+  195: box.error.SQL_COLUMN_COUNT
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql-tap/in1.test.lua b/test/sql-tap/in1.test.lua
index 76112cf..bc0dfb8 100755
--- a/test/sql-tap/in1.test.lua
+++ b/test/sql-tap/in1.test.lua
@@ -594,7 +594,7 @@ test:do_catchsql_test(
         SELECT b FROM t1 WHERE a NOT IN tb;
     ]], {
         -- <in-9.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-9.4>
     })
 
@@ -677,7 +677,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.2>
     })
 
@@ -689,7 +689,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.3>
     })
 
@@ -701,7 +701,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.4>
     })
 
@@ -713,7 +713,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.5>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.5>
     })
 
@@ -823,7 +823,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.14>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.14>
     })
 
@@ -1058,7 +1058,7 @@ test:do_catchsql_test(
         SELECT 0 WHERE (SELECT 0,0) OR (0 IN (1,2));
     ]], {
         -- <in-13.15>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
         -- </in-13.15>
     })
 
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index f2a071d..3b36d57 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -130,7 +130,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2);
     ]], {
         -- <select7-5.1>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.1>
     })
 
@@ -140,7 +140,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2);
     ]], {
         -- <select7-5.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.2>
     })
 
@@ -150,7 +150,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2 UNION SELECT b,a FROM t2);
     ]], {
         -- <select7-5.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.3>
     })
 
@@ -160,7 +160,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2 UNION SELECT * FROM t2);
     ]], {
         -- <select7-5.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.4>
     })
 
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 9357c40..da10ecb 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(47)
+test:plan(63)
 
 test:execsql([[
 	CREATE TABLE t0 (i INT PRIMARY KEY, a INT);
@@ -462,7 +462,7 @@ test:do_catchsql_test(
 		SELECT (1,2,3) == (1,2,3,4);
 	]], {
 		-- <sql-errors-1.41>
-		1,"row value misused"
+		1,"Unequal number of entries in row expression: left side has 3, but right side - 4"
 		-- </sql-errors-1.41>
 	})
 
@@ -526,4 +526,164 @@ test:do_catchsql_test(
 		-- </sql-errors-1.47>
 	})
 
+test:do_catchsql_test(
+	"sql-errors-1.48",
+	[[
+		SELECT (1,2) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.48>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
+		-- </sql-errors-1.48>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.49",
+	[[
+		SELECT (1,2) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.49>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.49>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.50",
+	[[
+		SELECT (1,2) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.50>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.50>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.51",
+	[[
+		SELECT (1,2) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.51>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.51>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.52",
+	[[
+		SELECT (1) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.52>
+		0, {true}
+		-- </sql-errors-1.52>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.53",
+	[[
+		SELECT (1,2,3) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.53>
+		0, {true}
+		-- </sql-errors-1.53>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.54",
+	[[
+		SELECT (1,2,3) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.54>
+		0, {true}
+		-- </sql-errors-1.54>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.55",
+	[[
+		SELECT (1,2,3) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.55>
+		0, {true}
+		-- </sql-errors-1.55>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.56",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.56>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
+		-- </sql-errors-1.56>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.57",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.57>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.57>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.58",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.58>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.58>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.59",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.59>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.59>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.60",
+	[[
+		SELECT (1,2,3) = (1,2,3);
+	]], {
+		-- <sql-errors-1.60>
+		0, {true}
+		-- </sql-errors-1.60>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.61",
+	[[
+		SELECT (1,2) = (1,2,3);
+	]], {
+		-- <sql-errors-1.61>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.61>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.62",
+	[[
+		SELECT (SELECT (1), (2)) = (1,2,3);
+	]], {
+		-- <sql-errors-1.62>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.62>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.63",
+	[[
+		SELECT (VALUES (1,2)) = (1,2,3);
+	]], {
+		-- <sql-errors-1.63>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.63>
+	})
+
 test:finish_test()
diff --git a/test/sql-tap/subselect.test.lua b/test/sql-tap/subselect.test.lua
index 5b71390..2cbceb8 100755
--- a/test/sql-tap/subselect.test.lua
+++ b/test/sql-tap/subselect.test.lua
@@ -49,7 +49,7 @@ test:do_catchsql_test(
         SELECT * FROM t1 WHERE a = (SELECT * FROM t1)
     ]], {
         -- <subselect-1.2>
-        1, "row value misused"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </subselect-1.2>
     })
 

[tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[n.pettik]

[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]


>>> /*
>>> * !IMPORTANT! Please follow instructions at start of the file
>>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
>>> index ba7eea5..8cf4d2e 100644
>>> --- a/src/box/sql/expr.c
>>> +++ b/src/box/sql/expr.c
>>> @@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
>>> * Expr pIn is an IN(...) expression. This function checks that the
>>> * sub-select on the RHS of the IN() operator has the same number of
>>> * columns as the vector on the LHS. Or, if the RHS of the IN() is not
>>> - * a sub-query, that the LHS is a vector of size 1.
>>> + * a sub-query, that the LHS must be a vector of size 1.
>> 
>> What is the point of changing this comment?
>> 
> In SQlite it wasn't possible to write something like
> "SELECT (1,2) in ..." so if length of the vector were more
> than 1 it means it was received using subselect. It isn't
> so in our case since we can work with such statements.
> To emphasize this, I changed the comment.

‘… is a vector…’ -> ‘… must be a vector…'

In context of comment they mean the same ->
meaningless change. 

> New patch:
> 
> From 32b665f9ad9dacc16a492c55396dd843f9a355e9 Mon Sep 17 00:00:00 2001
> Date: Sat, 18 May 2019 13:15:58 +0300
> Subject: [PATCH] sql: rework SQL errors of type "expected column count"
> 
> Before this patch, errors of type "expected column count" were
> divided into several different errors, and they all had an
> ER_SQL_PARSER_GENERIC error code. This patch creates a new error
> code for these errors.
> 
> In addition, at some point it became possible to use vectors as
> the left value in the IN operator. Since this was previously
> impossible, it led to a segmentation error.

This doesn’t seem to be decent description, at least to me.
Please, provide description of problem in details: since it
was assertion fault I guess it deserves to be explained.

Let me help you:

In SQL it is allowed to use vector expressions, i.e.
* brief description of vector expressions *.
For instance, * example of using vector expressions *
Accidentally, routines handling IN operator and vector
expressions contained bug. * Bug description *
Let’s fix it in the following way. * Fix description *



[-- Attachment #2: Type: text/html, Size: 15963 bytes --]

[tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[Mergen Imeev]

On Thu, Jun 06, 2019 at 10:22:49PM +0300, n.pettik wrote:
> 
> >>> /*
> >>> * !IMPORTANT! Please follow instructions at start of the file
> >>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> >>> index ba7eea5..8cf4d2e 100644
> >>> --- a/src/box/sql/expr.c
> >>> +++ b/src/box/sql/expr.c
> >>> @@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
> >>> * Expr pIn is an IN(...) expression. This function checks that the
> >>> * sub-select on the RHS of the IN() operator has the same number of
> >>> * columns as the vector on the LHS. Or, if the RHS of the IN() is not
> >>> - * a sub-query, that the LHS is a vector of size 1.
> >>> + * a sub-query, that the LHS must be a vector of size 1.
> >> 
> >> What is the point of changing this comment?
> >> 
> > In SQlite it wasn't possible to write something like
> > "SELECT (1,2) in ..." so if length of the vector were more
> > than 1 it means it was received using subselect. It isn't
> > so in our case since we can work with such statements.
> > To emphasize this, I changed the comment.
> 
> ‘… is a vector…’ -> ‘… must be a vector…'
> 
> In context of comment they mean the same ->
> meaningless change. 
> 
Fixed.

> > New patch:
> > 
> > From 32b665f9ad9dacc16a492c55396dd843f9a355e9 Mon Sep 17 00:00:00 2001
> > Date: Sat, 18 May 2019 13:15:58 +0300
> > Subject: [PATCH] sql: rework SQL errors of type "expected column count"
> > 
> > Before this patch, errors of type "expected column count" were
> > divided into several different errors, and they all had an
> > ER_SQL_PARSER_GENERIC error code. This patch creates a new error
> > code for these errors.
> > 
> > In addition, at some point it became possible to use vectors as
> > the left value in the IN operator. Since this was previously
> > impossible, it led to a segmentation error.
> 
> This doesn’t seem to be decent description, at least to me.
> Please, provide description of problem in details: since it
> was assertion fault I guess it deserves to be explained.
> 
> Let me help you:
> 
> In SQL it is allowed to use vector expressions, i.e.
> * brief description of vector expressions *.
> For instance, * example of using vector expressions *
> Accidentally, routines handling IN operator and vector
> expressions contained bug. * Bug description *
> Let’s fix it in the following way. * Fix description *
> 
> 
Fixed:

sql: allow to use vectors as left value of IN operator

In SQL, it is allowed to use vector expressions, that is, an
operation that uses vectors as operands. For instance, vector
comparison:
SELECT (1,2,3) < (1,2,4);

Accidentally, routines handling IN operator contained a bug: in
cases where we used a vector as the left value in the IN operator,
we received an assertion or a segmentation error.
Let's fix this by allowing vectors as the left value in the IN
operator and using additional checks.

Closes #4204

[tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[Mergen Imeev]

Fixed commit-message.

On Tue, Jun 11, 2019 at 11:40:25AM +0300, Mergen Imeev wrote:
> On Thu, Jun 06, 2019 at 10:22:49PM +0300, n.pettik wrote:
> > 
> > >>> /*
> > >>> * !IMPORTANT! Please follow instructions at start of the file
> > >>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> > >>> index ba7eea5..8cf4d2e 100644
> > >>> --- a/src/box/sql/expr.c
> > >>> +++ b/src/box/sql/expr.c
> > >>> @@ -2958,28 +2958,22 @@ sqlCodeSubselect(Parse * pParse,	/* Parsing context */
> > >>> * Expr pIn is an IN(...) expression. This function checks that the
> > >>> * sub-select on the RHS of the IN() operator has the same number of
> > >>> * columns as the vector on the LHS. Or, if the RHS of the IN() is not
> > >>> - * a sub-query, that the LHS is a vector of size 1.
> > >>> + * a sub-query, that the LHS must be a vector of size 1.
> > >> 
> > >> What is the point of changing this comment?
> > >> 
> > > In SQlite it wasn't possible to write something like
> > > "SELECT (1,2) in ..." so if length of the vector were more
> > > than 1 it means it was received using subselect. It isn't
> > > so in our case since we can work with such statements.
> > > To emphasize this, I changed the comment.
> > 
> > ‘… is a vector…’ -> ‘… must be a vector…'
> > 
> > In context of comment they mean the same ->
> > meaningless change. 
> > 
> Fixed.
> 
> > > New patch:
> > > 
> > > From 32b665f9ad9dacc16a492c55396dd843f9a355e9 Mon Sep 17 00:00:00 2001
> > > Date: Sat, 18 May 2019 13:15:58 +0300
> > > Subject: [PATCH] sql: rework SQL errors of type "expected column count"
> > > 
> > > Before this patch, errors of type "expected column count" were
> > > divided into several different errors, and they all had an
> > > ER_SQL_PARSER_GENERIC error code. This patch creates a new error
> > > code for these errors.
> > > 
> > > In addition, at some point it became possible to use vectors as
> > > the left value in the IN operator. Since this was previously
> > > impossible, it led to a segmentation error.
> > 
> > This doesn’t seem to be decent description, at least to me.
> > Please, provide description of problem in details: since it
> > was assertion fault I guess it deserves to be explained.
> > 
> > Let me help you:
> > 
> > In SQL it is allowed to use vector expressions, i.e.
> > * brief description of vector expressions *.
> > For instance, * example of using vector expressions *
> > Accidentally, routines handling IN operator and vector
> > expressions contained bug. * Bug description *
> > Let’s fix it in the following way. * Fix description *
> > 
> > 
> Fixed:
> 
> sql: allow to use vectors as left value of IN operator
> 
> In SQL, it is allowed to use vector expressions, that is, an
> operation that uses vectors as operands. For instance, vector
> comparison:
> SELECT (1,2,3) < (1,2,4);
> 
> Accidentally, routines handling IN operator contained a bug: in
> cases where we used a vector as the left value in the IN operator,
> we received an assertion or a segmentation error.
> Let's fix this by allowing vectors as the left value in the IN
> operator and using additional checks.
> 
> Closes #4204
> 

New patch:

From 98589eacdc8caf5a9db366d782338d5c3e551357 Mon Sep 17 00:00:00 2001
Date: Sat, 18 May 2019 13:15:58 +0300
Subject: [PATCH] sql: allow to use vectors as left value of IN operator

In SQL, it is allowed to use vector expressions, that is, an
operation that uses vectors as operands. For instance, vector
comparison:
SELECT (1,2,3) < (1,2,4);

Accidentally, routines handling IN operator contained a bug: in
cases where we used a vector as the left value in the IN operator,
we received an assertion in debug build or a segmentation fault in
release. This was due to some legacy code in which it was assumed
that the left value of the IN operator can have only one column in
case it is a vector. Let's fix this by allowing vectors of the
other sizes as the left value of the IN operator and providing
check which verifies that both sides of IN operator have the same
dimension.

Closes #4204

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 55299b7..be8dab2 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -249,6 +249,7 @@ struct errcode_record {
 	/*194 */_(ER_MULTIKEY_INDEX_MISMATCH,	"Field %s is used as multikey in one index and as single key in another") \
 	/*195 */_(ER_CREATE_CK_CONSTRAINT,	"Failed to create check constraint '%s': %s") \
 	/*196 */_(ER_CK_CONSTRAINT_FAILED,	"Check constraint failed '%s': %s") \
+	/*197 */_(ER_SQL_COLUMN_COUNT,		"Unequal number of entries in row expression: left side has %u, but right side - %u") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 898d16c..d7104d8 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2945,22 +2945,16 @@ int
 sqlExprCheckIN(Parse * pParse, Expr * pIn)
 {
 	int nVector = sqlExprVectorSize(pIn->pLeft);
-	const char *err;
 	if ((pIn->flags & EP_xIsSelect)) {
 		if (nVector != pIn->x.pSelect->pEList->nExpr) {
-			err = "sub-select returns %d columns - expected %d";
 			int expr_count = pIn->x.pSelect->pEList->nExpr;
-			diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-				 tt_sprintf(err, expr_count, nVector));
+			diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector,
+				 expr_count);
 			pParse->is_aborted = true;
 			return 1;
 		}
 	} else if (nVector != 1) {
-		assert((pIn->pLeft->flags & EP_xIsSelect) != 0);
-		int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr;
-		err = tt_sprintf("sub-select returns %d columns - expected 1",
-				 expr_count);
-		diag_set(ClientError, ER_SQL_PARSER_GENERIC, err);
+		diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector, 1);
 		pParse->is_aborted = true;
 		return 1;
 	}
@@ -4111,10 +4105,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			testcase(op == TK_SELECT);
 			if (op == TK_SELECT
 			    && (nCol = pExpr->x.pSelect->pEList->nExpr) != 1) {
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, nCol));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nCol, 1);
 				pParse->is_aborted = true;
 			} else {
 				return sqlCodeSubselect(pParse, pExpr, 0);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index fdf3703..0b90edd 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -776,15 +776,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 				nRight = sqlExprVectorSize(pExpr->pRight);
 			}
 			if (nLeft != nRight) {
-				testcase(pExpr->op == TK_EQ);
-				testcase(pExpr->op == TK_NE);
-				testcase(pExpr->op == TK_LT);
-				testcase(pExpr->op == TK_LE);
-				testcase(pExpr->op == TK_GT);
-				testcase(pExpr->op == TK_GE);
-				testcase(pExpr->op == TK_BETWEEN);
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 "row value misused");
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nLeft, nRight);
 				pParse->is_aborted = true;
 			}
 			break;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index a257e72..7c8da25 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -3582,12 +3582,10 @@ substExpr(Parse * pParse,	/* Report errors here */
 			assert(pExpr->pLeft == 0 && pExpr->pRight == 0);
 			if (sqlExprIsVector(pCopy)) {
 				assert((pCopy->flags & EP_xIsSelect) != 0);
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
 				int expr_count =
 					pCopy->x.pSelect->pEList->nExpr;
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, expr_count));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 expr_count, 1);
 				pParse->is_aborted = true;
 			} else {
 				pNew = sqlExprDup(db, pCopy, 0);
diff --git a/test/box/misc.result b/test/box/misc.result
index ec2c4fa..6e1c70d 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -524,6 +524,7 @@ t;
   194: box.error.MULTIKEY_INDEX_MISMATCH
   195: box.error.CREATE_CK_CONSTRAINT
   196: box.error.CK_CONSTRAINT_FAILED
+  197: box.error.SQL_COLUMN_COUNT
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql-tap/in1.test.lua b/test/sql-tap/in1.test.lua
index 76112cf..bc0dfb8 100755
--- a/test/sql-tap/in1.test.lua
+++ b/test/sql-tap/in1.test.lua
@@ -594,7 +594,7 @@ test:do_catchsql_test(
         SELECT b FROM t1 WHERE a NOT IN tb;
     ]], {
         -- <in-9.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-9.4>
     })
 
@@ -677,7 +677,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.2>
     })
 
@@ -689,7 +689,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.3>
     })
 
@@ -701,7 +701,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.4>
     })
 
@@ -713,7 +713,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.5>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.5>
     })
 
@@ -823,7 +823,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.14>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.14>
     })
 
@@ -1058,7 +1058,7 @@ test:do_catchsql_test(
         SELECT 0 WHERE (SELECT 0,0) OR (0 IN (1,2));
     ]], {
         -- <in-13.15>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
         -- </in-13.15>
     })
 
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index f2a071d..3b36d57 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -130,7 +130,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2);
     ]], {
         -- <select7-5.1>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.1>
     })
 
@@ -140,7 +140,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2);
     ]], {
         -- <select7-5.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.2>
     })
 
@@ -150,7 +150,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2 UNION SELECT b,a FROM t2);
     ]], {
         -- <select7-5.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.3>
     })
 
@@ -160,7 +160,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2 UNION SELECT * FROM t2);
     ]], {
         -- <select7-5.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.4>
     })
 
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index a8d3947..6362421 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(47)
+test:plan(63)
 
 test:execsql([[
 	CREATE TABLE t0 (i INT PRIMARY KEY, a INT);
@@ -462,7 +462,7 @@ test:do_catchsql_test(
 		SELECT (1,2,3) == (1,2,3,4);
 	]], {
 		-- <sql-errors-1.41>
-		1,"row value misused"
+		1,"Unequal number of entries in row expression: left side has 3, but right side - 4"
 		-- </sql-errors-1.41>
 	})
 
@@ -526,4 +526,164 @@ test:do_catchsql_test(
 		-- </sql-errors-1.47>
 	})
 
+test:do_catchsql_test(
+	"sql-errors-1.48",
+	[[
+		SELECT (1,2) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.48>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
+		-- </sql-errors-1.48>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.49",
+	[[
+		SELECT (1,2) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.49>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.49>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.50",
+	[[
+		SELECT (1,2) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.50>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.50>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.51",
+	[[
+		SELECT (1,2) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.51>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.51>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.52",
+	[[
+		SELECT (1) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.52>
+		0, {true}
+		-- </sql-errors-1.52>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.53",
+	[[
+		SELECT (1,2,3) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.53>
+		0, {true}
+		-- </sql-errors-1.53>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.54",
+	[[
+		SELECT (1,2,3) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.54>
+		0, {true}
+		-- </sql-errors-1.54>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.55",
+	[[
+		SELECT (1,2,3) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.55>
+		0, {true}
+		-- </sql-errors-1.55>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.56",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (1,2,3);
+	]], {
+		-- <sql-errors-1.56>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
+		-- </sql-errors-1.56>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.57",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT (1), (2), (3));
+	]], {
+		-- <sql-errors-1.57>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.57>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.58",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (VALUES (1,2,3), (2,3,4));
+	]], {
+		-- <sql-errors-1.58>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.58>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.59",
+	[[
+		SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+	]], {
+		-- <sql-errors-1.59>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.59>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.60",
+	[[
+		SELECT (1,2,3) = (1,2,3);
+	]], {
+		-- <sql-errors-1.60>
+		0, {true}
+		-- </sql-errors-1.60>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.61",
+	[[
+		SELECT (1,2) = (1,2,3);
+	]], {
+		-- <sql-errors-1.61>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.61>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.62",
+	[[
+		SELECT (SELECT (1), (2)) = (1,2,3);
+	]], {
+		-- <sql-errors-1.62>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.62>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-1.63",
+	[[
+		SELECT (VALUES (1,2)) = (1,2,3);
+	]], {
+		-- <sql-errors-1.63>
+		1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+		-- </sql-errors-1.63>
+	})
+
 test:finish_test()
diff --git a/test/sql-tap/subselect.test.lua b/test/sql-tap/subselect.test.lua
index ebfdf43..3cf87a1 100755
--- a/test/sql-tap/subselect.test.lua
+++ b/test/sql-tap/subselect.test.lua
@@ -49,7 +49,7 @@ test:do_catchsql_test(
         SELECT * FROM t1 WHERE a = (SELECT * FROM t1)
     ]], {
         -- <subselect-1.2>
-        1, "row value misused"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </subselect-1.2>
     })
 
 

[tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[n.pettik]

Lgtm.

Re: [tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[Vladimir Davydov]

On Tue, Jun 25, 2019 at 05:19:05PM +0300, Mergen Imeev wrote:
> From 98589eacdc8caf5a9db366d782338d5c3e551357 Mon Sep 17 00:00:00 2001
> Date: Sat, 18 May 2019 13:15:58 +0300
> Subject: [PATCH] sql: allow to use vectors as left value of IN operator
> 
> In SQL, it is allowed to use vector expressions, that is, an
> operation that uses vectors as operands. For instance, vector
> comparison:
> SELECT (1,2,3) < (1,2,4);
> 
> Accidentally, routines handling IN operator contained a bug: in
> cases where we used a vector as the left value in the IN operator,
> we received an assertion in debug build or a segmentation fault in
> release. This was due to some legacy code in which it was assumed
> that the left value of the IN operator can have only one column in
> case it is a vector. Let's fix this by allowing vectors of the
> other sizes as the left value of the IN operator and providing
> check which verifies that both sides of IN operator have the same
> dimension.
> 
> Closes #4204

Pushed to master. I failed to backport this to 2.1 - sql-tap/sql-errors
test doesn't pass. Please either backport the patch by yourself or let
me know which commits should be cherry-picked beside this one.

Re: [tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[Mergen Imeev]

Thank you! I rewritten original patch a bit:
https://github.com/tarantool/tarantool/tree/imeevma/gh-4204-rework-column-count-errors

I'm not sure what I did right, since I changed the patch that was
already pushed and cherry-picked it. I can easily rewrite it again
if necessary. New patch below.

On Fri, Jun 28, 2019 at 07:15:26PM +0300, Vladimir Davydov wrote:
> On Tue, Jun 25, 2019 at 05:19:05PM +0300, Mergen Imeev wrote:
> > From 98589eacdc8caf5a9db366d782338d5c3e551357 Mon Sep 17 00:00:00 2001
> > Date: Sat, 18 May 2019 13:15:58 +0300
> > Subject: [PATCH] sql: allow to use vectors as left value of IN operator
> > 
> > In SQL, it is allowed to use vector expressions, that is, an
> > operation that uses vectors as operands. For instance, vector
> > comparison:
> > SELECT (1,2,3) < (1,2,4);
> > 
> > Accidentally, routines handling IN operator contained a bug: in
> > cases where we used a vector as the left value in the IN operator,
> > we received an assertion in debug build or a segmentation fault in
> > release. This was due to some legacy code in which it was assumed
> > that the left value of the IN operator can have only one column in
> > case it is a vector. Let's fix this by allowing vectors of the
> > other sizes as the left value of the IN operator and providing
> > check which verifies that both sides of IN operator have the same
> > dimension.
> > 
> > Closes #4204
> 
> Pushed to master. I failed to backport this to 2.1 - sql-tap/sql-errors
> test doesn't pass. Please either backport the patch by yourself or let
> me know which commits should be cherry-picked beside this one.


New patch:

From 84272dc7eeee704d5bd3c786b464383fa500da02 Mon Sep 17 00:00:00 2001
Date: Sat, 18 May 2019 13:15:58 +0300
Subject: [PATCH] sql: allow to use vectors as left value of IN operator

In SQL, it is allowed to use vector expressions, that is, an
operation that uses vectors as operands. For instance, vector
comparison:
SELECT (1,2,3) < (1,2,4);

Accidentally, routines handling IN operator contained a bug: in
cases where we used a vector as the left value in the IN operator,
we received an assertion in debug build or a segmentation fault in
release. This was due to some legacy code in which it was assumed
that the left value of the IN operator can have only one column in
case it is a vector. Let's fix this by allowing vectors of the
other sizes as the left value of the IN operator and providing
check which verifies that both sides of IN operator have the same
dimension.

Closes #4204

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 3f8cb8e..c9af7dc 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -246,6 +246,7 @@ struct errcode_record {
 	/*191 */_(ER_SQL_PARSER_LIMIT,		"%s %d exceeds the limit (%d)") \
 	/*192 */_(ER_INDEX_DEF_UNSUPPORTED,	"%s are prohibited in an index definition") \
 	/*193 */_(ER_CK_DEF_UNSUPPORTED,	"%s are prohibited in a CHECK constraint definition") \
+	/*194 */_(ER_SQL_COLUMN_COUNT,		"Unequal number of entries in row expression: left side has %u, but right side - %u") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 3ee9a99..5e76a1b 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2968,22 +2968,16 @@ int
 sqlExprCheckIN(Parse * pParse, Expr * pIn)
 {
 	int nVector = sqlExprVectorSize(pIn->pLeft);
-	const char *err;
 	if ((pIn->flags & EP_xIsSelect)) {
 		if (nVector != pIn->x.pSelect->pEList->nExpr) {
-			err = "sub-select returns %d columns - expected %d";
 			int expr_count = pIn->x.pSelect->pEList->nExpr;
-			diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-				 tt_sprintf(err, expr_count, nVector));
+			diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector,
+				 expr_count);
 			pParse->is_aborted = true;
 			return 1;
 		}
 	} else if (nVector != 1) {
-		assert((pIn->pLeft->flags & EP_xIsSelect) != 0);
-		int expr_count = pIn->pLeft->x.pSelect->pEList->nExpr;
-		err = tt_sprintf("sub-select returns %d columns - expected 1",
-				 expr_count);
-		diag_set(ClientError, ER_SQL_PARSER_GENERIC, err);
+		diag_set(ClientError, ER_SQL_COLUMN_COUNT, nVector, 1);
 		pParse->is_aborted = true;
 		return 1;
 	}
@@ -4169,10 +4163,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			testcase(op == TK_SELECT);
 			if (op == TK_SELECT
 			    && (nCol = pExpr->x.pSelect->pEList->nExpr) != 1) {
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, nCol));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nCol, 1);
 				pParse->is_aborted = true;
 			} else {
 				return sqlCodeSubselect(pParse, pExpr, 0);
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 789e92f..28cc84e 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -795,15 +795,8 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 				nRight = sqlExprVectorSize(pExpr->pRight);
 			}
 			if (nLeft != nRight) {
-				testcase(pExpr->op == TK_EQ);
-				testcase(pExpr->op == TK_NE);
-				testcase(pExpr->op == TK_LT);
-				testcase(pExpr->op == TK_LE);
-				testcase(pExpr->op == TK_GT);
-				testcase(pExpr->op == TK_GE);
-				testcase(pExpr->op == TK_BETWEEN);
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 "row value misused");
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 nLeft, nRight);
 				pParse->is_aborted = true;
 			}
 			break;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index b1ec8c7..fc09068 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -3613,12 +3613,10 @@ substExpr(Parse * pParse,	/* Report errors here */
 			assert(pExpr->pLeft == 0 && pExpr->pRight == 0);
 			if (sqlExprIsVector(pCopy)) {
 				assert((pCopy->flags & EP_xIsSelect) != 0);
-				const char *err = "sub-select returns %d "\
-						  "columns - expected 1";
 				int expr_count =
 					pCopy->x.pSelect->pEList->nExpr;
-				diag_set(ClientError, ER_SQL_PARSER_GENERIC,
-					 tt_sprintf(err, expr_count));
+				diag_set(ClientError, ER_SQL_COLUMN_COUNT,
+					 expr_count, 1);
 				pParse->is_aborted = true;
 			} else {
 				pNew = sqlExprDup(db, pCopy, 0);
diff --git a/test/box/misc.result b/test/box/misc.result
index a1f7a09..ebad5bd 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -522,6 +522,7 @@ t;
   191: box.error.SQL_PARSER_LIMIT
   192: box.error.INDEX_DEF_UNSUPPORTED
   193: box.error.CK_DEF_UNSUPPORTED
+  194: box.error.SQL_COLUMN_COUNT
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql-tap/in1.test.lua b/test/sql-tap/in1.test.lua
index 835c10d..67e38a3 100755
--- a/test/sql-tap/in1.test.lua
+++ b/test/sql-tap/in1.test.lua
@@ -594,7 +594,7 @@ test:do_catchsql_test(
         SELECT b FROM t1 WHERE a NOT IN tb;
     ]], {
         -- <in-9.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-9.4>
     })
 
@@ -677,7 +677,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.2>
     })
 
@@ -689,7 +689,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.3>
     })
 
@@ -701,7 +701,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.4>
     })
 
@@ -713,7 +713,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.5>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.5>
     })
 
@@ -823,7 +823,7 @@ test:do_catchsql_test(
         );
     ]], {
         -- <in-12.14>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </in-12.14>
     })
 
@@ -1058,7 +1058,7 @@ test:do_catchsql_test(
         SELECT 0 WHERE (SELECT 0,0) OR (0 IN (1,2));
     ]], {
         -- <in-13.15>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
         -- </in-13.15>
     })
 
diff --git a/test/sql-tap/select2.test.lua b/test/sql-tap/select2.test.lua
index c7f1e5d..74e40f4 100755
--- a/test/sql-tap/select2.test.lua
+++ b/test/sql-tap/select2.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(16)
+test:plan(32)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -269,5 +269,166 @@ test:do_execsql_test(
         -- </select2-4.7>
     })
 
+test:do_catchsql_test(
+    "select2-5.1",
+    [[
+        SELECT (1,2) IN (1,2,3);
+    ]], {
+        -- <select2-5.1>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
+        -- </select2-5.1>
+    })
+
+test:do_catchsql_test(
+    "select2-5.2",
+    [[
+        SELECT (1,2) IN (SELECT (1), (2), (3));
+    ]], {
+        -- <select2-5.2>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.2>
+    })
+
+test:do_catchsql_test(
+    "select2-5.3",
+    [[
+        SELECT (1,2) IN (VALUES (1,2,3), (2,3,4));
+    ]], {
+        -- <select2-5.3>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.3>
+    })
+
+test:do_catchsql_test(
+    "select2-5.4",
+    [[
+        SELECT (1,2) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+    ]], {
+        -- <select2-5.4>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.4>
+    })
+
+test:do_catchsql_test(
+    "select2-5.5",
+    [[
+        SELECT (1) IN (1,2,3);
+    ]], {
+        -- <select2-5.5>
+        0, {1}
+        -- </select2-5.5>
+    })
+
+test:do_catchsql_test(
+    "select2-5.6",
+    [[
+        SELECT (1,2,3) IN (SELECT (1), (2), (3));
+    ]], {
+        -- <select2-5.6>
+        0, {1}
+        -- </select2-5.6>
+    })
+
+test:do_catchsql_test(
+    "select2-5.7",
+    [[
+        SELECT (1,2,3) IN (VALUES (1,2,3), (2,3,4));
+    ]], {
+        -- <select2-5.7>
+        0, {1}
+        -- </select2-5.7>
+    })
+
+test:do_catchsql_test(
+    "select2-5.8",
+    [[
+        SELECT (1,2,3) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+    ]], {
+        -- <select2-5.8>
+        0, {1}
+        -- </select2-5.8>
+    })
+
+test:do_catchsql_test(
+    "select2-5.9",
+    [[
+        SELECT (SELECT * FROM (VALUES (1,2))) IN (1,2,3);
+    ]], {
+        -- <select2-5.9>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 1"
+        -- </select2-5.9>
+    })
+
+test:do_catchsql_test(
+    "select2-5.10",
+    [[
+        SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT (1), (2), (3));
+    ]], {
+        -- <select2-5.10>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.10>
+    })
+
+test:do_catchsql_test(
+    "select2-5.11",
+    [[
+        SELECT (SELECT * FROM (VALUES (1,2))) IN (VALUES (1,2,3), (2,3,4));
+    ]], {
+        -- <select2-5.11>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.11>
+    })
+
+test:do_catchsql_test(
+    "select2-5.12",
+    [[
+        SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT * from (VALUES (1,2,3), (2,3,4)));
+    ]], {
+        -- <select2-5.12>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.12>
+    })
+
+test:do_catchsql_test(
+    "select2-5.13",
+    [[
+        SELECT (1,2,3) = (1,2,3);
+    ]], {
+        -- <select2-5.13>
+        0, {1}
+        -- </select2-5.13>
+    })
+
+test:do_catchsql_test(
+    "select2-5.14",
+    [[
+        SELECT (1,2) = (1,2,3);
+    ]], {
+        -- <select2-5.14>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.14>
+    })
+
+test:do_catchsql_test(
+    "select2-5.15",
+    [[
+        SELECT (SELECT (1), (2)) = (1,2,3);
+    ]], {
+        -- <select2-5.15>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.15>
+    })
+
+test:do_catchsql_test(
+    "select2-5.16",
+    [[
+        SELECT (VALUES (1,2)) = (1,2,3);
+    ]], {
+        -- <select2-5.16>
+        1, "Unequal number of entries in row expression: left side has 2, but right side - 3"
+        -- </select2-5.16>
+    })
+
+
 test:finish_test()
 
diff --git a/test/sql-tap/select7.test.lua b/test/sql-tap/select7.test.lua
index f2a071d..3b36d57 100755
--- a/test/sql-tap/select7.test.lua
+++ b/test/sql-tap/select7.test.lua
@@ -130,7 +130,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2);
     ]], {
         -- <select7-5.1>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.1>
     })
 
@@ -140,7 +140,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2);
     ]], {
         -- <select7-5.2>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.2>
     })
 
@@ -150,7 +150,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT a,b FROM t2 UNION SELECT b,a FROM t2);
     ]], {
         -- <select7-5.3>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.3>
     })
 
@@ -160,7 +160,7 @@ test:do_catchsql_test(
         SELECT 5 IN (SELECT * FROM t2 UNION SELECT * FROM t2);
     ]], {
         -- <select7-5.4>
-        1, "sub-select returns 2 columns - expected 1"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </select7-5.4>
     })
 
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index 4e173b6..9f7853e 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -462,7 +462,7 @@ test:do_catchsql_test(
 		SELECT (1,2,3) == (1,2,3,4);
 	]], {
 		-- <sql-errors-1.41>
-		1,"row value misused"
+		1,"Unequal number of entries in row expression: left side has 3, but right side - 4"
 		-- </sql-errors-1.41>
 	})
 
diff --git a/test/sql-tap/subselect.test.lua b/test/sql-tap/subselect.test.lua
index 5b71390..2cbceb8 100755
--- a/test/sql-tap/subselect.test.lua
+++ b/test/sql-tap/subselect.test.lua
@@ -49,7 +49,7 @@ test:do_catchsql_test(
         SELECT * FROM t1 WHERE a = (SELECT * FROM t1)
     ]], {
         -- <subselect-1.2>
-        1, "row value misused"
+        1, "Unequal number of entries in row expression: left side has 1, but right side - 2"
         -- </subselect-1.2>
     })
 

Re: [tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"

[Vladimir Davydov]

Pushed to 2.1, thanks!