From 4125c4ef33414f023d920be216c957d1b80253e9 Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Tue, 27 Aug 2019 20:17:04 +0300
Subject: [PATCH] sql: make GREATEST/LEAST built-ins accept at least two args
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
---
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 e9990340fd..0c1310bdf5 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -254,6 +254,7 @@ struct errcode_record {
/*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_NO_SUCH_FIELD_NAME, "Field '%s' was not found in the tuple") \
+ /*202 */_(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 1f9d917056..1957bd4c8e 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 07c019db95..0c28cec297 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 204c2671bf..c46c5a9d6a 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -531,6 +531,7 @@ t;
199: box.error.FUNC_INDEX_FORMAT
200: box.error.FUNC_INDEX_PARTS
201: box.error.NO_SUCH_FIELD_NAME
+ 202: 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 0b255e6593..e84ca25be7 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()
--
GitLab