From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B3CF12FB52 for ; Sat, 18 May 2019 06:53:40 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id VnqU4WVq5Nyu for ; Sat, 18 May 2019 06:53:40 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E7FBD2F5CB for ; Sat, 18 May 2019 06:53:39 -0400 (EDT) From: imeevma@tarantool.org Subject: [tarantool-patches] [PATCH v1 1/1] sql: rework SQL errors of type "expected column count" Date: Sat, 18 May 2019 13:53:37 +0300 Message-Id: <461ca6b5a31331d36ef7d23872613f0b1c0de9d0.1558176783.git.imeevma@gmail.com> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: korablev@tarantool.org Cc: tarantool-patches@freelists.org 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; ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -677,7 +677,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -689,7 +689,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -701,7 +701,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -713,7 +713,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -823,7 +823,7 @@ test:do_catchsql_test( ); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -1058,7 +1058,7 @@ test:do_catchsql_test( SELECT 0 WHERE (SELECT 0,0) OR (0 IN (1,2)); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 2 does not match the expected column count (1)" -- }) 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); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -140,7 +140,7 @@ test:do_catchsql_test( SELECT 5 IN (SELECT * FROM t2); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -150,7 +150,7 @@ test:do_catchsql_test( SELECT 5 IN (SELECT a,b FROM t2 UNION SELECT b,a FROM t2); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) @@ -160,7 +160,7 @@ test:do_catchsql_test( SELECT 5 IN (SELECT * FROM t2 UNION SELECT * FROM t2); ]], { -- - 1, "sub-select returns 2 columns - expected 1" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) 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); ]], { -- - 1,"row value misused" + 1,"Left value column count 3 does not match the expected column count (4)" -- }) @@ -526,4 +526,164 @@ test:do_catchsql_test( -- }) +test:do_catchsql_test( + "sql-errors-1.48", + [[ + SELECT (1,2) IN (1,2,3); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (1)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.49", + [[ + SELECT (1,2) IN (SELECT (1), (2), (3)); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.50", + [[ + SELECT (1,2) IN (VALUES (1,2,3), (2,3,4)); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.51", + [[ + SELECT (1,2) IN (SELECT * from (VALUES (1,2,3), (2,3,4))); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.52", + [[ + SELECT (1) IN (1,2,3); + ]], { + -- + 0, {true} + -- + }) + +test:do_catchsql_test( + "sql-errors-1.53", + [[ + SELECT (1,2,3) IN (SELECT (1), (2), (3)); + ]], { + -- + 0, {true} + -- + }) + +test:do_catchsql_test( + "sql-errors-1.54", + [[ + SELECT (1,2,3) IN (VALUES (1,2,3), (2,3,4)); + ]], { + -- + 0, {true} + -- + }) + +test:do_catchsql_test( + "sql-errors-1.55", + [[ + SELECT (1,2,3) IN (SELECT * from (VALUES (1,2,3), (2,3,4))); + ]], { + -- + 0, {true} + -- + }) + +test:do_catchsql_test( + "sql-errors-1.56", + [[ + SELECT (SELECT * FROM (VALUES (1,2))) IN (1,2,3); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (1)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.57", + [[ + SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT (1), (2), (3)); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.58", + [[ + SELECT (SELECT * FROM (VALUES (1,2))) IN (VALUES (1,2,3), (2,3,4)); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.59", + [[ + SELECT (SELECT * FROM (VALUES (1,2))) IN (SELECT * from (VALUES (1,2,3), (2,3,4))); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.60", + [[ + SELECT (1,2,3) = (1,2,3); + ]], { + -- + 0, {true} + -- + }) + +test:do_catchsql_test( + "sql-errors-1.61", + [[ + SELECT (1,2) = (1,2,3); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.62", + [[ + SELECT (SELECT (1), (2)) = (1,2,3); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + +test:do_catchsql_test( + "sql-errors-1.63", + [[ + SELECT (VALUES (1,2)) = (1,2,3); + ]], { + -- + 1, "Left value column count 2 does not match the expected column count (3)" + -- + }) + 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) ]], { -- - 1, "row value misused" + 1, "Left value column count 1 does not match the expected column count (2)" -- }) -- 2.7.4