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 74F27278F8 for ; Wed, 28 Aug 2019 10:55:34 -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 OwctRkwDJXTd for ; Wed, 28 Aug 2019 10:55:34 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 19982278E0 for ; Wed, 28 Aug 2019 10:55:33 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH] sql: make GREATEST/LEAST built-ins accept at least two args Date: Wed, 28 Aug 2019 17:55:25 +0300 Message-Id: <20190828145525.43365-1-korablev@tarantool.org> 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: tarantool-patches@freelists.org Cc: kyukhin@tarantool.org, 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