From: Mergen Imeev <imeevma@tarantool.org>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org
Subject: Re: [tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"
Date: Sat, 29 Jun 2019 13:33:49 +0300 [thread overview]
Message-ID: <20190629103348.GA4372@tarantool.org> (raw)
In-Reply-To: <20190628161526.dystben65fqeruji@esperanza>
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>
})
next prev parent reply other threads:[~2019-06-29 10:33 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-18 10:53 [tarantool-patches] " imeevma
2019-05-19 14:38 ` [tarantool-patches] " n.pettik
2019-05-25 11:12 ` Mergen Imeev
2019-06-06 19:22 ` n.pettik
2019-06-11 8:40 ` Mergen Imeev
2019-06-25 14:19 ` Mergen Imeev
2019-06-25 17:31 ` n.pettik
2019-06-28 16:15 ` Vladimir Davydov
2019-06-29 10:33 ` Mergen Imeev [this message]
2019-07-03 9:58 ` Vladimir Davydov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190629103348.GA4372@tarantool.org \
--to=imeevma@tarantool.org \
--cc=korablev@tarantool.org \
--cc=tarantool-patches@freelists.org \
--cc=vdavydov.dev@gmail.com \
--subject='Re: [tarantool-patches] Re: [PATCH v1 1/1] sql: rework SQL errors of type "expected column count"' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox