From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 dev.tarantool.org (Postfix) with ESMTPS id 261D443040F for ; Wed, 12 Aug 2020 18:15:53 +0300 (MSK) From: imeevma@tarantool.org Date: Wed, 12 Aug 2020 18:15:52 +0300 Message-Id: <0dfadabbd47f651890eac5b5c40733d11bb0dd1c.1597244875.git.imeevma@gmail.com> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v1 5/7] sql: check built-in functions argument types List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org This patch creates a uniform way to check the argument types of SQL built-in functions. Prior to this patch, argument types were checked inside functions. They are now checked in most cases in the ApplyType opcode. The only case where there is additional validation in a function is when a function can take either STRING or VARBINARY as an argument. In this case, the definition says that the function accepts a SCALAR, but in fact only STRING or VARBINARY should be accepted. This case will be completely fixed in the next patches of the patch set. Part of #4159 --- src/box/sql/expr.c | 4 ++++ src/box/sql/select.c | 26 ++++++++++++++++++++++++++ src/box/sql/sqlInt.h | 14 ++++++++++++++ test/sql-tap/func.test.lua | 22 +++++++++++----------- test/sql/boolean.result | 2 +- test/sql/checks.result | 8 -------- test/sql/checks.test.lua | 2 -- test/sql/types.result | 2 +- 8 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c index bc2182446..e2c8e1385 100644 --- a/src/box/sql/expr.c +++ b/src/box/sql/expr.c @@ -4120,6 +4120,10 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target) } else { r1 = 0; } + if (func->def->language == FUNC_LANGUAGE_SQL_BUILTIN) { + sql_emit_func_arg_type_check(v, func, r1, + nFarg); + } if (sql_func_flag_is_set(func, SQL_FUNC_NEEDCOLL)) { sqlVdbeAddOp4(v, OP_CollSeq, 0, 0, 0, (char *)coll, P4_COLLSEQ); diff --git a/src/box/sql/select.c b/src/box/sql/select.c index b0554a172..49f01eb0d 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -124,6 +124,31 @@ clearSelect(sql * db, Select * p, int bFree) } } +void +sql_emit_func_arg_type_check(struct Vdbe *vdbe, struct func *func, int reg, + uint32_t argc) +{ + if (argc == 0 || func->def->param_list == NULL) + return; + assert(func->def->param_count > 0); + uint32_t len = (uint32_t)func->def->param_count; + assert(len > 0); + size_t size = (argc + 1) * sizeof(enum field_type); + enum field_type *types = sqlDbMallocZero(sql_get(), size); + if (argc <= len) { + for (uint32_t i = 0; i < argc; ++i) + types[i] = func->def->param_list[i]; + } else { + for (uint32_t i = 0; i < len; ++i) + types[i] = func->def->param_list[i]; + for (uint32_t i = len; i < argc; ++i) + types[i] = func->def->param_list[len - 1]; + } + types[argc] = field_type_MAX; + sqlVdbeAddOp4(vdbe, OP_ApplyType, reg, argc, 0, (char *)types, + P4_DYNAMIC); +} + /* * Initialize a SelectDest structure. */ @@ -5420,6 +5445,7 @@ updateAccumulator(Parse * pParse, AggInfo * pAggInfo) vdbe_insert_distinct(pParse, pF->iDistinct, pF->reg_eph, addrNext, 1, regAgg); } + sql_emit_func_arg_type_check(v, pF->func, regAgg, nArg); if (sql_func_flag_is_set(pF->func, SQL_FUNC_NEEDCOLL)) { struct coll *coll = NULL; struct ExprList_item *pItem; diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h index adf90d824..d02614140 100644 --- a/src/box/sql/sqlInt.h +++ b/src/box/sql/sqlInt.h @@ -3883,6 +3883,20 @@ sql_index_type_str(struct sql *db, const struct index_def *idx_def); void sql_emit_table_types(struct Vdbe *v, struct space_def *def, int reg); +/** + * Code an OP_ApplyType opcode that try to cast implicitly types + * for given range of register starting from @a reg. These values + * then will be used as arguments of a function. + * + * @param vdbe VDBE. + * @param func Definition of the function. + * @param reg Register where types will be placed. + * @param argc Number of arguments. + */ +void +sql_emit_func_arg_type_check(struct Vdbe *vdbe, struct func *func, + int reg, uint32_t argc); + enum field_type sql_type_result(enum field_type lhs, enum field_type rhs); diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua index 3c088920f..82cf350ea 100755 --- a/test/sql-tap/func.test.lua +++ b/test/sql-tap/func.test.lua @@ -412,13 +412,13 @@ test:do_execsql_test( -- }) -test:do_execsql_test( +test:do_catchsql_test( "func-4.4.2", [[ SELECT abs(t1) FROM tbl1 ]], { -- - 0.0, 0.0, 0.0, 0.0, 0.0 + 1, "Type mismatch: can not convert this to number" -- }) @@ -502,13 +502,13 @@ test:do_execsql_test( -- }) -test:do_execsql_test( +test:do_catchsql_test( "func-4.13", [[ SELECT round(t1,2) FROM tbl1 ]], { -- - 0.0, 0.0, 0.0, 0.0, 0.0 + 1, "Type mismatch: can not convert this to double" -- }) @@ -893,7 +893,7 @@ test:do_execsql_test( test:do_execsql_test( "func-8.5", [[ - SELECT sum(x) FROM (SELECT '9223372036' || '854775807' AS x + SELECT sum(x) FROM (SELECT CAST('9223372036' || '854775807' AS INTEGER) AS x UNION ALL SELECT -9223372036854775807) ]], { -- @@ -904,7 +904,7 @@ test:do_execsql_test( test:do_execsql_test( "func-8.6", [[ - SELECT typeof(sum(x)) FROM (SELECT '9223372036' || '854775807' AS x + SELECT typeof(sum(x)) FROM (SELECT CAST('9223372036' || '854775807' AS INTEGER) AS x UNION ALL SELECT -9223372036854775807) ]], { -- @@ -915,7 +915,7 @@ test:do_execsql_test( test:do_execsql_test( "func-8.7", [[ - SELECT typeof(sum(x)) FROM (SELECT '9223372036' || '854775808' AS x + SELECT typeof(sum(x)) FROM (SELECT CAST('9223372036' || '854775808' AS INTEGER) AS x UNION ALL SELECT -9223372036854775807) ]], { -- @@ -926,7 +926,7 @@ test:do_execsql_test( test:do_execsql_test( "func-8.8", [[ - SELECT sum(x)>0.0 FROM (SELECT '9223372036' || '854775808' AS x + SELECT sum(x)>0.0 FROM (SELECT CAST('9223372036' || '854775808' AS INTEGER) AS x UNION ALL SELECT -9223372036850000000) ]], { -- @@ -985,7 +985,7 @@ test:do_execsql_test( test:do_execsql_test( "func-9.5", [[ - SELECT length(randomblob(32)), length(randomblob(-5)), + SELECT length(randomblob(32)), length(randomblob(0)), length(randomblob(2000)) ]], { -- @@ -2918,7 +2918,7 @@ test:do_catchsql_test( SELECT ROUND(X'FF') ]], { -- - 1, "Type mismatch: can not convert varbinary to numeric" + 1, "Type mismatch: can not convert varbinary to double" -- }) @@ -2928,7 +2928,7 @@ test:do_catchsql_test( SELECT RANDOMBLOB(X'FF') ]], { -- - 1, "Type mismatch: can not convert varbinary to numeric" + 1, "Type mismatch: can not convert varbinary to unsigned" -- }) diff --git a/test/sql/boolean.result b/test/sql/boolean.result index 51ec5820b..d366aca7d 100644 --- a/test/sql/boolean.result +++ b/test/sql/boolean.result @@ -276,7 +276,7 @@ SELECT is_boolean('true'); SELECT abs(a) FROM t0; | --- | - null - | - 'Inconsistent types: expected number got boolean' + | - 'Type mismatch: can not convert FALSE to number' | ... SELECT lower(a) FROM t0; | --- diff --git a/test/sql/checks.result b/test/sql/checks.result index 7b18e5d6b..3c942fb23 100644 --- a/test/sql/checks.result +++ b/test/sql/checks.result @@ -519,14 +519,6 @@ s:insert({1, 'string'}) --- - error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer''' ... -s:insert({1, {map=true}}) ---- -- error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer''' -... -s:insert({1, {'a', 'r','r','a','y'}}) ---- -- error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer''' -... s:insert({1, 3.14}) --- - error: 'Check constraint failed ''complex2'': typeof(coalesce(z,0))==''integer''' diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua index 301f8ea69..b55abe955 100644 --- a/test/sql/checks.test.lua +++ b/test/sql/checks.test.lua @@ -173,8 +173,6 @@ s:format({{name='X', type='integer'}, {name='Z', type='any'}}) _ = s:create_index('pk', {parts = {1, 'integer'}}) _ = box.space._ck_constraint:insert({s.id, 'complex2', false, 'SQL', 'typeof(coalesce(z,0))==\'integer\'', true}) s:insert({1, 'string'}) -s:insert({1, {map=true}}) -s:insert({1, {'a', 'r','r','a','y'}}) s:insert({1, 3.14}) s:insert({1, 666}) s:drop() diff --git a/test/sql/types.result b/test/sql/types.result index 2498f3a48..82a82117b 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -1322,7 +1322,7 @@ box.execute("SELECT upper(v) FROM t;") box.execute("SELECT abs(v) FROM t;") --- - null -- 'Inconsistent types: expected number got varbinary' +- 'Type mismatch: can not convert varbinary to number' ... box.execute("SELECT typeof(v) FROM t;") --- -- 2.25.1