* [tarantool-patches] [PATCH] sql: make GREATEST/LEAST built-ins accept at least two args
@ 2019-08-28 14:55 Nikita Pettik
[not found] ` <20190829103847.GF22684@atlas>
0 siblings, 1 reply; 2+ messages in thread
From: Nikita Pettik @ 2019-08-28 14:55 UTC (permalink / raw)
To: tarantool-patches; +Cc: kyukhin, Nikita Pettik
Before a46b52004 SQL implementation featured min()/max() functions
overloading: if one argument was passed, then aggregate version would be
invoked; otherwise - scalar one. We decided to get rid of it and rename
scalar version to LEAST()/GREATEST() correspondingly. However, assertion
inside their implementations has been remained: it verifies that number
of passed arguments is greater than 1. On the other hand, now one can
pass literally any number of arguments to this function, including one
(which results in fired mentioned assertion) and zero (which leads to
NULL dereference in expr.c: these functions are marked with
SQL_FUNC_NEEDCOLL flag, and as a consequence they are assumed to have at
least one argument). Firstly, let's place check that number of passed
arguments more than one. Secondly, let's not assume that functions with
SQL_FUNC_NEEDCOLL must have any arguments.
Closes #4453
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-4453-remove-redundant-assertion
Issue: https://github.com/tarantool/tarantool/issues/4453
src/box/errcode.h | 1 +
src/box/sql/expr.c | 3 ++-
src/box/sql/func.c | 7 ++++++-
test/box/misc.result | 1 +
test/sql-tap/func5.test.lua | 23 ++++++++++++++++++++++-
5 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/src/box/errcode.h b/src/box/errcode.h
index 46b0b365a..484097749 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -253,6 +253,7 @@ struct errcode_record {
/*198 */_(ER_FUNC_INDEX_FUNC, "Failed to build a key for functional index '%s' of space '%s': %s") \
/*199 */_(ER_FUNC_INDEX_FORMAT, "Key format doesn't match one defined in functional index '%s' of space '%s': %s") \
/*200 */_(ER_FUNC_INDEX_PARTS, "Wrong functional index definition: %s") \
+ /*201 */_(ER_FUNC_WRONG_ARG_COUNT, "Wrong number of arguments is passed to %s(): expected %s, got %d") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 1f9d91705..1957bd4c8 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4049,7 +4049,8 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
* is done using ANSI rules from
* collations_check_compatibility().
*/
- if ((pDef->funcFlags & SQL_FUNC_NEEDCOLL) != 0) {
+ if ((pDef->funcFlags & SQL_FUNC_NEEDCOLL) != 0 &&
+ nFarg > 0) {
struct coll *unused = NULL;
uint32_t curr_id = COLL_NONE;
bool is_curr_forced = false;
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 07c019db9..0c28cec29 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -81,8 +81,13 @@ minmaxFunc(sql_context * context, int argc, sql_value ** argv)
int iBest;
struct coll *pColl;
- assert(argc > 1);
mask = sql_user_data(context) == 0 ? 0 : -1;
+ if (argc < 2) {
+ diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT,
+ mask ? "GREATEST" : "LEAST", "at least two", argc);
+ context->is_aborted = true;
+ return;
+ }
pColl = sqlGetFuncCollSeq(context);
assert(mask == -1 || mask == 0);
iBest = 0;
diff --git a/test/box/misc.result b/test/box/misc.result
index 287d84e5b..e4de2e9b3 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -530,6 +530,7 @@ t;
198: box.error.FUNC_INDEX_FUNC
199: box.error.FUNC_INDEX_FORMAT
200: box.error.FUNC_INDEX_PARTS
+ 201: box.error.FUNC_WRONG_ARG_COUNT
...
test_run:cmd("setopt delimiter ''");
---
diff --git a/test/sql-tap/func5.test.lua b/test/sql-tap/func5.test.lua
index 0b255e659..e84ca25be 100755
--- a/test/sql-tap/func5.test.lua
+++ b/test/sql-tap/func5.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(19)
+test:plan(22)
--!./tcltestrunner.lua
-- 2010 August 27
@@ -257,4 +257,25 @@ test:do_execsql_test(
SELECT LEAST(false, 'STR', 1, 0.5);
]], { false } )
+-- gh-4453: GREATEST()/LEAST() require at least two arguments
+-- be passed to these functions.
+--
+test:do_catchsql_test(
+ "func-5-5.1",
+ [[
+ SELECT LEAST(false);
+ ]], { 1, "Wrong number of arguments is passed to LEAST(): expected at least two, got 1" } )
+
+test:do_catchsql_test(
+ "func-5-5.2",
+ [[
+ SELECT GREATEST('abc');
+ ]], { 1, "Wrong number of arguments is passed to GREATEST(): expected at least two, got 1" } )
+
+test:do_catchsql_test(
+ "func-5-5.3",
+ [[
+ SELECT LEAST();
+ ]], { 1, "Wrong number of arguments is passed to LEAST(): expected at least two, got 0" } )
+
test:finish_test()
--
2.15.1
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-08-29 13:27 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28 14:55 [tarantool-patches] [PATCH] sql: make GREATEST/LEAST built-ins accept at least two args Nikita Pettik
[not found] ` <20190829103847.GF22684@atlas>
2019-08-29 13:27 ` [tarantool-patches] " Nikita Pettik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox