Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions
@ 2020-08-14 15:04 imeevma
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 01/10] sql: do not return UNSIGNED in " imeevma
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: imeevma @ 2020-08-14 15:04 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

This patch-set makes SQL to use the ApplyType opcode to validate the argument
types of built-in functions.

https://github.com/tarantool/tarantool/issues/4159
https://github.com/tarantool/tarantool/tree/imeevma/gh-4159-rework-sql-builtins

@ChangeLog
 - Built-in function argument types are now properly checked (gh-4159).

Changes in v2:
 - Changed an approach to check types of STRING/VARBINARY arguments.
 - Added a new patch that changes signature of trim().

Mergen Imeev (10):
  sql: do not return UNSIGNED in built-in functions
  sql: fix functions return types
  sql: change signature of trim()
  box: add new options for functions
  sql: use has_vararg for built-in functions
  sql: add overloaded versions of the functions
  sql: move built-in function definitions in _func
  box: add param_list to 'struct func'
  sql: check built-in functions argument types
  sql: refactor sql/func.c

 src/box/alter.cc               |   12 +-
 src/box/bootstrap.snap         |  Bin 5976 -> 6446 bytes
 src/box/func.c                 |    1 +
 src/box/func_def.c             |   13 +
 src/box/func_def.h             |   20 +
 src/box/lua/call.c             |    2 +
 src/box/lua/upgrade.lua        |  192 ++++
 src/box/sql/analyze.c          |    6 +-
 src/box/sql/expr.c             |   33 +-
 src/box/sql/func.c             |  879 +++---------------
 src/box/sql/parse.y            |   35 +-
 src/box/sql/resolve.c          |    2 +-
 src/box/sql/select.c           |   26 +
 src/box/sql/sqlInt.h           |   19 +-
 src/box/sql/vdbeapi.c          |    2 +-
 src/box/sql/vdbemem.c          |    2 +-
 test/box-py/bootstrap.result   |    6 +-
 test/box/access.result         |    2 +-
 test/box/access.test.lua       |    2 +-
 test/box/access_bin.result     |    2 +-
 test/box/access_bin.test.lua   |    2 +-
 test/box/access_sysview.result |    8 +-
 test/box/function1.result      |    6 +-
 test/sql-tap/cse.test.lua      |   24 +-
 test/sql-tap/func.test.lua     |   46 +-
 test/sql-tap/orderby1.test.lua |    2 +-
 test/sql-tap/position.test.lua |   16 +-
 test/sql-tap/substr.test.lua   |    2 +-
 test/sql/boolean.result        |   32 +-
 test/sql/checks.result         |    8 -
 test/sql/checks.test.lua       |    2 -
 test/sql/collation.result      |    8 +
 test/sql/collation.test.lua    |    1 +
 test/sql/types.result          | 1547 +++++++++++++++++++++++++++++++-
 test/sql/types.test.lua        |  264 ++++++
 test/wal_off/func_max.result   |    8 +-
 36 files changed, 2290 insertions(+), 942 deletions(-)

-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 01/10] sql: do not return UNSIGNED in built-in functions
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
@ 2020-08-14 15:04 ` imeevma
  2020-08-22 14:23   ` Vladislav Shpilevoy
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 02/10] sql: fix functions return types imeevma
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:04 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

This patch forces functions to return INTEGER instead of UNSIGNED.
---
 src/box/sql/vdbeapi.c   |  2 +-
 test/sql/types.result   | 12 ++++++++++++
 test/sql/types.test.lua |  6 ++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 7c59ef83f..d1eeaf114 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -328,7 +328,7 @@ sql_result_double(sql_context * pCtx, double rVal)
 void
 sql_result_uint(sql_context *ctx, uint64_t u_val)
 {
-	mem_set_u64(ctx->pOut, u_val);
+	mem_set_int(ctx->pOut, u_val, false);
 }
 
 void
diff --git a/test/sql/types.result b/test/sql/types.result
index 442245186..95f7713e8 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -2795,3 +2795,15 @@ box.execute([[DROP TABLE ts;]])
 ---
 - row_count: 1
 ...
+--
+-- gh-4159: Make sure that functions returns values of type INTEGER
+-- instead of values of type UNSIGNED.
+--
+box.execute([[SELECT typeof(length('abc'));]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['integer']
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index 0270d9f8a..fff0057bd 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -623,3 +623,9 @@ box.execute([[DROP TABLE tb;]])
 box.execute([[DROP TABLE tt;]])
 box.execute([[DROP TABLE tv;]])
 box.execute([[DROP TABLE ts;]])
+
+--
+-- gh-4159: Make sure that functions returns values of type INTEGER
+-- instead of values of type UNSIGNED.
+--
+box.execute([[SELECT typeof(length('abc'));]])
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 02/10] sql: fix functions return types
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 01/10] sql: do not return UNSIGNED in " imeevma
@ 2020-08-14 15:04 ` imeevma
  2020-08-22 14:24   ` Vladislav Shpilevoy
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 03/10] sql: change signature of trim() imeevma
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:04 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

This patch fixes incorrect return types in SQL built-in function definitions.
---
 src/box/sql/func.c    | 10 +++++-----
 test/sql/types.result |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 487cdafe1..affb285aa 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2466,7 +2466,7 @@ static struct {
 	}, {
 	 .name = "IFNULL",
 	 .param_count = 2,
-	 .returns = FIELD_TYPE_INTEGER,
+	 .returns = FIELD_TYPE_SCALAR,
 	 .aggregate = FUNC_AGGREGATE_NONE,
 	 .is_deterministic = true,
 	 .flags = SQL_FUNC_COALESCE,
@@ -2516,7 +2516,7 @@ static struct {
 	}, {
 	 .name = "LIKE",
 	 .param_count = -1,
-	 .returns = FIELD_TYPE_INTEGER,
+	 .returns = FIELD_TYPE_BOOLEAN,
 	 .aggregate = FUNC_AGGREGATE_NONE,
 	 .is_deterministic = true,
 	 .flags = SQL_FUNC_NEEDCOLL | SQL_FUNC_LIKE,
@@ -2526,7 +2526,7 @@ static struct {
 	}, {
 	 .name = "LIKELIHOOD",
 	 .param_count = 2,
-	 .returns = FIELD_TYPE_BOOLEAN,
+	 .returns = FIELD_TYPE_SCALAR,
 	 .aggregate = FUNC_AGGREGATE_NONE,
 	 .is_deterministic = true,
 	 .flags = SQL_FUNC_UNLIKELY,
@@ -2536,7 +2536,7 @@ static struct {
 	}, {
 	 .name = "LIKELY",
 	 .param_count = 1,
-	 .returns = FIELD_TYPE_BOOLEAN,
+	 .returns = FIELD_TYPE_SCALAR,
 	 .aggregate = FUNC_AGGREGATE_NONE,
 	 .is_deterministic = true,
 	 .flags = SQL_FUNC_UNLIKELY,
@@ -2686,7 +2686,7 @@ static struct {
 	}, {
 	 .name = "ROUND",
 	 .param_count = -1,
-	 .returns = FIELD_TYPE_INTEGER,
+	 .returns = FIELD_TYPE_DOUBLE,
 	 .aggregate = FUNC_AGGREGATE_NONE,
 	 .is_deterministic = true,
 	 .flags = 0,
diff --git a/test/sql/types.result b/test/sql/types.result
index 95f7713e8..2498f3a48 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -231,7 +231,7 @@ box.execute("SELECT s LIKE NULL FROM t1;")
 ---
 - metadata:
   - name: COLUMN_1
-    type: integer
+    type: boolean
   rows:
   - [null]
 ...
@@ -257,7 +257,7 @@ box.execute("SELECT NULL LIKE s FROM t1;")
 ---
 - metadata:
   - name: COLUMN_1
-    type: integer
+    type: boolean
   rows:
   - [null]
 ...
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 03/10] sql: change signature of trim()
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 01/10] sql: do not return UNSIGNED in " imeevma
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 02/10] sql: fix functions return types imeevma
@ 2020-08-14 15:04 ` imeevma
  2020-08-22 14:26   ` Vladislav Shpilevoy
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 04/10] box: add new options for functions imeevma
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:04 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

This patch changes the signature of the SQL built-in trim() function.
This makes it easier to define a function in _func and fixes a bug where
the function loses collation when the BOTH, LEADING, or TRAILING
keywords are specified.
---
 src/box/sql/func.c          | 56 +++++++++++++++++--------------------
 src/box/sql/parse.y         | 35 +++++++++++------------
 test/sql/collation.result   |  8 ++++++
 test/sql/collation.test.lua |  1 +
 4 files changed, 51 insertions(+), 49 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index affb285aa..e5da21191 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1764,10 +1764,6 @@ trim_func_one_arg(struct sql_context *context, sql_value *arg)
 /**
  * Normalize args from @a argv input array when it has two args.
  *
- * Case: TRIM(<character_set> FROM <str>)
- * If user has specified <character_set> only, call trimming
- * procedure with TRIM_BOTH as the flags and that trimming set.
- *
  * Case: TRIM(LEADING/TRAILING/BOTH FROM <str>)
  * If user has specified side keyword only, then call trimming
  * procedure with the specified side and " " as the trimming set.
@@ -1776,32 +1772,30 @@ static void
 trim_func_two_args(struct sql_context *context, sql_value *arg1,
 		   sql_value *arg2)
 {
-	const unsigned char *input_str, *trim_set;
-	if ((input_str = sql_value_text(arg2)) == NULL)
-		return;
-
-	int input_str_sz = sql_value_bytes(arg2);
-	if (sql_value_type(arg1) == MP_INT || sql_value_type(arg1) == MP_UINT) {
-		uint8_t len_one = 1;
-		trim_procedure(context, sql_value_int(arg1),
-			       (const unsigned char *) " ", &len_one, 1,
-			       input_str, input_str_sz);
-	} else if ((trim_set = sql_value_text(arg1)) != NULL) {
-		int trim_set_sz = sql_value_bytes(arg1);
-		uint8_t *char_len;
-		int char_cnt = trim_prepare_char_len(context, trim_set,
-						     trim_set_sz, &char_len);
-		if (char_cnt == -1)
-			return;
-		trim_procedure(context, TRIM_BOTH, trim_set, char_len, char_cnt,
-			       input_str, input_str_sz);
-		sql_free(char_len);
-	}
+	assert(sql_value_type(arg2) == MP_UINT);
+	enum mp_type type = sql_value_type(arg1);
+	if (type == MP_NIL)
+		return sql_result_null(context);
+	const unsigned char *input_str = sql_value_text(arg1);
+	const unsigned char *trim_set;
+
+	int input_str_sz = sql_value_bytes(arg1);
+	uint8_t len_one = 1;
+	if (type == MP_BIN)
+		trim_set = (const unsigned char *) "\0";
+	else
+		trim_set = (const unsigned char *) " ";
+	trim_procedure(context, sql_value_int(arg2), trim_set,  &len_one, 1,
+		       input_str, input_str_sz);
 }
 
 /**
  * Normalize args from @a argv input array when it has three args.
  *
+ * Case: TRIM(<character_set> FROM <str>)
+ * If user has specified <character_set> only, call trimming
+ * procedure with TRIM_BOTH as the flags and that trimming set.
+ *
  * Case: TRIM(LEADING/TRAILING/BOTH <character_set> FROM <str>)
  * If user has specified side keyword and <character_set>, then
  * call trimming procedure with that args.
@@ -1810,20 +1804,20 @@ static void
 trim_func_three_args(struct sql_context *context, sql_value *arg1,
 		     sql_value *arg2, sql_value *arg3)
 {
-	assert(sql_value_type(arg1) == MP_INT || sql_value_type(arg1) == MP_UINT);
+	assert(sql_value_type(arg2) == MP_UINT);
 	const unsigned char *input_str, *trim_set;
-	if ((input_str = sql_value_text(arg3)) == NULL ||
-	    (trim_set = sql_value_text(arg2)) == NULL)
+	if ((input_str = sql_value_text(arg1)) == NULL ||
+	    (trim_set = sql_value_text(arg3)) == NULL)
 		return;
 
-	int trim_set_sz = sql_value_bytes(arg2);
-	int input_str_sz = sql_value_bytes(arg3);
+	int trim_set_sz = sql_value_bytes(arg3);
+	int input_str_sz = sql_value_bytes(arg1);
 	uint8_t *char_len;
 	int char_cnt = trim_prepare_char_len(context, trim_set, trim_set_sz,
 					     &char_len);
 	if (char_cnt == -1)
 		return;
-	trim_procedure(context, sql_value_int(arg1), trim_set, char_len,
+	trim_procedure(context, sql_value_int(arg2), trim_set, char_len,
 		       char_cnt, input_str, input_str_sz);
 	sql_free(char_len);
 }
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 995875566..9c4bf491b 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1123,32 +1123,31 @@ expr(A) ::= TRIM(X) LP trim_operands(Y) RP(E). {
 %type trim_operands {struct ExprList *}
 %destructor trim_operands {sql_expr_list_delete(pParse->db, $$);}
 
-trim_operands(A) ::= trim_from_clause(F) expr(Y). {
-  A = sql_expr_list_append(pParse->db, F, Y.pExpr);
+trim_operands(A) ::= trim_specification(N) expr(Z) FROM expr(Y). {
+  A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
+  struct Expr *p = sql_expr_new_dequoted(pParse->db, TK_INTEGER,
+                                         &sqlIntTokens[N]);
+  A = sql_expr_list_append(pParse->db, A, p);
+  A = sql_expr_list_append(pParse->db, A, Z.pExpr);
 }
 
-trim_operands(A) ::= expr(Y). {
+trim_operands(A) ::= trim_specification(N) FROM expr(Y). {
   A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
+  struct Expr *p = sql_expr_new_dequoted(pParse->db, TK_INTEGER,
+                                         &sqlIntTokens[N]);
+  A = sql_expr_list_append(pParse->db, A, p);
 }
 
-%type trim_from_clause {struct ExprList *}
-%destructor trim_from_clause {sql_expr_list_delete(pParse->db, $$);}
-
-/*
- * The following two rules cover three cases of keyword
- * (LEADING/TRAILING/BOTH) and <trim_character_set> combination.
- * The case when both of them are absent is disallowed.
- */
-trim_from_clause(A) ::= expr(Y) FROM. {
+trim_operands(A) ::= expr(Z) FROM expr(Y). {
   A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
+  struct Expr *p = sql_expr_new_dequoted(pParse->db, TK_INTEGER,
+                                         &sqlIntTokens[TRIM_BOTH]);
+  A = sql_expr_list_append(pParse->db, A, p);
+  A = sql_expr_list_append(pParse->db, A, Z.pExpr);
 }
 
-trim_from_clause(A) ::= trim_specification(N) expr_optional(Y) FROM. {
-  struct Expr *p = sql_expr_new_dequoted(pParse->db, TK_INTEGER,
-                                         &sqlIntTokens[N]);
-  A = sql_expr_list_append(pParse->db, NULL, p);
-  if (Y != NULL)
-    A = sql_expr_list_append(pParse->db, A, Y);
+trim_operands(A) ::= expr(Y). {
+  A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
 }
 
 %type expr_optional {struct Expr *}
diff --git a/test/sql/collation.result b/test/sql/collation.result
index bfc89e1b8..23ff3d06e 100644
--- a/test/sql/collation.result
+++ b/test/sql/collation.result
@@ -1133,6 +1133,14 @@ box.execute("SELECT DISTINCT trim(s2) FROM jj;")
   rows:
   - ['A']
 ...
+box.execute("SELECT DISTINCT trim(BOTH FROM s2) FROM jj;")
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['A']
+...
 box.execute("INSERT INTO jj VALUES (3, 'aS'), (4, 'AS');")
 ---
 - row_count: 2
diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua
index 407fc19dc..8c2e8a133 100644
--- a/test/sql/collation.test.lua
+++ b/test/sql/collation.test.lua
@@ -308,6 +308,7 @@ box.execute("DROP TABLE qms4;")
 box.execute("CREATE TABLE jj (s1 INT PRIMARY KEY, s2 VARCHAR(3) COLLATE \"unicode_ci\");")
 box.execute("INSERT INTO jj VALUES (1,'A'), (2,'a')")
 box.execute("SELECT DISTINCT trim(s2) FROM jj;")
+box.execute("SELECT DISTINCT trim(BOTH FROM s2) FROM jj;")
 box.execute("INSERT INTO jj VALUES (3, 'aS'), (4, 'AS');")
 box.execute("SELECT DISTINCT replace(s2, 'S', 's') FROM jj;")
 box.execute("SELECT DISTINCT substr(s2, 1, 1) FROM jj;")
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 04/10] box: add new options for functions
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (2 preceding siblings ...)
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 03/10] sql: change signature of trim() imeevma
@ 2020-08-14 15:04 ` imeevma
  2020-08-22 14:28   ` Vladislav Shpilevoy
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 05/10] sql: use has_vararg for built-in functions imeevma
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:04 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

The has_vararg option allows us to work with functions with a variable
number of arguments. This is required for built-in SQL functions.

Suppose this option is TRUE for a built-in SQL function. Then:
1) If param_list is empty, all arguments can be of any type.
2) If the length of param_list is not less than the number of the given
arguments, the types of the given arguments must be compatible with the
corresponding types described in param_list.
3) If the length of param_list is less than the number of given
arguments, the rest of the arguments must be compatible with the last
type in param_list.

The is_overloaded option allows us to deal with functions that can take
STRING or VARBINARY arguments. In case the first of these arguments is
of type VARBINARY, we use the overloaded version of the function. By
default, we use the STRING version of the function.

The has_overload option indicates that the function has an overloaded
version that accepts VARBINARY. See an explanation of the is_overloaded
option.

Part of #4159
---
 src/box/func_def.c | 13 +++++++++++++
 src/box/func_def.h | 18 ++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/src/box/func_def.c b/src/box/func_def.c
index 11d2bdb84..29929aea3 100644
--- a/src/box/func_def.c
+++ b/src/box/func_def.c
@@ -40,10 +40,17 @@ const char *func_aggregate_strs[] = {"none", "group"};
 
 const struct func_opts func_opts_default = {
 	/* .is_multikey = */ false,
+	/* .is_overloaded = */ false,
+	/* .has_overload = */ false,
+	/* .has_vararg = */ false,
 };
 
 const struct opt_def func_opts_reg[] = {
 	OPT_DEF("is_multikey", OPT_BOOL, struct func_opts, is_multikey),
+	OPT_DEF("is_overloaded", OPT_BOOL, struct func_opts, is_overloaded),
+	OPT_DEF("has_overload", OPT_BOOL, struct func_opts, has_overload),
+	OPT_DEF("has_vararg", OPT_BOOL, struct func_opts, has_vararg),
+	OPT_END,
 };
 
 int
@@ -51,6 +58,12 @@ func_opts_cmp(struct func_opts *o1, struct func_opts *o2)
 {
 	if (o1->is_multikey != o2->is_multikey)
 		return o1->is_multikey - o2->is_multikey;
+	if (o1->is_overloaded != o2->is_overloaded)
+		return o1->is_overloaded - o2->is_overloaded;
+	if (o1->has_overload != o2->has_overload)
+		return o1->has_overload - o2->has_overload;
+	if (o1->has_vararg != o2->has_vararg)
+		return o1->has_vararg - o2->has_vararg;
 	return 0;
 }
 
diff --git a/src/box/func_def.h b/src/box/func_def.h
index d99d89190..bab2186da 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -68,6 +68,24 @@ struct func_opts {
 	 * packed in array.
 	 */
 	bool is_multikey;
+	/**
+	 * True if this function is overloaded version of another function.
+	 *
+	 * Currently only used in built-in SQL functions.
+	 */
+	bool is_overloaded;
+	/**
+	 * True if this function has overloaded version.
+	 *
+	 * Currently only used in built-in SQL functions.
+	 */
+	bool has_overload;
+	/**
+	 * True if the function can have a variable number of arguments.
+	 *
+	 * Currently only used in built-in SQL functions.
+	 */
+	bool has_vararg;
 };
 
 extern const struct func_opts func_opts_default;
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 05/10] sql: use has_vararg for built-in functions
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (3 preceding siblings ...)
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 04/10] box: add new options for functions imeevma
@ 2020-08-14 15:05 ` imeevma
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions imeevma
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: imeevma @ 2020-08-14 15:05 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

After this patch, the has_varargs option will be used to determine that
the function has a variable number of arguments.

Part of #4159
---
 src/box/sql/func.c    | 3 ++-
 src/box/sql/resolve.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index e5da21191..1afee4924 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2182,7 +2182,7 @@ sql_func_by_signature(const char *name, int argc)
 	if (base == NULL || !base->def->exports.sql)
 		return NULL;
 
-	if (base->def->param_count != -1 && base->def->param_count != argc)
+	if (!base->def->opts.has_vararg && base->def->param_count != argc)
 		return NULL;
 	return base;
 }
@@ -2928,6 +2928,7 @@ func_sql_builtin_new(struct func_def *def)
 	def->returns = sql_builtins[idx].returns;
 	def->aggregate = sql_builtins[idx].aggregate;
 	def->exports.sql = sql_builtins[idx].export_to_sql;
+	def->opts.has_vararg = sql_builtins[idx].param_count == -1;
 	return &func->base;
 }
 
diff --git a/src/box/sql/resolve.c b/src/box/sql/resolve.c
index 6f625dc18..5238555c3 100644
--- a/src/box/sql/resolve.c
+++ b/src/box/sql/resolve.c
@@ -614,7 +614,7 @@ resolveExprStep(Walker * pWalker, Expr * pExpr)
 				pNC->nErr++;
 				return WRC_Abort;
 			}
-			if (func->def->param_count != -1 &&
+			if (!func->def->opts.has_vararg &&
 			    func->def->param_count != n) {
 				uint32_t argc = func->def->param_count;
 				const char *err = tt_sprintf("%d", argc);
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (4 preceding siblings ...)
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 05/10] sql: use has_vararg for built-in functions imeevma
@ 2020-08-14 15:05 ` imeevma
  2020-08-22 14:29   ` Vladislav Shpilevoy
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 07/10] sql: move built-in function definitions in _func imeevma
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:05 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

This patch adds overload function definitions for the length(),
substr(), trim(), and position() functions.

Part of #4159
---
 src/box/sql/analyze.c |  6 ++---
 src/box/sql/expr.c    | 29 ++++++++++++++++++++---
 src/box/sql/func.c    | 54 ++++++++++++++++++++++++++++++++++++++++---
 src/box/sql/sqlInt.h  |  5 ++--
 src/box/sql/vdbemem.c |  2 +-
 5 files changed, 84 insertions(+), 12 deletions(-)

diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index f74f9b358..d304b266e 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -716,7 +716,7 @@ callStatGet(Vdbe * v, int regStat4, int iParam, int regOut)
 {
 	assert(regOut != regStat4 && regOut != regStat4 + 1);
 	sqlVdbeAddOp2(v, OP_Integer, iParam, regStat4 + 1);
-	struct func *func = sql_func_by_signature("_sql_stat_get", 2);
+	struct func *func = sql_func_by_signature("_sql_stat_get", false, 2);
 	assert(func != NULL);
 	sqlVdbeAddOp4(v, OP_BuiltinFunction0, 0, regStat4, regOut,
 		      (char *)func, P4_FUNC);
@@ -856,7 +856,7 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space)
 		sqlVdbeAddOp2(v, OP_Integer, part_count, stat4_reg + 1);
 		sqlVdbeAddOp2(v, OP_Integer, part_count, stat4_reg + 2);
 		struct func *init_func =
-			sql_func_by_signature("_sql_stat_init", 3);
+			sql_func_by_signature("_sql_stat_init", false, 3);
 		assert(init_func != NULL);
 		sqlVdbeAddOp4(v, OP_BuiltinFunction0, 0, stat4_reg + 1,
 			      stat4_reg, (char *)init_func, P4_FUNC);
@@ -957,7 +957,7 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space)
 				  pk_part_count, key_reg);
 		assert(chng_reg == (stat4_reg + 1));
 		struct func *push_func =
-			sql_func_by_signature("_sql_stat_push", 3);
+			sql_func_by_signature("_sql_stat_push", false, 3);
 		assert(push_func != NULL);
 		sqlVdbeAddOp4(v, OP_BuiltinFunction0, 1, stat4_reg, tmp_reg,
 			      (char *)push_func, P4_FUNC);
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index bc2182446..99ca91bba 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -328,8 +328,15 @@ sql_expr_coll(Parse *parse, Expr *p, bool *is_explicit_coll, uint32_t *coll_id,
 		if (op == TK_FUNCTION) {
 			uint32_t arg_count = p->x.pList == NULL ? 0 :
 					     p->x.pList->nExpr;
+			bool has_blob_arg = false;
+			if (arg_count > 0) {
+				enum field_type type =
+					sql_expr_type(p->x.pList->a[0].pExpr);
+				has_blob_arg = type == FIELD_TYPE_VARBINARY;
+			}
 			struct func *func =
-				sql_func_by_signature(p->u.zToken, arg_count);
+				sql_func_by_signature(p->u.zToken, has_blob_arg,
+						      arg_count);
 			if (func == NULL)
 				break;
 			if (sql_func_flag_is_set(func, SQL_FUNC_DERIVEDCOLL) &&
@@ -3975,8 +3982,15 @@ sqlExprCodeTarget(Parse * pParse, Expr * pExpr, int target)
 			}
 			nFarg = pFarg ? pFarg->nExpr : 0;
 			assert(!ExprHasProperty(pExpr, EP_IntValue));
+			bool has_blob_arg = false;
+			if (nFarg > 0) {
+				enum field_type type =
+					sql_expr_type(pExpr->x.pList->a[0].pExpr);
+				has_blob_arg = type == FIELD_TYPE_VARBINARY;
+			}
 			zId = pExpr->u.zToken;
-			struct func *func = sql_func_by_signature(zId, nFarg);
+			struct func *func =
+				sql_func_by_signature(zId, has_blob_arg, nFarg);
 			if (func == NULL) {
 				diag_set(ClientError, ER_NO_SUCH_FUNCTION,
 					 zId);
@@ -5448,9 +5462,18 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr)
 						uint32_t argc =
 							pExpr->x.pList != NULL ?
 							pExpr->x.pList->nExpr : 0;
+						bool has_blob_arg = false;
+						if (argc > 0) {
+							enum field_type type =
+								sql_expr_type(pExpr->x.pList->a[0].pExpr);
+							has_blob_arg =
+								type == FIELD_TYPE_VARBINARY;
+						}
 						pItem->func =
 							sql_func_by_signature(
-								name, argc);
+								name,
+								has_blob_arg,
+								argc);
 						assert(pItem->func != NULL);
 						assert(pItem->func->def->
 						       language ==
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 1afee4924..ae1842824 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2169,19 +2169,27 @@ sql_is_like_func(struct Expr *expr)
 	    expr->x.pList->nExpr != 2)
 		return 0;
 	assert(!ExprHasProperty(expr, EP_xIsSelect));
-	struct func *func = sql_func_by_signature(expr->u.zToken, 2);
+	struct func *func = sql_func_by_signature(expr->u.zToken, false, 2);
 	if (func == NULL || !sql_func_flag_is_set(func, SQL_FUNC_LIKE))
 		return 0;
 	return 1;
 }
 
 struct func *
-sql_func_by_signature(const char *name, int argc)
+sql_func_by_signature(const char *name, bool has_blob_arg, int argc)
 {
 	struct func *base = func_by_name(name, strlen(name));
-	if (base == NULL || !base->def->exports.sql)
+	if (base == NULL || !base->def->exports.sql ||
+	    base->def->opts.is_overloaded)
 		return NULL;
 
+	if (has_blob_arg && base->def->opts.has_overload) {
+		const char *overload_name = tt_sprintf("%s_VARBINARY", name);
+		base = func_by_name(overload_name, strlen(overload_name));
+		assert(base != NULL && base->def->exports.sql);
+		assert(base->def->opts.is_overloaded);
+	}
+
 	if (!base->def->opts.has_vararg && base->def->param_count != argc)
 		return NULL;
 	return base;
@@ -2497,6 +2505,16 @@ static struct {
 	 .call = lengthFunc,
 	 .finalize = NULL,
 	 .export_to_sql = true,
+	}, {
+	 .name = "LENGTH_VARBINARY",
+	 .param_count = 1,
+	 .returns = FIELD_TYPE_INTEGER,
+	 .aggregate = FUNC_AGGREGATE_NONE,
+	 .is_deterministic = true,
+	 .flags = SQL_FUNC_LENGTH,
+	 .call = lengthFunc,
+	 .finalize = NULL,
+	 .export_to_sql = true,
 	}, {
 	 .name = "LESSER",
 	 .call = sql_builtin_stub,
@@ -2617,6 +2635,16 @@ static struct {
 	 .call = position_func,
 	 .finalize = NULL,
 	 .export_to_sql = true,
+	}, {
+	 .name = "POSITION_VARBINARY",
+	 .param_count = 2,
+	 .returns = FIELD_TYPE_INTEGER,
+	 .aggregate = FUNC_AGGREGATE_NONE,
+	 .is_deterministic = true,
+	 .flags = 0,
+	 .call = position_func,
+	 .finalize = NULL,
+	 .export_to_sql = true,
 	}, {
 	 .name = "POWER",
 	 .call = sql_builtin_stub,
@@ -2747,6 +2775,16 @@ static struct {
 	 .call = substrFunc,
 	 .finalize = NULL,
 	 .export_to_sql = true,
+	}, {
+	 .name = "SUBSTR_VARBINARY",
+	 .param_count = -1,
+	 .returns = FIELD_TYPE_STRING,
+	 .aggregate = FUNC_AGGREGATE_NONE,
+	 .is_deterministic = true,
+	 .flags = SQL_FUNC_DERIVEDCOLL,
+	 .call = substrFunc,
+	 .finalize = NULL,
+	 .export_to_sql = true,
 	}, {
 	 .name = "SUM",
 	 .param_count = 1,
@@ -2787,6 +2825,16 @@ static struct {
 	 .call = trim_func,
 	 .finalize = NULL,
 	 .export_to_sql = true,
+	}, {
+	 .name = "TRIM_VARBINARY",
+	 .param_count = -1,
+	 .returns = FIELD_TYPE_STRING,
+	 .aggregate = FUNC_AGGREGATE_NONE,
+	 .is_deterministic = true,
+	 .flags = SQL_FUNC_DERIVEDCOLL,
+	 .call = trim_func,
+	 .finalize = NULL,
+	 .export_to_sql = true,
 	}, {
 	 .name = "TYPEOF",
 	 .param_count = 1,
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index adf90d824..9ff1dd3ff 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -4441,13 +4441,14 @@ sql_func_flag_is_set(struct func *func, uint16_t flag)
  * A SQL method to find a function in a hash by its name and
  * count of arguments. Only functions that have 'SQL' engine
  * export field set true and have exactly the same signature
- * are returned.
+ * are returned. If has_blob_arg is set to true, the varbinary
+ * version of the function is returned is the functions has one.
  *
  * Returns not NULL function pointer when a valid and exported
  * to SQL engine function is found and NULL otherwise.
  */
 struct func *
-sql_func_by_signature(const char *name, int argc);
+sql_func_by_signature(const char *name, bool has_blob_arg, int argc);
 
 /**
  * Generate VDBE code to halt execution with correct error if
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 8e9ebf7ab..3af96c37c 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -1327,7 +1327,7 @@ valueFromFunction(sql * db,	/* The database connection */
 	pList = p->x.pList;
 	if (pList)
 		nVal = pList->nExpr;
-	struct func *func = sql_func_by_signature(p->u.zToken, nVal);
+	struct func *func = sql_func_by_signature(p->u.zToken, false, nVal);
 	if (func == NULL || func->def->language != FUNC_LANGUAGE_SQL_BUILTIN ||
 	    !func->def->is_deterministic ||
 	    sql_func_flag_is_set(func, SQL_FUNC_NEEDCOLL))
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 07/10] sql: move built-in function definitions in _func
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (5 preceding siblings ...)
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions imeevma
@ 2020-08-14 15:05 ` imeevma
  2020-08-22 14:30   ` Vladislav Shpilevoy
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 08/10] box: add param_list to 'struct func' imeevma
                   ` (3 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:05 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

This patch moves SQL built-in function definitions to _func. This helps
create an unified way to check the types of arguments. It also allows
users to see these definitions. Also, this patch enables overloading for
length(), trim(), position() and substr() functions.

Part of #4159
---
 src/box/bootstrap.snap         | Bin 5976 -> 6446 bytes
 src/box/lua/upgrade.lua        | 192 +++++++++++++++++++++++++++++++++
 src/box/sql/func.c             |   6 --
 test/box-py/bootstrap.result   |   6 +-
 test/box/access.result         |   2 +-
 test/box/access.test.lua       |   2 +-
 test/box/access_bin.result     |   2 +-
 test/box/access_bin.test.lua   |   2 +-
 test/box/access_sysview.result |   8 +-
 test/box/function1.result      |   6 +-
 test/wal_off/func_max.result   |   8 +-
 11 files changed, 212 insertions(+), 22 deletions(-)

diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
index 8bd4f7ce24216a8bcced6aa97c20c83eb7a02c77..17e5e2c65ea977620fae969da73b5449c9227078 100644
GIT binary patch
literal 6446
zcmV+}8PVobPC-x#FfK7O3RY!ub7^mGIv_GGHZCwNG&3z{VK`$kV=*x@VG2oZb97;D
zV`VxZI5{{mH#s$AEnzWaFfBARFg7h>IWRXZIb~sDV>vZ4Wn(xq3RXjGZ)0mZAbWiZ
z3e~y`y3H810M4`=qUWUm00000D77#B08nMc0NU5=Fi8-2Xl2a6Gw=*N1Dp2d#+F*A
z60ngZM6#TmglvJq_#EX=$!Jge7J1w0|Kzo1%_UPNi@+(ZlQ~>}FT|LOA)HF=P_aH#
zqoo}H1NZ{X0v~|TwrSK1iB%s^r5O^3L=UQr#wjWz(gUMWYU+P9$Y%#qt<{qQG`!uQ
z1Kw)D0dIMm!3G#;FoRx@!Pl@&4qK=N7|<%X;A{A#z=E&gbAk$J5`0lGJ2SzTJ4`~&
zNbp6AI`cIw^JV617(vw6aQK9v%oiU%5CI?Zp#u=K!v-E`hfof*!*+OVU^sk&8V;XM
z$D!5mX?)6LUZ+gv8#9?q#=m~|?{|~;+wWo*ZnsIec~Q89o9G_dmLJ(6@yH%oGVROm
z;mfjQUiM{$n`MJ6%W5IZKGz`2s+whiEBnl<)(il(PP5hvFhGMUkj|h9EZ~&cq*G?0
z2q?g^2PD9<V-R53r<em?p#fuFVHh#5@D=_Hpl$iW%(i;JWzClM;1<sp41(zegG{{O
za|fn{>X>HGrCqy&NoY<->-w^^gI;nw$YyupzU-iqmmTba+rb@tCMgAjw=TgTQc^JZ
zwB7zcLi)dE`~S)1h!&@S8Ub9^+`rdanY;V%HTR#UUdP!|zyJhjEx;D7wG7~DBLQ4p
zq{Y>)&SIoBmLjdMWYPL*1q;waX;*+AI<o@wYZB<8L^N8Qq1h}>07SkTk*^k~$`GxK
zN&vGi0ICdKHGR_?QmR+3q3Z%QciCc^>aHnN@+t*@yb4XpYhHt-9HR&wk5Gb+$0t#|
z6DZy{dEz}2Cs}>Unq>6}#3ZX$S$!^%(YStqW?V{^G~0f(aV7cjNR)VgB#HMvNO;c?
z&W-qRUZh6@i5Ss9o+28kfkTKuK?&i3f(kMO3Tj7)_dtSp|6|8{A3%EV@uPPhK6>BL
zgGkiDgGkJ=gGkIz96D$>a;)4{wZEF#z!A(=L_Q7X^jYJ^v1h)4OdT7<-HXkwrdY8W
zIx*OpW6%3{{hjO=a^1goMy>hTa7nCJSKa(=;wl-Bj!q1Arp%f#yqXSo#KfArTnVd^
z@#w^0XJR&D+}*xwW77ky6N8<ZgQM$CpE8k&l<i;`EQcL4`iJ9B85N?2LyZ{BOg_EB
zNtC29rcLaV<Y!`cgG^y=gUiofS54Pe?_d_+Lav!<QYP*};-t6HAAQr9W?>>nikX=7
zzvwEn>bA_{@psLg+@)0&clXb2>#8Xh=JD~;Eikw3%TvFz^xJ0R^2l@Y%ux@MLcaU=
zOf$$bu%}Pd#APOGR`qEU@8D?eJuh`zeE*XBHo0mp+@8NqT|9o}x>K#@qIYUy=1MbD
zc-?97b-U0R1=Fp|4%OTOuk_cgUei_gu7i3_+auq`)D6Zsnnu=Rgr~?5A}<RigeS<5
z9U*$?_V0lNp|$z3Gi&1m$UgF2Y}4Zht&4|`S=Wvp&>0bJ1{8Mb;6ZbfW5)~!j^Zc_
z#U`^?gd3Q;xsii~vVntzs&RvbVxe?w&|)}>qce=7I>#dnM{$%k=c`J_gQHGP&op2-
z>hwfo4h?!n8Z)RFXk@2kc7m31hHE6l4A(eD8LrVaib2LUhB3xAf+5B>ZsQkW^w$e8
z`rE}9{p~LoUcB!XUA(UrT)fXLwxDrgp~Z~riY&ffE8)v!1s45RaYg@BSkZs~Q&jQ4
z6jb~l#T5TPp^)PLC!+ZO=>!y3&nBK=KNC){e~Bj8&weGC_&<py{zpQI|DQ)Bk-rg0
z<X^-Q`R7lB5wtf$6ft`v1QABRwQ6E_gSw}w(Z2~|2=9Rq!uubB@J}CrpmFu^!;Fgu
zADTbs((bu-=%ITZc<4Td9lGy6sza0i%Av{Q5N+~J{)R8_ZNS0%8gB5O1{=Jep$6||
zpus<5n1KezONJR7IWeev^;%usYnpBk2L`Y0Uy!rC3ovZIeG4yW&w>luuh4?_+N;1q
z_HkCo9*zpxXWtda?Qh_?-A$L<-QMO+TTH=CTSTEvTWpJ5Ddge_DCFW6S1#rvn-jVV
z-Rka>O?SJi1tq#!Poj&ZMRbYoRhARW^}KoY<$2OWnNGfJa;{`@o?HUCl@iEJNCG*K
zud(_inD9RdYyKahgf0RJy@xPD?;wcKstKWO1Gq=g*E{<4n3#t<b+grSOgCN=@8G`O
z56lwlAfnOja6R5FOcUp5#oUyV7{`!TQ?e=X4Kg>Roeih^HL;By<<sGKOiWyZl}g&(
zn8x^+SV!n3?73%S>NAVwwPv^N+UTY}vsz|yV0vCtI-D<u`yHh@Fug3igyZ>?I2N2;
z(UN7R1q%_76(TA{LsW)lLo*cCl&Sr4NhvQmax{0*@HgmOQ~TwT*bVYsrEC9Isq*9k
zQ^0r#><C$qc3_PKllZG2*k-&kfLv22Ucje-mjtj90!jy9E@QZeF*3j?jL{Ftj1~D{
zMLiwlR1uE|3=tF2Zb&z@G?WwiBIHF%2p6Oj)I};NNp?fZkbrDRprfDK50{?;Sf5!X
z1SHv#WPl_B_-OGl@+peNda_`olpNfHrRNF}D@14*&v=MnNGUJ<Hf{bgqqOc{3ERQZ
zDjCoa&q=Ri&)qE&^}mXWX>}XB_DwJ3`+x0I?r!h3Pfx%O1kiL&W#M0?1!k^Ozp++V
zReqnWQd1~ysbsaTij~*m%iSyWXW}x8>z<2OYr9!h&F4G!@42g{!(a-<D*(BuTz1X1
zF!ft2i;4~QO<5xK&nkcWPp_~SDN{n>?mu;6C81Y|{Qhm#)QPzS8Fw!lMG@+D8+R{W
z_1tE4Uwu`R0lHaQyPxMSY#Mhjrb}D|g{o`+8dD}FdcyuUQ+4g%bcu(Yc%bq(XiS+{
zs7cqq&FS~K)%sa?wW*0jS8ck*C*5q@tfp=I@>M^%z?Em8`mLqz?>FDXuD@PWzqh<r
z)tt6Xp3Tof-G#z@<#~U<?oGejl$d)iVx^kz|GHi4xVk{4Fuy{cTj%?=9|Ai<<!>-u
z;vgnf#<YoLvYv~}oveD&E8i&-0|BWqrdarA$<J#F#Y%p7)~)LTTlJ_}HLJ_t^Y_cA
z%r`OFwNQV)n??P-QVrK{lMYL6#IOx6IyGWmbc|v|SO^xA&G|{g2MtUFM|p5`X-1ba
zlE}D?fh2}4j3Y2=UvTlFfG!B&V#^i-XdyrrTCgy>qR5IYRbZjw$`n?lphYo*g6N4M
zCjw}q3KLY806>W;Nk~CL=!hUAq7(szh$llh5u!;DOn{L60OZ4x9)5UGvO^LbkmPVA
z>hslugOMAG*g&KP9vX(sAVh{BG3<B&k@EuvA20N9fkz8FSkSRT4i#|3xdF#USB*D1
zZene43{%O9aCFX`IXPeydSKdjZQT5Ia@B~zFqN$6gszLPU*aWO>TZj=XJH*Sx^=r!
z-7WAfGI5MbHiV;m6W6@Nm`XPE+#(FJ#iMU)Uy{@*U=GkuN8KMPesR)9*0=&uW<8Z+
zX4R{xuKlb0UcbU6hEd6ew6<`UEHhI}IGzuRldzCn_;#iDGm2LWlWt)oXYH0-Vq3H4
z#7E59FPB8FTmuOO39Z>}gMQcEXCNRUy}D|U6(VZOzxn2(Wc^!&;sTOi3)7#!PunJ+
zJ~07Sx}OtoF@cE(ShG{Y<!)baIV>zd2}ru%uGhp`Na*!$IVLt5!B@NSx-@Z-2)mvV
zX9>Yp<N3tgnivZRz8+4@jft<Q;oZXAme|S%y<4ufvpF#>jhNa}63?n|OG!*bwd36m
zaN;2kypXtD?x$no0H);hY`i+!<&Lm$6=axN5>t_?`F6e~1^~^km&8*#;DPBWu@nxN
zmRM+moD~kzK+G+Pqh!!Ekoa<@RPU&k#85Es`hv?T@skUd*og&Ry5G*08nEMC;U*RM
zZa&*i3p1f$x0J-P33j_&(h>tnP*bvH;U#BM-pyyL@oGx8E&OAqiw>8=-E2*)Bno%4
zDRGiAa?tFMbTu9mBZ(lF`{|N!Ond|a19rJ3_IW_+#>Dhn;vNUETHx!hOeB)-x8p6b
zkp_@HTyMwZ#6_UAq?CMJbhjK66FKrcF)h`6y%X>dr_`sr@pwO*78YVm;`w?`9OS+E
zYDiAJ^OtZwB?jVaNJ{*pqZv~}($#RepO-7_qiKos)oi+*EoJ)ccwMvth)bvxi12Vh
z$QV;H13*9k0HYcN5X1zvw9+ON009gpbhvQRl7JC0;czGzOhzfBa*#rZG64W!1T#=G
z*8xqXuSAJO2fHAj9icpHP_y&76Od#7P7{ghYfiGGt;>XAJ^s~{CvtjP5Go*D=`CH-
zExFP=zp{I}rCV~PS9+&wx};lj<yU%T*L2CZ<j$}B%C6~>F4;}5^vbU3mM+PiUg@1(
z^DW(yJH65?x#uNak~_WfE4k)Nx@32H=T~;kmvl+4{H9lO&6jjbuJrbzGNbZ9H#=Oa
zPROnl;GTHHqoImmGht5K97juG@O8JYuZbs}e!XZ5ub2_yyG(YCIwc<FN-9Z54kZkg
zbrgin%|7u$$96@z)&D;}WU-HKpCQ6pNK=L4lKy!$lmKQ_J|eaIC>F99=`+LV>?(Vz
zY*=3D1`G0xGr7aRooE7)#%DzXWU{=na#GMUa*s$A5YjlnE_4NorktX~>KiVOAOd)!
z=;~(07cPfzA;Et`7}!O5FTd#W2>NJ10X9UUgeIt2Gup+_s>+~I4p>O-nNOprr2qDy
zBgMyuZ{u?6gvMpN9v7e0Mb)8O$s;^WOWAKIG$}MAxu>XP_uJseDh%Ac1=ipWS=l~4
z%ZwzFL}LDu$@=~o5C*9^4O)vGTL9=SIo1O0(@`n3iairXe?SHZaKva$#Aa}n+S(%x
z;Gd#xYy<xT)S}7!KseNR9R3JgvJ*T8_X;TjI6$RPVy-UvPQu(XqyWMNJ$F;;TL@%y
z{`&!Ti+<#RV#%PguRk!#_Z-M6!orq=9!_eJp)ajo0Fq}4YRF?r4jv&FTX`k#Qi97U
z{zbM2Ubv1tD0F!s0+<A~KBmGTAZ5W|H803*Iez*C7Bp8hx{$pP;2@Mh)`8IogXuV}
zfZmIB#gw=L)%-aKA_HY4j4KIfj8U7mP#0E7>N3WDlDHPrQ-GFdISFl4^;<ZwHzO1p
z{9RbqwTTG+5#k9;fqUuFxLIMyB5@KSYx5Ywh_ZYUUJ$Z>;x}m(_kRMk^3)7yf1bzh
zo(k0bjBdYe$ve&sU*B$P7|IQFiiX)ORb3%PR9h^=P>+zM-Q=7kuCdIS-ci?lGq7@d
z`%{R&H9_3|W{IF0X7r<6sSDW$FT=XNSj4!l!ur64P%3G*`og@BEVgSv;VNIj8V_e_
z#Jq0OX&(`XY3|<-qT}JT0d!2}Wd#g|%MoJIOC~ar(gKJr71@CaW0(g5=*aQ^`I!iK
zWWIwNR>U>alZtyjRF3~a`$c@`5$kM!?%DDsk<!({e>Kf~yX<xVvlcWF>`{yxBek}X
zzuDk&h)m0M;_-5L44fDQe%AM8+H15bQv&dCOc$^iQQTcN@SFzgLZpdb=;14G{<I6+
z5aw)F%yRb+#^g77Kfar^u&4#(77Q(T@SASinv52hTwxeSB}|qu7^5X7SBH7vk-2@D
zINq>w{&aCLZW4?^89T^w|8-!28q7%V_+VU1j!ErGAs9Vh^cWDOA>CxmwlEB%Bw)r{
z!g7q>TM@fi6@55BhWn>6PGH8cY4`#a#T$}rH~0D;ggXg5_y+hBY`x`^<O@$oR5WM~
zKV5^D;<x%3ujK=|W`o$>^&~^5;adJziPvHx^IOGgzGD1>0qFu!Dj5C>G31K%Zxv0u
z%d0JBlKINRNT7m6YWcCswnCnu4?V9Gc?6Nk?g$Xdq!fo{043+jp3HA*yly~Qie3gX
z40-_K6oFn)nR?$wf$@!E%f)Wws2tI$+U9LjUYN_aIvg9$DIAk4<!r$dWX2-6y7+|K
z-qtrWOb>2wekSx$Xw*C8|I@+mR|m*c#=?}mF|7ZakH9uI*V!I%mzNSc95|y*suk~P
zn6QS25j>y3w6~yWn|VwlGoTlQ)a|n*V9~^hEO_{#XN)j0Nt?Z|xVlsvQV<gD&@SL+
zAd2b#R=V5d6eeXMjrZv22i5<}<p-nR3o`Zj_vkK^En%5G6~NFOcHGL_DTjU{LryBo
z?Juj5DGyhYcq41$-Ur0-dO$KPU)0?WUK^MJ+sfZgw3&R;#)_zO_q-MLfk?;>J9_&{
z70MjCAhL{raaxg7TY^<n%d<Ct1mlTc(_hQs1m?{biTaN&eq@Diul}^)qIsl<>6OLl
z<pPF@J1v&LoKpj00P8KseU_*&ofgm3QqC{S#)(a^bP{5U6K3$GpoFfv+ooIWscm9e
zO}6xeMLHc-pq5cw*K+J;o1zC_MJ?Ypp_5**6c|3hEAxi@X_}*J%%zqtG&@{@^^F4w
zI7ir^y4XAHD!O%f=0tiZ9_dEw17OiA(EZUH%b^q^X5+$1S{LbAInF7Mu)gpZ%4JM}
zzQehv7D-Wk{$1qM`qO}NF>EW(X2!O^?<^E63|QRn_1lcZV^=Ql_|L8^^C1Bu*1z;n
zG=N3NU0TkELc5Xs(mAhgH57l@6{u6haU!6@w(mj0Q^gY;DG<(_qS>R8y*6Y4Sxt|0
zawU5x`WmH>Hzp7zwG+*%M3KIR+(h?>=DqaQn5l{Y9HwcMQwZ*NO2i9yEMVKJ@rj5o
zEbDBj4yYS^EP;Y-ga@K(;0FnBOnGe%xx(Ga((tk06zRKotO!6OH}it-+cM&PO42l5
z@?{|vU(DVaVHUm^LdVaBjsG&Lh*CSS>jribnBvygElT|WU1MggRkN!Rs%{Hy5$QJN
zq>Zz5QPvnjDl|F|F$$>spp@GMAQqQd=+n340uCGzo?ydT4ZUR$7CrJGNB?~wLR@~e
zzwh-B;_{9G`7DgXr;``$;7&NY{W4LO$5>Po@i7j|ZI_XhmYlXTUgZI)PlPpG6V1$c
zM?_pGYYZb4nw^3gBvkx>C|a-~ZPih@7@EPO9WwsOo{l1Rnk9gbK+JlKi6p(yY)DQ-
zNv(0SGD={7(QM^3u2USn5dKLD)y#KU0Q5aD4GwaTV1)9^=tx8KvD6M}!wbSC4Z2sO
zundl&#@}=#0B+w)<Im1U)X$nihzf=~phkzZKQ!cqcZ5kgMDIpvI1E?Juc=6keBVps
zgWh&t&zi!BjixhTMh%oVKIN7dghM)HuSRbeJV%T-jZ108B*%jxE+<q1Fo&<&?%Bkd
z81J*koQ*=jGGh23g!0Si2t)8uX2!g=6_@rkWaD5S#XYAUb2R}0+r$Bo7KIr*Yb|O$
z@r1Mm8{CxgPl)n*u<98k%OkK8m?n%8@gDaKrGeQKh;qs2GN@bg;er^oqjNPhlcV`^
z{ONd`Rz{vvXlRTG1|JMo^)6qsNTEfF`6~i_qLTuHY=``C;>amhbi)_QJk<rC2cNsn
zroKEP94*;_<U`*NgH}>{TttqjS7_WxM4$-1>?4gu!^6Vy`ehDPVP_k`*M$|oV}@bF
z3MEgo#0QQpo!IBzjYqSXgNZ3mX&Hz6CDC?D5hsF9LwoREE5V};d!<GOz{U|sAF_mx
zyuOm|r{0Wj*?58p*6-f%b#t*9GLLvY5DsBLGbOMaH10rnog-f8=xFM45|AbmoSGL`
zyfZrm(p1VzXZ`h^t}~ASJNZorkK9*EHz78uwI}uK8lKFW$8gFc`(EbYsX!~$pEX40
z=Aa%IBV#rA|9)b?2N?@f`-LX^F+UxsPUkoUQ#X_eVX^iR-ecRavPg=V2@Xw4V+T5G
zPYf(<ZksK%$}M$_FH46kXp}1J#K?#-h&V`pUYv*8d1k~R$qVbxD|nH`ajJ7R2)=Zh
zBu;H&0smN~d`J$MDQ~@%{oAr{tRR#QLbRxhw)PyvOl>tio42<wTz|H+^n`<e|M(nH
zMK~)f!)?qZeiedbnGQ{kUhBYUEcbwy$jTwrBkVg9=L(1xI8nJJ@Ad2f;U2H>#z~zI
z148n%eM9C!dntt6!XQdNcVxg^orh%33mG$4$3bW&CKi|!rBRyl!d@oQ-w?&~ja0LN
zfCYpq01!@ZQtm=hFp!c55fe#J-ALzF9d0C!hE0i7{LX8Ey{}2U28g4#$<7!YbZQ)<
zHBANq6YOJsZ0f#3+*^nJeBcQZyNRR;&N&aE)IFYhJF^pxESEtMQF5wC#fkrvkx>?n
zYcqs5?8o8LhYTAOe&-1<I-TmWUfU&c@eqQNuQy!~76%}Sc%A8jusQ%iiPzaKh^vDT
zrYA{`dwQ`tZt%EdY?&~GJlpLHRtI@DgM?a*hQyn%Zwk1prAwU>_i>J@AM%fSwbUTQ
IAJq`8?W$`(1ONa4

literal 5976
zcmV-e7pLe`PC-x#FfK7O3RY!ub7^mGIv_GGGcGVKGC49WXEHQ3GGb;i3Q2BrbYX5|
zWjY`=Fl01gGdD6VGGjS5Ei`5^GA&{=GB_<bHDfU`VqrL8H#1}kRzqxWV{1AfdwmKD
z)w&D1%@*JQ&RsBQp``!-0000ewJ-euP`$PQ3ejvYNf2nO>HuFnz!wkj#k+nW%pPgt
zBZ0)WtpN`tMI1&W;^gZsQ&N;l)}E-@Bxx#$t*xi7PvyfprWdR<qHD&edh~*lhI(p?
zO)caC=K`_<Z~kv%<)pw3o8_FK4QC~wfU^-yz*){hAi)C>(4G!J07hhyME2Rh12qO5
z03$jVXaJ1pRDgk60$>Z!*eC$j5|Pg}34qnqumK~c0b2t`L_iG~k?1@i4HzdHfB+|Q
zBANqOVgUxQ#40o_ktH4rAQGvnh(zbqk(fnv8maP`cPgLx#(XB9@vq<g`z_`D_S@~l
z-AWBNcZYkpi|uh``Eex{kLz(g!@lb#zUzACUEg)M*HyT#s~)baQx&djs@FBBuFjll
zO+Zj<G;2Ko0kx+9DD4@*08@EIPUZCpfB@@iCV+J<1HigE$(VVEW6HckF=XE1I|PoY
z)%l5`)$sw9wj0`ob$UOcFpN(qjOq!UyfEx@7e#wQ@z^ayK6Sg=7Z|85{G!`JI{PaJ
zW(%LbY@wIj7H*-l%ac%u^CA>FbrA}kH{AbCPXF6(|2w%f)%I+$Y2l*g{=M$Xyxo7R
zxqlS(dQX-uShQ$;C5zTpu(X=#N~@Qww7S(*n6##{r1g|4T05ms3HoPkO3*)PQi5JZ
z2K`g1R@;+AqwR^J^s_1bY<r+2)xMrYp?x`lLTpp?MR883R=tRA8`QjAT@=+{O`c?x
ziIc1%HOXpLkC+^nARUiNj*iD9OFR-K-XBTgy%8iib<U3D)M;cSr&&357Maz&ii~Jp
zM2axke!O`NDe^*ycpqel_e4VYP!P_65OMxvNC$lY(LtdfI;ev^d{kBO5UQ%$(W9y=
zh==z&c6gs7$a@?*dcTpQcN;i*uW_STq+z32oKd4#%uX3JYB6T0yiK*&qQQ_M%vnkv
z6=u}=BF3?2zJp637{%RP=1$Qq*$W_9jLos<{k#6n^$)r3-#69F0p4;*tkpK%{AFUS
znGg_=EXF3yoH9MzF4wfgnzuX|!<q>J$zp6Ukc_w6cWq?C!9cPYn?E>A-6&NqF0E!k
zI2Q90v}OeycC=B}*kEhng)FuvOe4yLeFSlH5$V6%CUe@dOZWJ@=2NXQ3yZh=XO?YK
zbPEd+_0eoFv+T=Jzq$0wW~1`RGm_0w6Olu{`}fN=R%v2Xzo?7KT-0moQ6>(;VY&CL
z)U5k{CHE{+)qJ=;f1BDp{-nB5oo3fJbushgnm1V8sQb2k*i;kKY`YTE%m%ge*Q{1i
zRrjuiT1D9--$oP<#yBj_YzT<Zj~+hWwTg$(jvhfgG_md9>)27dlOu?B2ZxS*<lDV-
zBS-Da1`gU+jT=vzQjHcIe$lW|!!x4>O$H9bVQUqgyiOVJAc`l(jI|1ejJ4`TjJ0a5
z0)kPK;V>LV!#E7+c#PpN99H`ShBXtyVN+AiE@C)r%E`r_+H;F8XiqJ;$WzEX^~_?6
z)`*1`t?`O1T4QUp0*h^|;)-pg!isI&#wn`kuM|}DH;O6x+g~W8c;6?YcwZ-=c%4l=
zQS)NL37XduO@O^w!`Dj*Ci+ieiT;sLqW}JnNaBAZkododBmQ?1VZ{GK6!E`P2qLna
zK@72eAcR=|4?(Pd{XPKke-A(W&w~&DJ9p?Ie;s(pKZhOi&mRXJYI$zRftJSx9E`q>
zs*Al9>W-pDpQnZ!yr;ni?`NpNJ7u7u<`u&XG%pxrVE(K_yZ3q_hVET}p?g0sbl-iL
z3sN4;kCbmAOv+1n7Qnn;p#|?%V8QzoR`4DL6}&$o1@ANgg_@o|C(!ipF(KWm@9ODR
zQFMDfCG^>z1U}o3P{Q`xi$H?*A&j6s2qI{&{RbgrzXycu^?Z<h_FsG2o<~pH@qD$t
z?RWC9t_OHnw}U&Z%er13WY<my*{$Q1-R#n_)7xjA-a6^@wzpl+V$0<$wp(4rw%BTM
zHMd&NnOb0;BmHyP<Vq&z$tCAVWsqkjgPa3okO%pktiJ&V|7$Sk{|q+h7--P@1sU{i
z0S2s`3s|-S3s^Re9AXixf~z>hBC!Yu%V>OaWwe50G`a~r+tV$HW1PV`@ou>tkcnaJ
z-!9jSTH+UL7alIE%x{T-hHZAapDqV%LUmz$To@|X`z5hVs5He2ZHkpDN=8hyCp4ue
zOiD?#4(6n>0d+_>efaRvylul@VRMcRs6*mb$TyU({R;&ORU7mG;~udcW=9&0Dib2{
zPk)fjSS7$pMVoklp8#J1z)t`?nZaDja3RAqhDi()&}u1L`o)%fKvr*-dP-rGqLg?^
zyCkS29TFI%FK&=>M^8sy+#Ge8;jCs(MmH;?rXpJrm!d7J&npovL$VCPGUN)86(T00
zC7Eof3bt~?2M<vhn&Lx>j~d1_A*Gm=n?CxL+WcLr(YpU+EC`2T&5V+GM)o=O+*`M(
zzm^)APPegZ&-6pS|JNSn?e<=K)DA`yh1q<=g+CNFn7Ky%-cs8%`8^UVMW2{`!o{}f
zR#x4Yw_54XMI{&4J-b>eyVq3C=R5cBd7Gj{A^OCslk6syU2{D|{qEwrqJw`;lveuZ
zl)wF>TKEf-H^brXA8q0%q&I2x{mZ6k6LUE-?(Q-z8ESSLcXzdVW^=l=eyYy|TQ05J
z&vPF#jk~+35-%ya>e|0XbcvOctpCkdUHcbR;v^(+QvM2!=n^9x>H4!7{XVl@KkJ?{
zb+Oo{%+{sSOqR_l%Caw4HB}o_dG@H^UFrUQ^Ih!vYZdjoj@N0LQC7;+`FW`OaG13`
z@9)>0={JiObI<M;s`>tJ+q;fx8%z%KJLH*dzF+$uu^m$W3Q;9KGJ<79nRqtq*;Q^N
z7L``{Mwi&gNRAQR!bL@XR?#OWQpB@n+cwCiN4=&wRsNp8S5jrZi^#5r`t!|n_4i3R
zT)$G<qgr?fkvK+_iHU~%T<ooIIn1n3`T5(XsLJXs<l=kC^)e2+#6v=+^fvmV4@J7b
zTyO!}LiR-qrYw}OK)PaPMNt(*RA3)d$b@j1Qi@v?O;FgL;NrxVCbTe-WeF@wTuH(T
z5==)d8BwJODnv{fLW&TzAZS1+{gC8Clpdh)@MH%kIyA|F2@Xg%9NDm>1|>8knE{Cm
zM`ADn!|Vkk7mD~P3L^qGqy-@?1X+Pdg`pIHpzz}aA0`O(>A{Cb2|Gv#8bOB$IYPhz
z!i^7X?5UxL#@CEIGVsK>1H*$C9RPXB>|hzbnl0h5KY#xC!E}aV+IMZ-{B1s8$YS_v
zw&d*EeZ3MF=}I^2>du9MxHQ}LWV+el>n^eEYL>Kx!}u<$S&8x0EJ-WNP`J8B&)UFr
zNt=W*qo<C$pX7p>(#F*Yf>K`n6F0A^mYS~poBUqCLnV$~&61inoKA^vOkWc>Ep=gh
zNGmrg*G5Q3NNR4`qu;gn*~kb<t8IED1qYk*Z@#%CSO2<CyaoC75dHakl$G+R6Kl!Z
z0h>4r2TP1)4(}<a!-2`^w(yk=oOVE-kBO~l;Pc^hOU#tP&xY%HVd5nWc)ldAa>32k
z`;oyhF%=7bz8o3c5>JuS!->Huv6KpYIGv4mYvNiQ8x#`LuyRmHtaQce;mkB~5}ce)
z2iz@jls8R0UDFnZVkO3g#82GVe!QO&J81*-A#oE5dSQG?%w&QnB}S4!mxYfaAcI5V
zB@p<UY)Gu+0WdjT5+`v$iIFt01M+U@7+y~cA7S8!``vh5*vJArC?uXKz~kw<D6vrj
zwx=5xF2V-w;eIz;&-QeygNxR9+2wRO+>MEesLA1ONjxMC6whgA>n*Vm1avy!u1ju-
zgB&o1r&D4g18`AMKqVeR<Fo_vdP)qWfM3qXi)!K@RZl-JJDhHbeH?|TqM&R)pB>y|
z3jOVHy&mw^g?Sje^nSi3-tppoHm4>I`cS!F66^3brzFnN%h;TDwww<5MGNDQ`q^$f
z-VKcc@_Jsn@D2CpYXbU~*hYJ^>7sBwCa%GMJYX0lrcvDSa=so;3=_{_$>5%lSOzhN
zxTu4JE1FV>W&i*HKmY(U8w3!9)vS}S3KIZ;fr5j=<WguP6bND%D3eu`Ap?M+00NLe
z07z&6Tpg7GabK8LFI#C?U@Zv>tR+E#ZH#FzNgQ`WroiFLan{tlfTbgB|Gh;!Y%xx%
zk`>T&gv_tCNQW)PNma4}nvRh9wHE2H#W<--RzTAcGQZX$9kv)JRmloyIzr~xTBO4k
z<D@EC0Zm89{9222*kYViB`cum2$^4Nkq%poe^fkOJ{puJS2M=fMW)_{j0_$`6@tyE
z*=P!3Dn_VvER`aoI1yw1cs@~CVJSaBy7qnyPa7o+*cQk64?y7NzK@CrLT4)y%VC<1
z-*f+Hb-JSj^xURSGlfXJE<f7$DXWgQZ#qi+3^s2AzbNzu9zmiF*pXCs7w7xhr$h>&
zms0^Veo$FNG9_{HVB12IACUv5#L1D+WDJ8VimHj)MNwheCI<_JBg>zG6CoF&93d!0
z*n_nd?LDdvi0g!)gr0Bd6nzMDc7%LM<sjvJEgvTju!~SAVd+}Tdsz!2BF1h(!_b#f
zCk5AQ$4P~86z@n$JLoW*OYH80aVQo;+7#h+cMJG|$lOjdfoKZ9Hm*yIbJ6V%U5Xyh
zYel(-;jV}zl1kV8a_F$JImhj(np%0~29^rr&B4tHsKO1~Z=!fFe7XK+m5*hNU00%s
z?o#ikDTQCVy$g(90y{MJ6r~8oyBF~F<DB^N6lp)b<MO*ca$+Qs(8qf&PtW}lU-vgK
zxXn!G$aq$Bjo(E3Xbj~M>*Idzd673F-Lj`&$W$i@vfDRXw!lDOjpVg4TWT9;t%mqR
z&k!jQ|E|NE0M{Uu!G2Sxiy5j?Spl}kbnKTxR8dclo|6x{^!UVw(*B~T@tlU#AAeSe
zm?5Yhvcd1|Kz#S{kuNWhdN*{j!Er`ggN$DUp~sUAFM?X*OokRgEE)2UdopebL3wLG
z_=%)b<9Ndm#bNXn@So_H1fgeo52Fuc2|?9rKLP`16x}=-w@6F`rN^5LFC++_9#1wK
zAdk$=FzGEYWXxew!{s1S6E@%D;y443dFEfaSL>BvG~L*LRyn_X11QRbP!$Fh2cPP5
zq_};L4EXUWUj|03mi1JOrh@DF7uU>UCVgAa3x5sjGIZ#fK&fo`6|1<4!G9HtW!L0B
znmN^#k552kR^~~^uG;!d@;<ZPOgcy4kSq$ea0*J>ZwB0Yt_{F^{@N>7;EiZ45pXcA
z?ws_=1?{Z4>QOa%V*zlnw>erc>ZE4#&Ll6WKDb5@n=(^4P94g6gNGIwOJ7p*H&gZy
z+`NlEcm??x!7ZUwogwr!UD|v#=B!~%Ji{78^!%8orm;JuE$QrgiJhT97VVAWf4l93
zt?M$%N<vKdH<`(1HEBe-i43A?*=LDmMKdm7nfQkk_XxkAfZJn-sY_ecMIpKkwn9@3
zgl*P;Tj}^Zg`Aj>aPHt;<HVoHjbI$-gEtiXpD7k9=`h_)^EPd6D}QA=(T8rGBS-P&
zwy@QL(1*ivywxGbu^Ob~slRZz+eDY#ckS6ZY$yE&(wl_~Z6S_27q5gypD-nI*rC75
ztb)r(m)TMgl&@CK%S#;3)pF1pSUIW0&t;Bdj*1B|rlS^|7YWm$My;}_;Myca5!11l
zb8HNj?%OGr$ebfSBJJF*`~F#U!rTeZ`1zn81t%x)4wn)lkQPcAqF@;-YU3t6>?!Gl
zawQP-zla5M>{O)yvCbvRtM+6HuLc3P{CK7rIYkq=@IQX@X%x-TMJTDIORdhcxK@Rh
z0BqP7dy;xIS#w7SUR>ZwI|j|^`Ox@D65h=Bx*mjAo5;DEt~WIb^CPD<Mqm#2F2x(9
zxZUDX#`;f7GiJEZ{z9U`A2K`=w`Rt^)*pmBNes%+hyjBctb4jHU@m^AjFFN7fs_wC
zEF)miNiN;=rR?&VzBG4Jw>qd&MoM8=&^?H5aDdD6c~xUjP0I2&r|AFDg>DCt-@wx!
zkpLkdivC6CrlHBci0uSPN>PZ?0XL+KLX(R7DwI4WiG}tw0wogGd!ok+sxE-snjD1a
zHCi|`%sP5TP+D9=YB@nT7#&HXj-em7Ab;#!do)CcR(n7NWbxm+p*L;N<>!RF-K-m$
z1Ji6HswE{dAdJflhR{V~!|am-H^fh!QxAiw!PbDV08RY6X7smRYqAg`vDolYTU<?_
zJ*iQa76FfL5f66WJ`ArSizT5#I@oXD(D&vo!~up~M6|eN7^APqVoB&r<I#8zUeah*
zWBQ$UOt*c!*T=-!s&n$9edr0b<OUu5w{GO?c6?A~!|ffu(qp#!#<DJ<<sCu|YmK5m
z{0|XJ?b*N((So&-hDAmn2n(AkNIxb>C!<Zd;3f2v%whxoK9`xc-lAgY)mf7Z%_r85
zh|+quS>~4aU!gB|Z~N@%XuF@pYG(em3vdzTOoN#a(}7Se*ehq)I(XqwT2w)rF~B-C
zdP;?Up&zy&9%#IKHC%^Qdq4$2kfnq^OAY+7qZ~@hC`cm)m`x2&iO@f^gBQdDjd!nx
z>(FWss2~Wkl+b6Xfj@SXLunZWX~Y1tso^OR`iFK{kl0Rvhx`Y!4_LYYScj?h1)Gq<
zN*5>#Dg*n`@kQf6{EjV742uZ90hHD{NZC*sxsT5)a@isKmzaNfJaP9H`JagVN9PyW
ziSQ*_X15vnK2fCWT!qpr-Q4x3`9!m)ZGS?-!0$<>DgF~AP&SFrflxizEobZ+c<~@v
zG&0kq*m?2L^mwER35-VEI}0Zx<$@IB!glL9x(y-nIO4;|R;IQaHw+ZsR-xm0T#9xw
z;-C@3Yb7g@guCzP$I2WH1jq*}2#q5NWEFj4eKh|wl0!OPZUkCYFbqoARpk5qm^Q%3
zUtY9@_@U2)>;{vD;a5zxlJMmep>gtE678o1B^M@V6gjw7NEC*xiE4qlVelnVP9thL
zJ6tcK2xQQoFL)t5(6R<<L#JVQ6;Vw|6gfEhFQOPQFo%_HOMGlI;j6(!=Igg<GJRM4
zBEy=}>Z_EH_iMk+W8`~-UKj{0bSve$kR!lyC$Hm;CNd_+G|)jSs0^6k2CiYy#{K5z
zOr!;8#y(CbN<{A1N|9)SgzgV88Be)RxS~=DmdFh1f;NMLhix~=wQM*DEW-|s#mgK&
z$Fex+HW5}(j=?pi*}i2FcCXTg7#tl`5m3wT;Q{x{+a(T<sO<oFg_W{c6NgF$Uvgq^
zMAi{SiXKZGcu9Hnbx!?f|AQPNWIB^vSOe-<{FsB~>%X=z0qX^g=jE0!<#0HmKAE3X
z?Qdlztz~*kV16F^EuOyXbFv$Ai!@dUeVmS-AE`DA>Q_1aKPrC244$$f4BZ>7gR2c-
z9D06I7kqkPC<;+e7>HKYja@d|_qf11VYfb79Mvf$v9b<b4LPI}cCd8)Em;4T(*_4Y
zY%p*WAUc1ODM}Kq5gE3!fZ!ch;{1pP!JgH#)Wbl)`tpIc9Ka(1fE@Ia%NjJ_InaVV
z{Rjc@SQ_Gov#YRTaVsI`mXT1BU^2}+*;z9k+G7yg8k;-=x!$+=a$;>dkc9#O*lvDM
zjT`tN2r=(}V)SBiSR!VRSBgr9a3?4`d*wNA@*YW<$t}%sllMr<Om1n8o4iL-W^zkC
zlcqB+zvOId2>P#BMz)0X*Zmx;QY*H}-|Ua*)SWMg1^N_8RIlVbPImSXCMmzA;fFre
G5UuU4uRFp3

diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index add791cd7..290741940 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -971,6 +971,197 @@ local function upgrade_to_2_3_1()
     create_session_settings_space()
 end
 
+--------------------------------------------------------------------------------
+-- Tarantool 2.5.2
+--------------------------------------------------------------------------------
+
+local function update_sql_builtin_functions()
+    local _func = box.space[box.schema.FUNC_ID]
+    local _priv = box.space[box.schema.PRIV_ID]
+    local updates
+    local datetime = os.date("%Y-%m-%d %H:%M:%S")
+    local overloaded_function_list = {
+        "LENGTH_VARBINARY", "POSITION_VARBINARY", "TRIM_VARBINARY",
+        "SUBSTR_VARBINARY",
+    }
+
+    for _, v in pairs(overloaded_function_list) do
+        local t = _func:auto_increment({ADMIN, v, 1, 'SQL_BUILTIN', '',
+                                       'function', {}, 'any', 'none', 'none',
+                                        false, false, true, {}, setmap({}), '',
+                                        datetime, datetime})
+        _priv:replace{ADMIN, PUBLIC, 'function', t.id, box.priv.X}
+    end
+
+    _func:run_triggers(false)
+    updates = {{'=', 'param_list', {'number'}}, {'=', 'returns', 'number'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('ABS', updates)
+
+    updates = {{'=', 'param_list', {'number'}}, {'=', 'returns', 'number'},
+               {'=', 'aggregate', 'group'}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('AVG', updates)
+    _func.index[2]:update('SUM', updates)
+    _func.index[2]:update('TOTAL', updates)
+
+    updates = {{'=', 'param_list', {'unsigned'}}, {'=', 'returns', 'string'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('CHAR', updates)
+
+    updates = {{'=', 'param_list', {'string'}}, {'=', 'returns', 'integer'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('CHARACTER_LENGTH', updates)
+    _func.index[2]:update('CHAR_LENGTH', updates)
+
+    updates = {{'=', 'param_list', {'string'}}, {'=', 'returns', 'integer'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_overload = true}}}
+    _func.index[2]:update('LENGTH', updates)
+
+    updates = {{'=', 'param_list', {'varbinary'}}, {'=', 'returns', 'integer'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {is_overloaded = true}}}
+    _func.index[2]:update('LENGTH_VARBINARY', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'scalar'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('COALESCE', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'integer'},
+               {'=', 'aggregate', 'group'}, {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('COUNT', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'scalar'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('GREATEST', updates)
+    _func.index[2]:update('LEAST', updates)
+
+    updates = {{'=', 'param_list', {'scalar', 'scalar'}},
+               {'=', 'returns', 'string'}, {'=', 'aggregate', 'group'},
+               {'=', 'exports', {'SQL'}}, {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('GROUP_CONCAT', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'string'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('HEX', updates)
+
+    updates = {{'=', 'param_list', {'scalar', 'scalar'}},
+               {'=', 'returns', 'scalar'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('IFNULL', updates)
+    _func.index[2]:update('NULLIF', updates)
+
+    updates = {{'=', 'param_list', {'string', 'string', 'string'}},
+               {'=', 'returns', 'boolean'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}, {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('LIKE', updates)
+
+    updates = {{'=', 'param_list', {'scalar', 'double'}},
+               {'=', 'returns', 'scalar'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('LIKELIHOOD', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'scalar'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('LIKELY', updates)
+    _func.index[2]:update('UNLIKELY', updates)
+
+    updates = {{'=', 'param_list', {'string'}}, {'=', 'returns', 'string'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('LOWER', updates)
+    _func.index[2]:update('SOUNDEX', updates)
+    _func.index[2]:update('UNICODE', updates)
+    _func.index[2]:update('UPPER', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'string'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('QUOTE', updates)
+    _func.index[2]:update('TYPEOF', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'scalar'},
+               {'=', 'aggregate', 'group'}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('MAX', updates)
+    _func.index[2]:update('MIN', updates)
+
+    updates = {{'=', 'param_list', {'string', 'string'}},
+               {'=', 'returns', 'integer'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}, {'=', 'opts', {has_overload = true}}}
+    _func.index[2]:update('POSITION', updates)
+
+    updates = {{'=', 'param_list', {'varbinary', 'varbinary'}},
+               {'=', 'returns', 'integer'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}, {'=', 'opts', {is_overloaded = true}}}
+    _func.index[2]:update('POSITION_VARBINARY', updates)
+
+    updates = {{'=', 'param_list', {'scalar'}}, {'=', 'returns', 'string'},
+               {'=', 'is_deterministic', true}, {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('PRINTF', updates)
+
+    updates = {{'=', 'param_list', {'string', 'unsigned', 'string'}},
+               {'=', 'returns', 'string'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true, has_overload = true}}}
+    _func.index[2]:update('TRIM', updates)
+
+    updates = {{'=', 'param_list', {'varbinary', 'unsigned', 'varbinary'}},
+               {'=', 'returns', 'string'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true, is_overloaded = true}}}
+    _func.index[2]:update('TRIM_VARBINARY', updates)
+
+    updates = {{'=', 'returns', 'integer'}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('RANDOM', updates)
+
+    updates = {{'=', 'returns', 'integer'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('ROW_COUNT', updates)
+
+    updates = {{'=', 'param_list', {'unsigned'}},
+               {'=', 'returns', 'varbinary'}, {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('RANDOMBLOB', updates)
+
+    updates = {{'=', 'param_list', {'unsigned'}},
+               {'=', 'returns', 'varbinary'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('ZEROBLOB', updates)
+
+    updates = {{'=', 'param_list', {'string', 'string', 'string'}},
+               {'=', 'returns', 'string'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('REPLACE', updates)
+
+    updates = {{'=', 'param_list', {'double', 'unsigned'}},
+               {'=', 'returns', 'double'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}, {'=', 'opts', {has_vararg = true}}}
+    _func.index[2]:update('ROUND', updates)
+
+    updates = {{'=', 'param_list', {'string', 'integer', 'integer'}},
+               {'=', 'returns', 'string'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true, has_overload = true}}}
+    _func.index[2]:update('SUBSTR', updates)
+
+    updates = {{'=', 'param_list', {'varbinary', 'integer', 'integer'}},
+               {'=', 'returns', 'string'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}},
+               {'=', 'opts', {has_vararg = true, is_overloaded = true}}}
+    _func.index[2]:update('SUBSTR_VARBINARY', updates)
+
+    updates = {{'=', 'returns', 'string'}, {'=', 'is_deterministic', true},
+               {'=', 'exports', {'SQL'}}}
+    _func.index[2]:update('VERSION', updates)
+    _func:run_triggers(true)
+end
+
+local function upgrade_to_2_5_2()
+    update_sql_builtin_functions()
+end
+
 --------------------------------------------------------------------------------
 
 local function get_version()
@@ -1007,6 +1198,7 @@ local function upgrade(options)
         {version = mkversion(2, 2, 1), func = upgrade_to_2_2_1, auto = true},
         {version = mkversion(2, 3, 0), func = upgrade_to_2_3_0, auto = true},
         {version = mkversion(2, 3, 1), func = upgrade_to_2_3_1, auto = true},
+        {version = mkversion(2, 5, 2), func = upgrade_to_2_5_2, auto = true},
     }
 
     for _, handler in ipairs(handlers) do
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index ae1842824..1fbffa535 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2971,12 +2971,6 @@ func_sql_builtin_new(struct func_def *def)
 	func->flags = sql_builtins[idx].flags;
 	func->call = sql_builtins[idx].call;
 	func->finalize = sql_builtins[idx].finalize;
-	def->param_count = sql_builtins[idx].param_count;
-	def->is_deterministic = sql_builtins[idx].is_deterministic;
-	def->returns = sql_builtins[idx].returns;
-	def->aggregate = sql_builtins[idx].aggregate;
-	def->exports.sql = sql_builtins[idx].export_to_sql;
-	def->opts.has_vararg = sql_builtins[idx].param_count == -1;
 	return &func->base;
 }
 
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index 0876e77a6..289289443 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -4,7 +4,7 @@ box.internal.bootstrap()
 box.space._schema:select{}
 ---
 - - ['max_id', 511]
-  - ['version', 2, 3, 1]
+  - ['version', 2, 5, 2]
 ...
 box.space._cluster:select{}
 ---
@@ -242,6 +242,10 @@ box.space._priv:select{}
   - [1, 2, 'function', 65, 4]
   - [1, 2, 'function', 66, 4]
   - [1, 2, 'function', 67, 4]
+  - [1, 2, 'function', 68, 4]
+  - [1, 2, 'function', 69, 4]
+  - [1, 2, 'function', 70, 4]
+  - [1, 2, 'function', 71, 4]
   - [1, 2, 'space', 276, 2]
   - [1, 2, 'space', 277, 1]
   - [1, 2, 'space', 281, 1]
diff --git a/test/box/access.result b/test/box/access.result
index 20b1b8b35..b72fa8b02 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -703,7 +703,7 @@ box.schema.func.exists(1)
 ---
 - true
 ...
-box.schema.func.exists(68)
+box.schema.func.exists(72)
 ---
 - false
 ...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 3e083a383..759d0a94f 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -282,7 +282,7 @@ box.schema.user.exists{}
 box.schema.func.exists('nosuchfunc')
 box.schema.func.exists('guest')
 box.schema.func.exists(1)
-box.schema.func.exists(68)
+box.schema.func.exists(72)
 box.schema.func.exists('box.schema.user.info')
 box.schema.func.exists()
 box.schema.func.exists(nil)
diff --git a/test/box/access_bin.result b/test/box/access_bin.result
index c58f331d3..d345e5996 100644
--- a/test/box/access_bin.result
+++ b/test/box/access_bin.result
@@ -298,7 +298,7 @@ box.schema.user.grant('guest', 'execute', 'universe')
 function f1() return box.space._func:get(1)[4] end
 ---
 ...
-function f2() return box.space._func:get(68)[4] end
+function f2() return box.space._func:get(72)[4] end
 ---
 ...
 box.schema.func.create('f1')
diff --git a/test/box/access_bin.test.lua b/test/box/access_bin.test.lua
index 41d5f4245..13d8041a9 100644
--- a/test/box/access_bin.test.lua
+++ b/test/box/access_bin.test.lua
@@ -112,7 +112,7 @@ test:drop()
 -- notice that guest can execute stuff, but can't read space _func
 box.schema.user.grant('guest', 'execute', 'universe')
 function f1() return box.space._func:get(1)[4] end
-function f2() return box.space._func:get(68)[4] end
+function f2() return box.space._func:get(72)[4] end
 box.schema.func.create('f1')
 box.schema.func.create('f2',{setuid=true})
 c = net.connect(box.cfg.listen)
diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result
index 799d19f03..f9ffae127 100644
--- a/test/box/access_sysview.result
+++ b/test/box/access_sysview.result
@@ -258,11 +258,11 @@ box.session.su('guest')
 ...
 #box.space._vpriv:select{}
 ---
-- 82
+- 86
 ...
 #box.space._vfunc:select{}
 ---
-- 67
+- 71
 ...
 #box.space._vcollation:select{}
 ---
@@ -290,11 +290,11 @@ box.session.su('guest')
 ...
 #box.space._vpriv:select{}
 ---
-- 82
+- 86
 ...
 #box.space._vfunc:select{}
 ---
-- 67
+- 71
 ...
 #box.space._vsequence:select{}
 ---
diff --git a/test/box/function1.result b/test/box/function1.result
index 928cd5758..3cce0f4e2 100644
--- a/test/box/function1.result
+++ b/test/box/function1.result
@@ -97,7 +97,7 @@ box.func["function1.args"]
   exports:
     lua: true
     sql: false
-  id: 68
+  id: 72
   setuid: false
   is_multikey: false
   is_deterministic: false
@@ -593,7 +593,7 @@ func
   exports:
     lua: true
     sql: false
-  id: 68
+  id: 72
   setuid: false
   is_multikey: false
   is_deterministic: false
@@ -665,7 +665,7 @@ func
   exports:
     lua: true
     sql: false
-  id: 68
+  id: 72
   setuid: false
   is_multikey: false
   is_deterministic: false
diff --git a/test/wal_off/func_max.result b/test/wal_off/func_max.result
index 78db38d6b..1aef99c66 100644
--- a/test/wal_off/func_max.result
+++ b/test/wal_off/func_max.result
@@ -42,11 +42,11 @@ test_run:cmd("setopt delimiter ''");
 ...
 func_limit()
 ---
-- error: 'Failed to create function ''func31934'': function id is too big'
+- error: 'Failed to create function ''func31930'': function id is too big'
 ...
 drop_limit_func()
 ---
-- error: Function 'func31934' does not exist
+- error: Function 'func31930' does not exist
 ...
 box.schema.user.create('testuser')
 ---
@@ -62,11 +62,11 @@ session.su('testuser')
 ...
 func_limit()
 ---
-- error: 'Failed to create function ''func31934'': function id is too big'
+- error: 'Failed to create function ''func31930'': function id is too big'
 ...
 drop_limit_func()
 ---
-- error: Function 'func31934' does not exist
+- error: Function 'func31930' does not exist
 ...
 session.su('admin')
 ---
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 08/10] box: add param_list to 'struct func'
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (6 preceding siblings ...)
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 07/10] sql: move built-in function definitions in _func imeevma
@ 2020-08-14 15:05 ` imeevma
  2020-08-22 14:30   ` Vladislav Shpilevoy
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 09/10] sql: check built-in functions argument types imeevma
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:05 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

This is needed to create an uniform way to check the types of arguments
of SQL built-in functions.

Part of #4159
---
 src/box/alter.cc   | 12 ++++++++++--
 src/box/func.c     |  1 +
 src/box/func_def.h |  2 ++
 src/box/lua/call.c |  2 ++
 src/box/sql/func.c |  1 +
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index ba96d9c62..0914a7615 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3293,7 +3293,11 @@ func_def_new_from_tuple(struct tuple *tuple)
 		diag_set(OutOfMemory, def_sz, "malloc", "def");
 		return NULL;
 	}
-	auto def_guard = make_scoped_guard([=] { free(def); });
+	def->param_list = NULL;
+	auto def_guard = make_scoped_guard([=] {
+		free(def->param_list);
+		free(def);
+	});
 	if (func_def_get_ids_from_tuple(tuple, &def->fid, &def->uid) != 0)
 		return NULL;
 	if (def->fid > BOX_FUNCTION_MAX) {
@@ -3403,6 +3407,8 @@ func_def_new_from_tuple(struct tuple *tuple)
 		if (param_list == NULL)
 			return NULL;
 		uint32_t argc = mp_decode_array(&param_list);
+		uint32_t size = sizeof(enum field_type) * argc;
+		def->param_list = (enum field_type *)malloc(size);
 		for (uint32_t i = 0; i < argc; i++) {
 			 if (mp_typeof(*param_list) != MP_STR) {
 				diag_set(ClientError, ER_FIELD_TYPE,
@@ -3412,7 +3418,8 @@ func_def_new_from_tuple(struct tuple *tuple)
 			}
 			uint32_t len;
 			const char *str = mp_decode_str(&param_list, &len);
-			if (STRN2ENUM(field_type, str, len) == field_type_MAX) {
+			def->param_list[i] = STRN2ENUM(field_type, str, len);
+			if (def->param_list[i] == field_type_MAX) {
 				diag_set(ClientError, ER_CREATE_FUNCTION,
 					  def->name, "invalid argument type");
 				return NULL;
@@ -3433,6 +3440,7 @@ func_def_new_from_tuple(struct tuple *tuple)
 		/* By default export to Lua, but not other frontends. */
 		def->exports.lua = true;
 		def->param_count = 0;
+		assert(def->param_list == NULL);
 	}
 	if (func_def_check(def) != 0)
 		return NULL;
diff --git a/src/box/func.c b/src/box/func.c
index 8087c953f..1d20f8872 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -510,6 +510,7 @@ func_c_destroy(struct func *base)
 {
 	assert(base->vtab == &func_c_vtab);
 	assert(base != NULL && base->def->language == FUNC_LANGUAGE_C);
+	free(base->def->param_list);
 	struct func_c *func = (struct func_c *) base;
 	func_c_unload(func);
 	TRASH(base);
diff --git a/src/box/func_def.h b/src/box/func_def.h
index bab2186da..712a5123b 100644
--- a/src/box/func_def.h
+++ b/src/box/func_def.h
@@ -129,6 +129,8 @@ struct func_def {
 	bool is_sandboxed;
 	/** The count of function's input arguments. */
 	int param_count;
+	/** List of input arguments to the function. */
+	enum field_type *param_list;
 	/** The type of the value returned by function. */
 	enum field_type returns;
 	/** Function aggregate option. */
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 0315e720c..1dc4589dd 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -783,6 +783,7 @@ func_lua_destroy(struct func *func)
 {
 	assert(func != NULL && func->def->language == FUNC_LANGUAGE_LUA);
 	assert(func->vtab == &func_lua_vtab);
+	free(func->def->param_list);
 	TRASH(func);
 	free(func);
 }
@@ -812,6 +813,7 @@ func_persistent_lua_destroy(struct func *base)
 	assert(base != NULL && base->def->language == FUNC_LANGUAGE_LUA &&
 	       base->def->body != NULL);
 	assert(base->vtab == &func_persistent_lua_vtab);
+	free(base->def->param_list);
 	struct func_lua *func = (struct func_lua *) base;
 	func_persistent_lua_unload(func);
 	free(func);
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 1fbffa535..c289f2de0 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2979,6 +2979,7 @@ func_sql_builtin_destroy(struct func *func)
 {
 	assert(func->vtab == &func_sql_builtin_vtab);
 	assert(func->def->language == FUNC_LANGUAGE_SQL_BUILTIN);
+	free(func->def->param_list);
 	free(func);
 }
 
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 09/10] sql: check built-in functions argument types
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (7 preceding siblings ...)
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 08/10] box: add param_list to 'struct func' imeevma
@ 2020-08-14 15:05 ` imeevma
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 10/10] sql: refactor sql/func.c imeevma
  2020-08-22 14:25 ` [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions Vladislav Shpilevoy
  10 siblings, 0 replies; 20+ messages in thread
From: imeevma @ 2020-08-14 15:05 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

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 the ApplyType opcode.

Closes #4159
---
 src/box/sql/expr.c             |    4 +
 src/box/sql/select.c           |   26 +
 src/box/sql/sqlInt.h           |   14 +
 test/sql-tap/cse.test.lua      |   24 +-
 test/sql-tap/func.test.lua     |   46 +-
 test/sql-tap/orderby1.test.lua |    2 +-
 test/sql-tap/position.test.lua |   16 +-
 test/sql-tap/substr.test.lua   |    2 +-
 test/sql/boolean.result        |   32 +-
 test/sql/checks.result         |    8 -
 test/sql/checks.test.lua       |    2 -
 test/sql/types.result          | 1535 +++++++++++++++++++++++++++++++-
 test/sql/types.test.lua        |  258 ++++++
 13 files changed, 1828 insertions(+), 141 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 99ca91bba..68b55f0e4 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -4134,6 +4134,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 9ff1dd3ff..38fa83df0 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/cse.test.lua b/test/sql-tap/cse.test.lua
index 341b6de01..18ddbf47c 100755
--- a/test/sql-tap/cse.test.lua
+++ b/test/sql-tap/cse.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(118)
+test:plan(116)
 
 --!./tcltestrunner.lua
 -- 2008 April 1
@@ -193,28 +193,6 @@ test:do_execsql_test(
         -- </cse-1.12>
     })
 
-
-
-test:do_execsql_test(
-    "cse-1.13",
-    [[
-        SELECT upper(b), typeof(b), b FROM t1
-    ]], {
-        -- <cse-1.13>
-        "11", "integer", 11, "21", "integer", 21
-        -- </cse-1.13>
-    })
-
-test:do_execsql_test(
-    "cse-1.14",
-    [[
-        SELECT b, typeof(b), upper(b), typeof(b), b FROM t1
-    ]], {
-        -- <cse-1.14>
-        11, "integer", "11", "integer", 11, 21, "integer", "21", "integer", 21
-        -- </cse-1.14>
-    })
-
 -- Overflow the column cache.  Create queries involving more and more
 -- columns until the cache overflows.  Verify correct operation throughout.
 --
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index 3c088920f..575a21007 100755
--- a/test/sql-tap/func.test.lua
+++ b/test/sql-tap/func.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(14694)
+test:plan(14693)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -95,7 +95,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-1.4",
     [[
-        SELECT coalesce(length(a),-1) FROM t2
+        SELECT coalesce(length(CAST(a AS STRING)),-1) FROM t2
     ]], {
         -- <func-1.4>
         1, -1, 3, -1, 5
@@ -197,7 +197,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-2.9",
     [[
-        SELECT substr(a,1,1) FROM t2
+        SELECT substr(CAST(a AS STRING),1,1) FROM t2
     ]], {
         -- <func-2.9>
         "1", "", "3", "", "6"
@@ -207,7 +207,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-2.10",
     [[
-        SELECT substr(a,2,2) FROM t2
+        SELECT substr(CAST(a AS STRING),2,2) FROM t2
     ]], {
         -- <func-2.10>
         "", "", "45", "", "78"
@@ -412,13 +412,13 @@ test:do_execsql_test(
         -- </func-4.4.1>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "func-4.4.2",
     [[
         SELECT abs(t1) FROM tbl1
     ]], {
         -- <func-4.4.2>
-        0.0, 0.0, 0.0, 0.0, 0.0
+        1, "Type mismatch: can not convert this to number"
         -- </func-4.4.2>
     })
 
@@ -502,13 +502,13 @@ test:do_execsql_test(
         -- </func-4.12>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "func-4.13",
     [[
         SELECT round(t1,2) FROM tbl1
     ]], {
         -- <func-4.13>
-        0.0, 0.0, 0.0, 0.0, 0.0
+        1, "Type mismatch: can not convert this to double"
         -- </func-4.13>
     })
 
@@ -760,18 +760,6 @@ test:do_execsql_test(
         -- </func-5.2>
     })
 
-test:do_execsql_test(
-    "func-5.3",
-    [[
-        SELECT upper(a), lower(a) FROM t2
-    ]], {
-        -- <func-5.3>
-        "1","1","","","345","345","","","67890","67890"
-        -- </func-5.3>
-    })
-
-
-
 test:do_catchsql_test(
     "func-5.5",
     [[
@@ -797,7 +785,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-6.2",
     [[
-        SELECT coalesce(upper(a),'nil') FROM t2
+        SELECT coalesce(upper(CAST(a AS STRING)),'nil') FROM t2
     ]], {
         -- <func-6.2>
         "1","nil","345","nil","67890"
@@ -893,7 +881,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)
     ]], {
         -- <func-8.5>
@@ -904,7 +892,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)
     ]], {
         -- <func-8.6>
@@ -915,7 +903,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)
     ]], {
         -- <func-8.7>
@@ -926,7 +914,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)
     ]], {
         -- <func-8.8>
@@ -985,7 +973,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))
     ]], {
         -- <func-9.5>
@@ -2918,7 +2906,7 @@ test:do_catchsql_test(
         SELECT ROUND(X'FF')
     ]], {
         -- <func-76.1>
-        1, "Type mismatch: can not convert varbinary to numeric"
+        1, "Type mismatch: can not convert varbinary to double"
         -- </func-76.1>
     })
 
@@ -2928,7 +2916,7 @@ test:do_catchsql_test(
         SELECT RANDOMBLOB(X'FF')
     ]], {
         -- <func-76.2>
-        1, "Type mismatch: can not convert varbinary to numeric"
+        1, "Type mismatch: can not convert varbinary to unsigned"
         -- </func-76.2>
     })
 
@@ -2938,7 +2926,7 @@ test:do_catchsql_test(
         SELECT SOUNDEX(X'FF')
     ]], {
         -- <func-76.3>
-        1, "Type mismatch: can not convert varbinary to text"
+        1, "Type mismatch: can not convert varbinary to string"
         -- </func-76.3>
     })
 
diff --git a/test/sql-tap/orderby1.test.lua b/test/sql-tap/orderby1.test.lua
index 51e8d301f..95a8de487 100755
--- a/test/sql-tap/orderby1.test.lua
+++ b/test/sql-tap/orderby1.test.lua
@@ -735,7 +735,7 @@ test:do_execsql_test(
         SELECT (
           SELECT 'hardware' FROM ( 
             SELECT 'software' ORDER BY 'firmware' ASC, 'sportswear' DESC
-          ) GROUP BY 1 HAVING length(b) <> 0
+          ) GROUP BY 1 HAVING length(CAST(b AS STRING)) <> 0
         )
         FROM abc;
     ]], {
diff --git a/test/sql-tap/position.test.lua b/test/sql-tap/position.test.lua
index e0455abc9..08fee3796 100755
--- a/test/sql-tap/position.test.lua
+++ b/test/sql-tap/position.test.lua
@@ -228,7 +228,7 @@ test:do_test(
         return test:catchsql "SELECT position(34, 12345);"
     end, {
         -- <position-1.23>
-        1, "Inconsistent types: expected text or varbinary got unsigned"
+        1, "Type mismatch: can not convert 34 to string"
         -- </position-1.23>
     })
 
@@ -238,7 +238,7 @@ test:do_test(
         return test:catchsql "SELECT position(34, 123456.78);"
     end, {
         -- <position-1.24>
-        1, "Inconsistent types: expected text or varbinary got real"
+        1, "Type mismatch: can not convert 34 to string"
         -- </position-1.24>
     })
 
@@ -248,7 +248,7 @@ test:do_test(
         return test:catchsql "SELECT position(x'3334', 123456.78);"
     end, {
         -- <position-1.25>
-        1, "Inconsistent types: expected text or varbinary got real"
+        1, "Type mismatch: can not convert 123456.78 to varbinary"
         -- </position-1.25>
     })
 
@@ -554,7 +554,7 @@ test:do_test(
         return test:catchsql("SELECT position('x', x'78c3a4e282ac79');")
     end, {
         -- <position-1.54>
-        1, "Inconsistent types: expected text got varbinary"
+        1, "Type mismatch: can not convert varbinary to string"
         -- </position-1.54>
     })
 
@@ -564,7 +564,7 @@ test:do_test(
         return test:catchsql "SELECT position('y', x'78c3a4e282ac79');"
     end, {
         -- <position-1.55>
-        1, "Inconsistent types: expected text got varbinary"
+        1, "Type mismatch: can not convert varbinary to string"
         -- </position-1.55>
     })
 
@@ -614,7 +614,7 @@ test:do_test(
         return test:catchsql "SELECT position(x'79', 'xä€y');"
     end, {
         -- <position-1.57.1>
-        1, "Inconsistent types: expected varbinary got text"
+        1, "Type mismatch: can not convert xä€y to varbinary"
         -- </position-1.57.1>
     })
 
@@ -624,7 +624,7 @@ test:do_test(
         return test:catchsql "SELECT position(x'a4', 'xä€y');"
     end, {
         -- <position-1.57.2>
-        1, "Inconsistent types: expected varbinary got text"
+        1, "Type mismatch: can not convert xä€y to varbinary"
         -- </position-1.57.2>
     })
 
@@ -634,7 +634,7 @@ test:do_test(
         return test:catchsql "SELECT position('y', x'78c3a4e282ac79');"
     end, {
         -- <position-1.57.3>
-        1, "Inconsistent types: expected text got varbinary"
+        1, "Type mismatch: can not convert varbinary to string"
         -- </position-1.57.3>
     })
 
diff --git a/test/sql-tap/substr.test.lua b/test/sql-tap/substr.test.lua
index a9e656e6d..a970f9e93 100755
--- a/test/sql-tap/substr.test.lua
+++ b/test/sql-tap/substr.test.lua
@@ -25,7 +25,7 @@ test:plan(93)
 --
 test:execsql [[
     CREATE TABLE t1(id integer primary key --autoincrement
-    , t text, b SCALAR)
+    , t text, b VARBINARY)
 ]]
 
 local function substr_test(id, string, i1, i2, result)
diff --git a/test/sql/boolean.result b/test/sql/boolean.result
index 51ec5820b..b525e908c 100644
--- a/test/sql/boolean.result
+++ b/test/sql/boolean.result
@@ -276,29 +276,17 @@ 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;
  | ---
- | - metadata:
- |   - name: COLUMN_1
- |     type: string
- |   rows:
- |   - ['false']
- |   - ['true']
- |   - [null]
- |   - [null]
+ | - null
+ | - 'Type mismatch: can not convert FALSE to string'
  | ...
 SELECT upper(a) FROM t0;
  | ---
- | - metadata:
- |   - name: COLUMN_1
- |     type: string
- |   rows:
- |   - ['FALSE']
- |   - ['TRUE']
- |   - [null]
- |   - [null]
+ | - null
+ | - 'Type mismatch: can not convert FALSE to string'
  | ...
 SELECT quote(a) FROM t0;
  | ---
@@ -314,14 +302,8 @@ SELECT quote(a) FROM t0;
 -- gh-4462: LENGTH didn't take BOOLEAN arguments.
 SELECT length(a) FROM t0;
  | ---
- | - metadata:
- |   - name: COLUMN_1
- |     type: integer
- |   rows:
- |   - [5]
- |   - [4]
- |   - [null]
- |   - [null]
+ | - null
+ | - 'Type mismatch: can not convert FALSE to string'
  | ...
 SELECT typeof(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..bd503c700 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -215,25 +215,22 @@ box.execute("INSERT INTO t1 VALUES (randomblob(5));")
 box.execute("SELECT * FROM t1 WHERE s LIKE 'blob';")
 ---
 - null
-- 'Inconsistent types: expected text got varbinary'
+- 'Type mismatch: can not convert varbinary to string'
 ...
 box.execute("SELECT * FROM t1 WHERE 'blob' LIKE s;")
 ---
 - null
-- 'Inconsistent types: expected text got varbinary'
+- 'Type mismatch: can not convert varbinary to string'
 ...
 box.execute("SELECT * FROM t1 WHERE 'blob' LIKE x'0000';")
 ---
 - null
-- 'Inconsistent types: expected text got varbinary'
+- 'Type mismatch: can not convert varbinary to string'
 ...
 box.execute("SELECT s LIKE NULL FROM t1;")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: boolean
-  rows:
-  - [null]
+- null
+- 'Type mismatch: can not convert varbinary to string'
 ...
 box.execute("DELETE FROM t1;")
 ---
@@ -246,20 +243,17 @@ box.execute("INSERT INTO t1 VALUES (1);")
 box.execute("SELECT * FROM t1 WHERE s LIKE 'int';")
 ---
 - null
-- 'Inconsistent types: expected text got unsigned'
+- 'Type mismatch: can not convert 1 to string'
 ...
 box.execute("SELECT * FROM t1 WHERE 'int' LIKE 4;")
 ---
 - null
-- 'Inconsistent types: expected text got unsigned'
+- 'Type mismatch: can not convert 4 to string'
 ...
 box.execute("SELECT NULL LIKE s FROM t1;")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: boolean
-  rows:
-  - [null]
+- null
+- 'Type mismatch: can not convert 1 to string'
 ...
 box.space.T1:drop()
 ---
@@ -830,19 +824,13 @@ box.execute("DELETE FROM t WHERE i < 18446744073709551613;")
 ...
 box.execute("SELECT lower(i) FROM t;")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: string
-  rows:
-  - ['18446744073709551613']
+- null
+- 'Type mismatch: can not convert 18446744073709551613 to string'
 ...
 box.execute("SELECT upper(i) FROM t;")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: string
-  rows:
-  - ['18446744073709551613']
+- null
+- 'Type mismatch: can not convert 18446744073709551613 to string'
 ...
 box.execute("SELECT abs(i) FROM t;")
 ---
@@ -1312,17 +1300,17 @@ box.execute("SELECT group_concat(v) FROM t;")
 box.execute("SELECT lower(v) FROM t;")
 ---
 - null
-- 'Inconsistent types: expected text got varbinary'
+- 'Type mismatch: can not convert varbinary to string'
 ...
 box.execute("SELECT upper(v) FROM t;")
 ---
 - null
-- 'Inconsistent types: expected text got varbinary'
+- 'Type mismatch: can not convert varbinary to string'
 ...
 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;")
 ---
@@ -1879,25 +1867,13 @@ box.execute("SELECT group_concat(d) FROM t;")
 ...
 box.execute("SELECT lower(d) FROM t;")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: string
-  rows:
-  - ['10.0']
-  - ['-2.0']
-  - ['3.3']
-  - ['1.8e+19']
+- null
+- 'Type mismatch: can not convert 10.0 to string'
 ...
 box.execute("SELECT upper(d) FROM t;")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: string
-  rows:
-  - ['10.0']
-  - ['-2.0']
-  - ['3.3']
-  - ['1.8E+19']
+- null
+- 'Type mismatch: can not convert 10.0 to string'
 ...
 box.execute("SELECT abs(d) FROM t;")
 ---
@@ -2807,3 +2783,1474 @@ box.execute([[SELECT typeof(length('abc'));]])
   rows:
   - ['integer']
 ...
+-- Make sure the function argument types are checked.
+box.execute([[SELECT abs(-1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1]
+...
+box.execute([[SELECT abs(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1]
+...
+box.execute([[SELECT abs(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT abs(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to number'
+...
+box.execute([[SELECT abs('a');]])
+---
+- null
+- 'Type mismatch: can not convert a to number'
+...
+box.execute([[SELECT abs(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to number'
+...
+box.execute([[SELECT char(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to unsigned'
+...
+box.execute([[SELECT char(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ["\x01"]
+...
+box.execute([[SELECT char(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ["\x01"]
+...
+box.execute([[SELECT char(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to unsigned'
+...
+box.execute([[SELECT char('a');]])
+---
+- null
+- 'Type mismatch: can not convert a to unsigned'
+...
+box.execute([[SELECT char(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to unsigned'
+...
+box.execute([[SELECT character_length(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT character_length(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT character_length(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT character_length(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT character_length('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT character_length(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[SELECT char_length(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT char_length(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT char_length(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT char_length(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT char_length('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT char_length(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[SELECT coalesce(-1, -1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [-1]
+...
+box.execute([[SELECT coalesce(1, 1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute([[SELECT coalesce(1.5, 1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT coalesce(true, true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [true]
+...
+box.execute([[SELECT coalesce('a', 'a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['a']
+...
+box.execute([[SELECT coalesce(X'33', X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['3']
+...
+box.execute([[SELECT greatest(-1, -1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [-1]
+...
+box.execute([[SELECT greatest(1, 1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute([[SELECT greatest(1.5, 1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT greatest(true, true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [true]
+...
+box.execute([[SELECT greatest('a', 'a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['a']
+...
+box.execute([[SELECT greatest(X'33', X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['3']
+...
+box.execute([[SELECT hex(-1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['2D31']
+...
+box.execute([[SELECT hex(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['31']
+...
+box.execute([[SELECT hex(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['312E35']
+...
+box.execute([[SELECT hex(true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['54525545']
+...
+box.execute([[SELECT hex('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['61']
+...
+box.execute([[SELECT hex(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['33']
+...
+box.execute([[SELECT ifnull(-1, -1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [-1]
+...
+box.execute([[SELECT ifnull(1, 1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute([[SELECT ifnull(1.5, 1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT ifnull(true, true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [true]
+...
+box.execute([[SELECT ifnull('a', 'a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['a']
+...
+box.execute([[SELECT ifnull(X'33', X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['3']
+...
+box.execute([[SELECT least(-1, -1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [-1]
+...
+box.execute([[SELECT least(1, 1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute([[SELECT least(1.5, 1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT least(true, true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [true]
+...
+box.execute([[SELECT least('a', 'a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['a']
+...
+box.execute([[SELECT least(X'33', X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['3']
+...
+box.execute([[SELECT length(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT length(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT length(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT length(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT length('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT length(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT likelihood(-1, -1);]])
+---
+- null
+- Illegal parameters, second argument to likelihood() must be a constant between 0.0
+  and 1.0
+...
+box.execute([[SELECT likelihood(1, 1);]])
+---
+- null
+- Illegal parameters, second argument to likelihood() must be a constant between 0.0
+  and 1.0
+...
+box.execute([[SELECT likelihood(1.5, 1.5);]])
+---
+- null
+- Illegal parameters, second argument to likelihood() must be a constant between 0.0
+  and 1.0
+...
+box.execute([[SELECT likelihood(true, true);]])
+---
+- null
+- Illegal parameters, second argument to likelihood() must be a constant between 0.0
+  and 1.0
+...
+box.execute([[SELECT likelihood('a', 'a');]])
+---
+- null
+- Illegal parameters, second argument to likelihood() must be a constant between 0.0
+  and 1.0
+...
+box.execute([[SELECT likelihood(X'33', X'33');]])
+---
+- null
+- Illegal parameters, second argument to likelihood() must be a constant between 0.0
+  and 1.0
+...
+box.execute([[SELECT likely(-1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [-1]
+...
+box.execute([[SELECT likely(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT likely(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: double
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT likely(true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: boolean
+  rows:
+  - [true]
+...
+box.execute([[SELECT likely('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['a']
+...
+box.execute([[SELECT likely(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: varbinary
+  rows:
+  - ['3']
+...
+box.execute([[SELECT lower(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT lower(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT lower(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT lower(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT lower('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['a']
+...
+box.execute([[SELECT lower(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[SELECT nullif(-1, -1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [null]
+...
+box.execute([[SELECT nullif(1, 1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [null]
+...
+box.execute([[SELECT nullif(1.5, 1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [null]
+...
+box.execute([[SELECT nullif(true, true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [null]
+...
+box.execute([[SELECT nullif('a', 'a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [null]
+...
+box.execute([[SELECT nullif(X'33', X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [null]
+...
+box.execute([[SELECT position(-1, -1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT position(1, 1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT position(1.5, 1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT position(true, true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT position('a', 'a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT position(X'33', X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT printf(-1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['-1']
+...
+box.execute([[SELECT printf(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['1']
+...
+box.execute([[SELECT printf(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['1.5']
+...
+box.execute([[SELECT printf(true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['TRUE']
+...
+box.execute([[SELECT printf('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['a']
+...
+box.execute([[SELECT printf(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['3']
+...
+box.execute([[SELECT quote(-1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - [-1]
+...
+box.execute([[SELECT quote(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - [1]
+...
+box.execute([[SELECT quote(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['1.5']
+...
+box.execute([[SELECT quote(true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['TRUE']
+...
+box.execute([[SELECT quote('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['''a''']
+...
+box.execute([[SELECT quote(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['X''33''']
+...
+box.execute([[SELECT randomblob(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to unsigned'
+...
+box.execute([[SELECT randomblob(0);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: varbinary
+  rows:
+  - [null]
+...
+box.execute([[SELECT randomblob(0.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: varbinary
+  rows:
+  - [null]
+...
+box.execute([[SELECT randomblob(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to unsigned'
+...
+box.execute([[SELECT randomblob('a');]])
+---
+- null
+- 'Type mismatch: can not convert a to unsigned'
+...
+box.execute([[SELECT randomblob(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to unsigned'
+...
+box.execute([[SELECT replace(-1, -1, -1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT replace(1, 1, 1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT replace(1.5, 1.5, 1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT replace(true, true, true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT replace('a', 'a', 'a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['a']
+...
+box.execute([[SELECT replace(X'33', X'33', X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[SELECT round(-1, -1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to unsigned'
+...
+box.execute([[SELECT round(1, 1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: double
+  rows:
+  - [1]
+...
+box.execute([[SELECT round(1.5, 1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: double
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT round(true, true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to double'
+...
+box.execute([[SELECT round('a', 'a');]])
+---
+- null
+- 'Type mismatch: can not convert a to double'
+...
+box.execute([[SELECT round(X'33', X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to double'
+...
+box.execute([[SELECT soundex(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT soundex(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT soundex(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT soundex(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT soundex('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['A000']
+...
+box.execute([[SELECT soundex(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[SELECT substr(-1, -1, -1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT substr(1, 1, 1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT substr(1.5, 1.5, 1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT substr(true, true, true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT substr('a', 'a', 'a');]])
+---
+- null
+- 'Type mismatch: can not convert a to integer'
+...
+box.execute([[SELECT substr(X'33', X'33', X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to integer'
+...
+box.execute([[SELECT typeof(-1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['integer']
+...
+box.execute([[SELECT typeof(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['integer']
+...
+box.execute([[SELECT typeof(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['double']
+...
+box.execute([[SELECT typeof(true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['boolean']
+...
+box.execute([[SELECT typeof('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['string']
+...
+box.execute([[SELECT typeof(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['varbinary']
+...
+box.execute([[SELECT unicode(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT unicode(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT unicode(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT unicode(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT unicode('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - [97]
+...
+box.execute([[SELECT unicode(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[SELECT unlikely(-1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [-1]
+...
+box.execute([[SELECT unlikely(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT unlikely(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: double
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT unlikely(true);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: boolean
+  rows:
+  - [true]
+...
+box.execute([[SELECT unlikely('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['a']
+...
+box.execute([[SELECT unlikely(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: varbinary
+  rows:
+  - ['3']
+...
+box.execute([[SELECT upper(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT upper(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT upper(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT upper(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT upper('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['A']
+...
+box.execute([[SELECT upper(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[SELECT zeroblob(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to unsigned'
+...
+box.execute([[SELECT zeroblob(1);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: varbinary
+  rows:
+  - ["\0"]
+...
+box.execute([[SELECT zeroblob(1.5);]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: varbinary
+  rows:
+  - ["\0"]
+...
+box.execute([[SELECT zeroblob(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to unsigned'
+...
+box.execute([[SELECT zeroblob('a');]])
+---
+- null
+- 'Type mismatch: can not convert a to unsigned'
+...
+box.execute([[SELECT zeroblob(X'33');]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to unsigned'
+...
+box.execute([[SELECT trim(-1);]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT trim(1);]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT trim(1.5);]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT trim(true);]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT trim('a');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['a']
+...
+box.execute([[SELECT trim(X'33');]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['3']
+...
+box.execute([[SELECT -1 like -1;]])
+---
+- null
+- 'Type mismatch: can not convert -1 to string'
+...
+box.execute([[SELECT 1 like 1;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to string'
+...
+box.execute([[SELECT 1.5 like 1.5;]])
+---
+- null
+- 'Type mismatch: can not convert 1.5 to string'
+...
+box.execute([[SELECT true like true;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to string'
+...
+box.execute([[SELECT 'a' like 'a';]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: boolean
+  rows:
+  - [true]
+...
+box.execute([[SELECT X'33' like X'33';]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to string'
+...
+box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, u UNSIGNED, d DOUBLE, b BOOLEAN, s STRING, v VARBINARY);]])
+---
+- row_count: 1
+...
+box.execute([[INSERT INTO t VALUES (-1, 1, 1.5, true, 'a', X'33');]])
+---
+- row_count: 1
+...
+box.execute([[SELECT avg(i) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [-1]
+...
+box.execute([[SELECT avg(u) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1]
+...
+box.execute([[SELECT avg(d) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT avg(b) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to number'
+...
+box.execute([[SELECT avg(s) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert a to number'
+...
+box.execute([[SELECT avg(v) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to number'
+...
+box.execute([[SELECT count(i) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT count(u) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT count(d) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT count(b) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT count(s) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT count(v) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: integer
+  rows:
+  - [1]
+...
+box.execute([[SELECT group_concat(i) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['-1']
+...
+box.execute([[SELECT group_concat(u) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['1']
+...
+box.execute([[SELECT group_concat(d) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['1.5']
+...
+box.execute([[SELECT group_concat(b) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['TRUE']
+...
+box.execute([[SELECT group_concat(s) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['a']
+...
+box.execute([[SELECT group_concat(v) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: string
+  rows:
+  - ['3']
+...
+box.execute([[SELECT max(i) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [-1]
+...
+box.execute([[SELECT max(u) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute([[SELECT max(d) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT max(b) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [true]
+...
+box.execute([[SELECT max(s) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['a']
+...
+box.execute([[SELECT max(v) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['3']
+...
+box.execute([[SELECT min(i) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [-1]
+...
+box.execute([[SELECT min(u) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute([[SELECT min(d) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT min(b) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - [true]
+...
+box.execute([[SELECT min(s) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['a']
+...
+box.execute([[SELECT min(v) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: scalar
+  rows:
+  - ['3']
+...
+box.execute([[SELECT sum(i) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [-1]
+...
+box.execute([[SELECT sum(u) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1]
+...
+box.execute([[SELECT sum(d) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT sum(b) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to number'
+...
+box.execute([[SELECT sum(s) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert a to number'
+...
+box.execute([[SELECT sum(v) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to number'
+...
+box.execute([[SELECT total(i) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [-1]
+...
+box.execute([[SELECT total(u) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1]
+...
+box.execute([[SELECT total(d) FROM t;]])
+---
+- metadata:
+  - name: COLUMN_1
+    type: number
+  rows:
+  - [1.5]
+...
+box.execute([[SELECT total(b) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert TRUE to number'
+...
+box.execute([[SELECT total(s) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert a to number'
+...
+box.execute([[SELECT total(v) FROM t;]])
+---
+- null
+- 'Type mismatch: can not convert varbinary to number'
+...
+box.execute([[DROP TABLE t;]])
+---
+- row_count: 1
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index fff0057bd..61483e7e9 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -629,3 +629,261 @@ box.execute([[DROP TABLE ts;]])
 -- instead of values of type UNSIGNED.
 --
 box.execute([[SELECT typeof(length('abc'));]])
+
+-- Make sure the function argument types are checked.
+box.execute([[SELECT abs(-1);]])
+box.execute([[SELECT abs(1);]])
+box.execute([[SELECT abs(1.5);]])
+box.execute([[SELECT abs(true);]])
+box.execute([[SELECT abs('a');]])
+box.execute([[SELECT abs(X'33');]])
+
+box.execute([[SELECT char(-1);]])
+box.execute([[SELECT char(1);]])
+box.execute([[SELECT char(1.5);]])
+box.execute([[SELECT char(true);]])
+box.execute([[SELECT char('a');]])
+box.execute([[SELECT char(X'33');]])
+
+box.execute([[SELECT character_length(-1);]])
+box.execute([[SELECT character_length(1);]])
+box.execute([[SELECT character_length(1.5);]])
+box.execute([[SELECT character_length(true);]])
+box.execute([[SELECT character_length('a');]])
+box.execute([[SELECT character_length(X'33');]])
+
+box.execute([[SELECT char_length(-1);]])
+box.execute([[SELECT char_length(1);]])
+box.execute([[SELECT char_length(1.5);]])
+box.execute([[SELECT char_length(true);]])
+box.execute([[SELECT char_length('a');]])
+box.execute([[SELECT char_length(X'33');]])
+
+box.execute([[SELECT coalesce(-1, -1);]])
+box.execute([[SELECT coalesce(1, 1);]])
+box.execute([[SELECT coalesce(1.5, 1.5);]])
+box.execute([[SELECT coalesce(true, true);]])
+box.execute([[SELECT coalesce('a', 'a');]])
+box.execute([[SELECT coalesce(X'33', X'33');]])
+
+box.execute([[SELECT greatest(-1, -1);]])
+box.execute([[SELECT greatest(1, 1);]])
+box.execute([[SELECT greatest(1.5, 1.5);]])
+box.execute([[SELECT greatest(true, true);]])
+box.execute([[SELECT greatest('a', 'a');]])
+box.execute([[SELECT greatest(X'33', X'33');]])
+
+box.execute([[SELECT hex(-1);]])
+box.execute([[SELECT hex(1);]])
+box.execute([[SELECT hex(1.5);]])
+box.execute([[SELECT hex(true);]])
+box.execute([[SELECT hex('a');]])
+box.execute([[SELECT hex(X'33');]])
+
+box.execute([[SELECT ifnull(-1, -1);]])
+box.execute([[SELECT ifnull(1, 1);]])
+box.execute([[SELECT ifnull(1.5, 1);]])
+box.execute([[SELECT ifnull(true, true);]])
+box.execute([[SELECT ifnull('a', 'a');]])
+box.execute([[SELECT ifnull(X'33', X'33');]])
+
+box.execute([[SELECT least(-1, -1);]])
+box.execute([[SELECT least(1, 1);]])
+box.execute([[SELECT least(1.5, 1.5);]])
+box.execute([[SELECT least(true, true);]])
+box.execute([[SELECT least('a', 'a');]])
+box.execute([[SELECT least(X'33', X'33');]])
+
+box.execute([[SELECT length(-1);]])
+box.execute([[SELECT length(1);]])
+box.execute([[SELECT length(1.5);]])
+box.execute([[SELECT length(true);]])
+box.execute([[SELECT length('a');]])
+box.execute([[SELECT length(X'33');]])
+
+box.execute([[SELECT likelihood(-1, -1);]])
+box.execute([[SELECT likelihood(1, 1);]])
+box.execute([[SELECT likelihood(1.5, 1.5);]])
+box.execute([[SELECT likelihood(true, true);]])
+box.execute([[SELECT likelihood('a', 'a');]])
+box.execute([[SELECT likelihood(X'33', X'33');]])
+
+box.execute([[SELECT likely(-1);]])
+box.execute([[SELECT likely(1);]])
+box.execute([[SELECT likely(1.5);]])
+box.execute([[SELECT likely(true);]])
+box.execute([[SELECT likely('a');]])
+box.execute([[SELECT likely(X'33');]])
+
+box.execute([[SELECT lower(-1);]])
+box.execute([[SELECT lower(1);]])
+box.execute([[SELECT lower(1.5);]])
+box.execute([[SELECT lower(true);]])
+box.execute([[SELECT lower('a');]])
+box.execute([[SELECT lower(X'33');]])
+
+box.execute([[SELECT nullif(-1, -1);]])
+box.execute([[SELECT nullif(1, 1);]])
+box.execute([[SELECT nullif(1.5, 1.5);]])
+box.execute([[SELECT nullif(true, true);]])
+box.execute([[SELECT nullif('a', 'a');]])
+box.execute([[SELECT nullif(X'33', X'33');]])
+
+box.execute([[SELECT position(-1, -1);]])
+box.execute([[SELECT position(1, 1);]])
+box.execute([[SELECT position(1.5, 1.5);]])
+box.execute([[SELECT position(true, true);]])
+box.execute([[SELECT position('a', 'a');]])
+box.execute([[SELECT position(X'33', X'33');]])
+
+box.execute([[SELECT printf(-1);]])
+box.execute([[SELECT printf(1);]])
+box.execute([[SELECT printf(1.5);]])
+box.execute([[SELECT printf(true);]])
+box.execute([[SELECT printf('a');]])
+box.execute([[SELECT printf(X'33');]])
+
+box.execute([[SELECT quote(-1);]])
+box.execute([[SELECT quote(1);]])
+box.execute([[SELECT quote(1.5);]])
+box.execute([[SELECT quote(true);]])
+box.execute([[SELECT quote('a');]])
+box.execute([[SELECT quote(X'33');]])
+
+box.execute([[SELECT randomblob(-1);]])
+box.execute([[SELECT randomblob(0);]])
+box.execute([[SELECT randomblob(0.5);]])
+box.execute([[SELECT randomblob(true);]])
+box.execute([[SELECT randomblob('a');]])
+box.execute([[SELECT randomblob(X'33');]])
+
+box.execute([[SELECT replace(-1, -1, -1);]])
+box.execute([[SELECT replace(1, 1, 1);]])
+box.execute([[SELECT replace(1.5, 1.5, 1.5);]])
+box.execute([[SELECT replace(true, true, true);]])
+box.execute([[SELECT replace('a', 'a', 'a');]])
+box.execute([[SELECT replace(X'33', X'33', X'33');]])
+
+box.execute([[SELECT round(-1, -1);]])
+box.execute([[SELECT round(1, 1);]])
+box.execute([[SELECT round(1.5, 1.5);]])
+box.execute([[SELECT round(true, true);]])
+box.execute([[SELECT round('a', 'a');]])
+box.execute([[SELECT round(X'33', X'33');]])
+
+box.execute([[SELECT soundex(-1);]])
+box.execute([[SELECT soundex(1);]])
+box.execute([[SELECT soundex(1.5);]])
+box.execute([[SELECT soundex(true);]])
+box.execute([[SELECT soundex('a');]])
+box.execute([[SELECT soundex(X'33');]])
+
+box.execute([[SELECT substr(-1, -1, -1);]])
+box.execute([[SELECT substr(1, 1, 1);]])
+box.execute([[SELECT substr(1.5, 1.5, 1.5);]])
+box.execute([[SELECT substr(true, true, true);]])
+box.execute([[SELECT substr('a', 'a', 'a');]])
+box.execute([[SELECT substr(X'33', X'33', X'33');]])
+
+box.execute([[SELECT typeof(-1);]])
+box.execute([[SELECT typeof(1);]])
+box.execute([[SELECT typeof(1.5);]])
+box.execute([[SELECT typeof(true);]])
+box.execute([[SELECT typeof('a');]])
+box.execute([[SELECT typeof(X'33');]])
+
+box.execute([[SELECT unicode(-1);]])
+box.execute([[SELECT unicode(1);]])
+box.execute([[SELECT unicode(1.5);]])
+box.execute([[SELECT unicode(true);]])
+box.execute([[SELECT unicode('a');]])
+box.execute([[SELECT unicode(X'33');]])
+
+box.execute([[SELECT unlikely(-1);]])
+box.execute([[SELECT unlikely(1);]])
+box.execute([[SELECT unlikely(1.5);]])
+box.execute([[SELECT unlikely(true);]])
+box.execute([[SELECT unlikely('a');]])
+box.execute([[SELECT unlikely(X'33');]])
+
+box.execute([[SELECT upper(-1);]])
+box.execute([[SELECT upper(1);]])
+box.execute([[SELECT upper(1.5);]])
+box.execute([[SELECT upper(true);]])
+box.execute([[SELECT upper('a');]])
+box.execute([[SELECT upper(X'33');]])
+
+box.execute([[SELECT zeroblob(-1);]])
+box.execute([[SELECT zeroblob(1);]])
+box.execute([[SELECT zeroblob(1.5);]])
+box.execute([[SELECT zeroblob(true);]])
+box.execute([[SELECT zeroblob('a');]])
+box.execute([[SELECT zeroblob(X'33');]])
+
+box.execute([[SELECT trim(-1);]])
+box.execute([[SELECT trim(1);]])
+box.execute([[SELECT trim(1.5);]])
+box.execute([[SELECT trim(true);]])
+box.execute([[SELECT trim('a');]])
+box.execute([[SELECT trim(X'33');]])
+
+box.execute([[SELECT -1 like -1;]])
+box.execute([[SELECT 1 like 1;]])
+box.execute([[SELECT 1.5 like 1.5;]])
+box.execute([[SELECT true like true;]])
+box.execute([[SELECT 'a' like 'a';]])
+box.execute([[SELECT X'33' like X'33';]])
+
+box.execute([[CREATE TABLE t (i INTEGER PRIMARY KEY, u UNSIGNED, d DOUBLE, b BOOLEAN, s STRING, v VARBINARY);]])
+box.execute([[INSERT INTO t VALUES (-1, 1, 1.5, true, 'a', X'33');]])
+
+box.execute([[SELECT avg(i) FROM t;]])
+box.execute([[SELECT avg(u) FROM t;]])
+box.execute([[SELECT avg(d) FROM t;]])
+box.execute([[SELECT avg(b) FROM t;]])
+box.execute([[SELECT avg(s) FROM t;]])
+box.execute([[SELECT avg(v) FROM t;]])
+
+box.execute([[SELECT count(i) FROM t;]])
+box.execute([[SELECT count(u) FROM t;]])
+box.execute([[SELECT count(d) FROM t;]])
+box.execute([[SELECT count(b) FROM t;]])
+box.execute([[SELECT count(s) FROM t;]])
+box.execute([[SELECT count(v) FROM t;]])
+
+box.execute([[SELECT group_concat(i) FROM t;]])
+box.execute([[SELECT group_concat(u) FROM t;]])
+box.execute([[SELECT group_concat(d) FROM t;]])
+box.execute([[SELECT group_concat(b) FROM t;]])
+box.execute([[SELECT group_concat(s) FROM t;]])
+box.execute([[SELECT group_concat(v) FROM t;]])
+
+box.execute([[SELECT max(i) FROM t;]])
+box.execute([[SELECT max(u) FROM t;]])
+box.execute([[SELECT max(d) FROM t;]])
+box.execute([[SELECT max(b) FROM t;]])
+box.execute([[SELECT max(s) FROM t;]])
+box.execute([[SELECT max(v) FROM t;]])
+
+box.execute([[SELECT min(i) FROM t;]])
+box.execute([[SELECT min(u) FROM t;]])
+box.execute([[SELECT min(d) FROM t;]])
+box.execute([[SELECT min(b) FROM t;]])
+box.execute([[SELECT min(s) FROM t;]])
+box.execute([[SELECT min(v) FROM t;]])
+
+box.execute([[SELECT sum(i) FROM t;]])
+box.execute([[SELECT sum(u) FROM t;]])
+box.execute([[SELECT sum(d) FROM t;]])
+box.execute([[SELECT sum(b) FROM t;]])
+box.execute([[SELECT sum(s) FROM t;]])
+box.execute([[SELECT sum(v) FROM t;]])
+
+box.execute([[SELECT total(i) FROM t;]])
+box.execute([[SELECT total(u) FROM t;]])
+box.execute([[SELECT total(d) FROM t;]])
+box.execute([[SELECT total(b) FROM t;]])
+box.execute([[SELECT total(s) FROM t;]])
+box.execute([[SELECT total(v) FROM t;]])
+
+box.execute([[DROP TABLE t;]])
\ No newline at end of file
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Tarantool-patches] [PATCH v2 10/10] sql: refactor sql/func.c
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (8 preceding siblings ...)
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 09/10] sql: check built-in functions argument types imeevma
@ 2020-08-14 15:05 ` imeevma
  2020-08-22 14:31   ` Vladislav Shpilevoy
  2020-08-22 14:25 ` [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions Vladislav Shpilevoy
  10 siblings, 1 reply; 20+ messages in thread
From: imeevma @ 2020-08-14 15:05 UTC (permalink / raw)
  To: v.shpilevoy, tsafin; +Cc: tarantool-patches

After changing the way of checking the types of arguments, some of the
code in sql/func.c is no longer used. This patch removes this code.

Follow-up of #4159
---
 src/box/sql/func.c | 841 +++++----------------------------------------
 1 file changed, 87 insertions(+), 754 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index c289f2de0..de1e4d13d 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -504,44 +504,21 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
 {
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
-	switch (sql_value_type(argv[0])) {
-	case MP_UINT: {
-		sql_result_uint(context, sql_value_uint64(argv[0]));
-		break;
-	}
-	case MP_INT: {
+	enum mp_type type = sql_value_type(argv[0]);
+	if (type == MP_NIL)
+		return sql_result_null(context);
+	if (type == MP_UINT)
+		return sql_result_uint(context, sql_value_uint64(argv[0]));
+	if (type == MP_INT) {
 		int64_t value = sql_value_int64(argv[0]);
 		assert(value < 0);
-		sql_result_uint(context, -value);
-		break;
-	}
-	case MP_NIL:{
-			/* IMP: R-37434-19929 Abs(X) returns NULL if X is NULL. */
-			sql_result_null(context);
-			break;
-		}
-	case MP_BOOL:
-	case MP_BIN:
-	case MP_ARRAY:
-	case MP_MAP: {
-		diag_set(ClientError, ER_INCONSISTENT_TYPES, "number",
-			 mem_type_to_str(argv[0]));
-		context->is_aborted = true;
-		return;
-	}
-	default:{
-			/* Because sql_value_double() returns 0.0 if the argument is not
-			 * something that can be converted into a number, we have:
-			 * IMP: R-01992-00519 Abs(X) returns 0.0 if X is a string or blob
-			 * that cannot be converted to a numeric value.
-			 */
-			double rVal = sql_value_double(argv[0]);
-			if (rVal < 0)
-				rVal = -rVal;
-			sql_result_double(context, rVal);
-			break;
-		}
+		return sql_result_uint(context, -value);
 	}
+	assert(type == MP_DOUBLE);
+	double value = sql_value_double(argv[0]);
+	if (value < 0)
+		value = -value;
+	return sql_result_double(context, value);
 }
 
 /**
@@ -839,19 +816,14 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
 	if (argc == 2) {
 		if (sql_value_is_null(argv[1]))
 			return;
+		assert(sql_value_type(argv[1]) == MP_UINT);
 		n = sql_value_int(argv[1]);
 		if (n < 0)
 			n = 0;
 	}
 	if (sql_value_is_null(argv[0]))
 		return;
-	enum mp_type mp_type = sql_value_type(argv[0]);
-	if (mp_type_is_bloblike(mp_type)) {
-		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_to_diag_str(argv[0]), "numeric");
-		context->is_aborted = true;
-		return;
-	}
+	assert(sql_value_type(argv[0]) == MP_DOUBLE);
 	r = sql_value_double(argv[0]);
 	/* If Y==0 and X will fit in a 64-bit int,
 	 * handle the rounding directly,
@@ -988,12 +960,7 @@ randomBlob(sql_context * context, int argc, sql_value ** argv)
 	unsigned char *p;
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
-	if (mp_type_is_bloblike(sql_value_type(argv[0]))) {
-		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_to_diag_str(argv[0]), "numeric");
-		context->is_aborted = true;
-		return;
-	}
+	assert(sql_value_type(argv[0]) == MP_UINT);
 	n = sql_value_int(argv[0]);
 	if (n < 1)
 		return;
@@ -1538,6 +1505,7 @@ zeroblobFunc(sql_context * context, int argc, sql_value ** argv)
 	i64 n;
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
+	assert(sql_value_type(argv[0]) == MP_UINT);
 	n = sql_value_int64(argv[0]);
 	if (n < 0)
 		n = 0;
@@ -1943,18 +1911,10 @@ sum_step(struct sql_context *context, int argc, sql_value **argv)
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
 	struct SumCtx *p = sql_aggregate_context(context, sizeof(*p));
-	int type = sql_value_type(argv[0]);
+	enum mp_type type = sql_value_type(argv[0]);
 	if (type == MP_NIL || p == NULL)
 		return;
-	if (type != MP_DOUBLE && type != MP_INT && type != MP_UINT) {
-		if (mem_apply_numeric_type(argv[0]) != 0) {
-			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_to_diag_str(argv[0]), "number");
-			context->is_aborted = true;
-			return;
-		}
-		type = sql_value_type(argv[0]);
-	}
+	assert(type == MP_DOUBLE || type == MP_INT || type == MP_UINT);
 	p->cnt++;
 	if (type == MP_INT || type == MP_UINT) {
 		int64_t v = sql_value_int64(argv[0]);
@@ -2229,703 +2189,76 @@ static struct {
 	uint16_t flags;
 	void (*call)(sql_context *ctx, int argc, sql_value **argv);
 	void (*finalize)(sql_context *ctx);
-	/** Members below are related to struct func_def. */
-	bool is_deterministic;
-	int param_count;
-	enum field_type returns;
-	enum func_aggregate aggregate;
-	bool export_to_sql;
 } sql_builtins[] = {
-	{.name = "ABS",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_NUMBER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = absFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "AVG",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_NUMBER,
-	 .is_deterministic = false,
-	 .aggregate = FUNC_AGGREGATE_GROUP,
-	 .flags = 0,
-	 .call = sum_step,
-	 .finalize = avgFinalize,
-	 .export_to_sql = true,
-	}, {
-	 .name = "CEIL",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "CEILING",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "CHAR",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_STRING,
-	 .is_deterministic = true,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .flags = 0,
-	 .call = charFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	 }, {
-	 .name = "CHARACTER_LENGTH",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = lengthFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "CHAR_LENGTH",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = lengthFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "COALESCE",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_COALESCE,
-	 .call = sql_builtin_stub,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "COUNT",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_GROUP,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .call = countStep,
-	 .finalize = countFinalize,
-	 .export_to_sql = true,
-	}, {
-	 .name = "CURRENT_DATE",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "CURRENT_TIME",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "CURRENT_TIMESTAMP",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "DATE",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "DATETIME",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "EVERY",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "EXISTS",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "EXP",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "EXTRACT",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "FLOOR",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "GREATER",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "GREATEST",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_NEEDCOLL | SQL_FUNC_MAX,
-	 .call = minmaxFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "GROUP_CONCAT",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_GROUP,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .call = groupConcatStep,
-	 .finalize = groupConcatFinalize,
-	 .export_to_sql = true,
-	}, {
-	 .name = "HEX",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = hexFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "IFNULL",
-	 .param_count = 2,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_COALESCE,
-	 .call = sql_builtin_stub,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "JULIANDAY",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "LEAST",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_NEEDCOLL | SQL_FUNC_MIN,
-	 .call = minmaxFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "LENGTH",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_LENGTH,
-	 .call = lengthFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "LENGTH_VARBINARY",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_LENGTH,
-	 .call = lengthFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "LESSER",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "LIKE",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_BOOLEAN,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_NEEDCOLL | SQL_FUNC_LIKE,
-	 .call = likeFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "LIKELIHOOD",
-	 .param_count = 2,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_UNLIKELY,
-	 .call = sql_builtin_stub,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "LIKELY",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_UNLIKELY,
-	 .call = sql_builtin_stub,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "LN",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "LOWER",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_DERIVEDCOLL | SQL_FUNC_NEEDCOLL,
-	 .call = LowerICUFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "MAX",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_GROUP,
-	 .is_deterministic = false,
-	 .flags = SQL_FUNC_NEEDCOLL | SQL_FUNC_MAX,
-	 .call = minmaxStep,
-	 .finalize = minMaxFinalize,
-	 .export_to_sql = true,
-	}, {
-	 .name = "MIN",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_GROUP,
-	 .is_deterministic = false,
-	 .flags = SQL_FUNC_NEEDCOLL | SQL_FUNC_MIN,
-	 .call = minmaxStep,
-	 .finalize = minMaxFinalize,
-	 .export_to_sql = true,
-	}, {
-	 .name = "MOD",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "NULLIF",
-	 .param_count = 2,
-	 .returns = FIELD_TYPE_SCALAR,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_NEEDCOLL,
-	 .call = nullifFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "OCTET_LENGTH",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "POSITION",
-	 .param_count = 2,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_NEEDCOLL,
-	 .call = position_func,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "POSITION_VARBINARY",
-	 .param_count = 2,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = position_func,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "POWER",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "PRINTF",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = printfFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "QUOTE",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = quoteFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "RANDOM",
-	 .param_count = 0,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .call = randomFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "RANDOMBLOB",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_VARBINARY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .call = randomBlob,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "REPLACE",
-	 .param_count = 3,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_DERIVEDCOLL,
-	 .call = replaceFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "ROUND",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_DOUBLE,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = roundFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "ROW_COUNT",
-	 .param_count = 0,
-	 .returns = FIELD_TYPE_INTEGER,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = sql_row_count,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "SOME",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "SOUNDEX",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = soundexFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "SQRT",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "STRFTIME",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "SUBSTR",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_DERIVEDCOLL,
-	 .call = substrFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "SUBSTR_VARBINARY",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_DERIVEDCOLL,
-	 .call = substrFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "SUM",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_NUMBER,
-	 .aggregate = FUNC_AGGREGATE_GROUP,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .call = sum_step,
-	 .finalize = sumFinalize,
-	 .export_to_sql = true,
-	}, {
-	 .name = "TIME",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "TOTAL",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_NUMBER,
-	 .aggregate = FUNC_AGGREGATE_GROUP,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .call = sum_step,
-	 .finalize = totalFinalize,
-	 .export_to_sql = true,
-	}, {
-	 .name = "TRIM",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_DERIVEDCOLL,
-	 .call = trim_func,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "TRIM_VARBINARY",
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_DERIVEDCOLL,
-	 .call = trim_func,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "TYPEOF",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_TYPEOF,
-	 .call = typeofFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "UNICODE",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = unicodeFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "UNLIKELY",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_BOOLEAN,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_UNLIKELY,
-	 .call = sql_builtin_stub,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "UPPER",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = SQL_FUNC_DERIVEDCOLL | SQL_FUNC_NEEDCOLL,
-	 .call = UpperICUFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "VERSION",
-	 .param_count = 0,
-	 .returns = FIELD_TYPE_STRING,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = sql_func_version,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "ZEROBLOB",
-	 .param_count = 1,
-	 .returns = FIELD_TYPE_VARBINARY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = true,
-	 .flags = 0,
-	 .call = zeroblobFunc,
-	 .finalize = NULL,
-	 .export_to_sql = true,
-	}, {
-	 .name = "_sql_stat_get",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "_sql_stat_init",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	}, {
-	 .name = "_sql_stat_push",
-	 .call = sql_builtin_stub,
-	 .export_to_sql = false,
-	 .param_count = -1,
-	 .returns = FIELD_TYPE_ANY,
-	 .aggregate = FUNC_AGGREGATE_NONE,
-	 .is_deterministic = false,
-	 .flags = 0,
-	 .finalize = NULL,
-	},
+	{"ABS", 0, absFunc, NULL},
+	{"AVG", 0, sum_step, avgFinalize},
+	{"CEIL", 0, sql_builtin_stub, NULL},
+	{"CEILING", 0, sql_builtin_stub, NULL},
+	{"CHAR", 0, charFunc, NULL},
+	{"CHARACTER_LENGTH", 0, lengthFunc, NULL},
+	{"CHAR_LENGTH", 0, lengthFunc, NULL},
+	{"COALESCE", SQL_FUNC_COALESCE, sql_builtin_stub, NULL},
+	{"COUNT", 0, countStep, countFinalize},
+	{"CURRENT_DATE", 0, sql_builtin_stub, NULL},
+	{"CURRENT_TIME", 0, sql_builtin_stub, NULL},
+	{"CURRENT_TIMESTAMP", 0, sql_builtin_stub, NULL},
+	{"DATE", 0, sql_builtin_stub, NULL},
+	{"DATETIME", 0, sql_builtin_stub, NULL},
+	{"EVERY", 0, sql_builtin_stub, NULL},
+	{"EXISTS", 0, sql_builtin_stub, NULL},
+	{"EXP", 0, sql_builtin_stub, NULL},
+	{"EXTRACT", 0, sql_builtin_stub, NULL},
+	{"FLOOR", 0, sql_builtin_stub, NULL},
+	{"GREATER", 0, sql_builtin_stub, NULL},
+	{"GREATEST", SQL_FUNC_NEEDCOLL | SQL_FUNC_MAX, minmaxFunc, NULL},
+	{"GROUP_CONCAT", 0, groupConcatStep, groupConcatFinalize},
+	{"HEX", 0, hexFunc, NULL},
+	{"IFNULL", SQL_FUNC_COALESCE, sql_builtin_stub, NULL},
+	{"JULIANDAY", 0, sql_builtin_stub, NULL},
+	{"LEAST", SQL_FUNC_NEEDCOLL | SQL_FUNC_MIN, minmaxFunc, NULL},
+	{"LENGTH", SQL_FUNC_LENGTH, lengthFunc, NULL},
+	{"LENGTH_VARBINARY", SQL_FUNC_LENGTH, lengthFunc, NULL},
+	{"LESSER", 0, sql_builtin_stub, NULL},
+	{"LIKE", SQL_FUNC_NEEDCOLL | SQL_FUNC_LIKE, likeFunc, NULL},
+	{"LIKELIHOOD", SQL_FUNC_UNLIKELY, sql_builtin_stub, NULL},
+	{"LIKELY", SQL_FUNC_UNLIKELY, sql_builtin_stub, NULL},
+	{"LN", 0, sql_builtin_stub, NULL},
+	{"LOWER", SQL_FUNC_DERIVEDCOLL | SQL_FUNC_NEEDCOLL, LowerICUFunc, NULL},
+	{"MAX", SQL_FUNC_NEEDCOLL | SQL_FUNC_MAX, minmaxStep, minMaxFinalize},
+	{"MIN", SQL_FUNC_NEEDCOLL | SQL_FUNC_MIN, minmaxStep, minMaxFinalize},
+	{"MOD", 0, sql_builtin_stub, NULL},
+	{"NULLIF", SQL_FUNC_NEEDCOLL, nullifFunc, NULL},
+	{"OCTET_LENGTH", 0, sql_builtin_stub, NULL},
+	{"POSITION", SQL_FUNC_NEEDCOLL, position_func, NULL},
+	{"POSITION_VARBINARY", SQL_FUNC_NEEDCOLL, position_func, NULL},
+	{"POWER", 0, sql_builtin_stub, NULL},
+	{"PRINTF", 0, printfFunc, NULL},
+	{"QUOTE", 0, quoteFunc, NULL},
+	{"RANDOM", 0, randomFunc, NULL},
+	{"RANDOMBLOB", 0, randomBlob, NULL},
+	{"REPLACE", SQL_FUNC_DERIVEDCOLL, replaceFunc, NULL},
+	{"ROUND", 0, roundFunc, NULL},
+	{"ROW_COUNT", 0, sql_row_count, NULL},
+	{"SOME", 0, sql_builtin_stub, NULL},
+	{"SOUNDEX", 0, soundexFunc, NULL},
+	{"SQRT", 0, sql_builtin_stub, NULL},
+	{"STRFTIME", 0, sql_builtin_stub, NULL},
+	{"SUBSTR", SQL_FUNC_DERIVEDCOLL, substrFunc, NULL},
+	{"SUBSTR_VARBINARY", SQL_FUNC_DERIVEDCOLL, substrFunc, NULL},
+	{"SUM", 0, sum_step, sumFinalize},
+	{"TIME", 0, sql_builtin_stub, NULL},
+	{"TOTAL", 0, sum_step, totalFinalize},
+	{"TRIM", SQL_FUNC_DERIVEDCOLL, trim_func, NULL},
+	{"TRIM_VARBINARY", SQL_FUNC_DERIVEDCOLL, trim_func, NULL},
+	{"TYPEOF", SQL_FUNC_TYPEOF, typeofFunc, NULL},
+	{"UNICODE", 0, unicodeFunc, NULL},
+	{"UNLIKELY", SQL_FUNC_UNLIKELY, sql_builtin_stub, NULL},
+	{"UPPER", SQL_FUNC_DERIVEDCOLL | SQL_FUNC_NEEDCOLL, UpperICUFunc, NULL},
+	{"VERSION", 0, sql_func_version, NULL},
+	{"ZEROBLOB", 0, zeroblobFunc, NULL},
+	{"_sql_stat_get", 0, sql_builtin_stub, NULL},
+	{"_sql_stat_init", 0, sql_builtin_stub, NULL},
+	{"_sql_stat_push", 0, sql_builtin_stub, NULL},
 };
 
 static struct func_vtab func_sql_builtin_vtab;
-- 
2.25.1

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 01/10] sql: do not return UNSIGNED in built-in functions
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 01/10] sql: do not return UNSIGNED in " imeevma
@ 2020-08-22 14:23   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:23 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Hi! Thanks for the patch!

On 14.08.2020 17:04, imeevma@tarantool.org wrote:
> This patch forces functions to return INTEGER instead of UNSIGNED.

Why? I don't see a reason why length() would need to return int instead
of uint.

> diff --git a/test/sql/types.result b/test/sql/types.result
> index 442245186..95f7713e8 100644
> --- a/test/sql/types.result
> +++ b/test/sql/types.result
> @@ -2795,3 +2795,15 @@ box.execute([[DROP TABLE ts;]])
>  ---
>  - row_count: 1
>  ...
> +--
> +-- gh-4159: Make sure that functions returns values of type INTEGER
> +-- instead of values of type UNSIGNED.

What are the other functions besides length()?

> +--
> +box.execute([[SELECT typeof(length('abc'));]])

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 02/10] sql: fix functions return types
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 02/10] sql: fix functions return types imeevma
@ 2020-08-22 14:24   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:24 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patch!

On 14.08.2020 17:04, imeevma@tarantool.org wrote:
> This patch fixes incorrect return types in SQL built-in function definitions.

Why are they incorrect? You need to provide more descriptive message.
It took for me some time to scan the sqlite doc to find what do these
functions do and why do you change their types.

> ---
>  src/box/sql/func.c    | 10 +++++-----
>  test/sql/types.result |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 487cdafe1..affb285aa 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
"UNLIKELY" still returns boolean. But according to the doc, it
should return the single argument unchanged. So also should be
SCALAR.

Please, check the other functions too.

Also you need to add tests on the changed types. For example,
you changed likely() return type, but I don't see any tests
failing. So looks like it is not covered.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions
  2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
                   ` (9 preceding siblings ...)
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 10/10] sql: refactor sql/func.c imeevma
@ 2020-08-22 14:25 ` Vladislav Shpilevoy
  10 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:25 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patchset!

Should there be a docbot request somewhere? It seems the
visible behaviour is changed.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 03/10] sql: change signature of trim()
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 03/10] sql: change signature of trim() imeevma
@ 2020-08-22 14:26   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:26 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patch!

On 14.08.2020 17:04, imeevma@tarantool.org wrote:
> This patch changes the signature of the SQL built-in trim() function.
> This makes it easier to define a function in _func and fixes a bug where
> the function loses collation when the BOTH, LEADING, or TRAILING
> keywords are specified.

I am not sure I understand. Did you break the backward compatibility by
changing the public function? Or did you change only internal implementation?
What exactly has changed? Where is refactoring, and where is the bugfix?
What is so hard about its signature now, that it does not allow to define it
in _func?

If this is a bugfix, it should be on a separate branch, with changelog,
so as it could be pushed to the older versions, according to our policy.

> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index affb285aa..e5da21191 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -1776,32 +1772,30 @@ static void
>  trim_func_two_args(struct sql_context *context, sql_value *arg1,
>  		   sql_value *arg2)
>  {
> -	const unsigned char *input_str, *trim_set;
> -	if ((input_str = sql_value_text(arg2)) == NULL)
> -		return;
> -
> -	int input_str_sz = sql_value_bytes(arg2);
> -	if (sql_value_type(arg1) == MP_INT || sql_value_type(arg1) == MP_UINT) {
> -		uint8_t len_one = 1;
> -		trim_procedure(context, sql_value_int(arg1),
> -			       (const unsigned char *) " ", &len_one, 1,
> -			       input_str, input_str_sz);
> -	} else if ((trim_set = sql_value_text(arg1)) != NULL) {
> -		int trim_set_sz = sql_value_bytes(arg1);
> -		uint8_t *char_len;
> -		int char_cnt = trim_prepare_char_len(context, trim_set,
> -						     trim_set_sz, &char_len);
> -		if (char_cnt == -1)
> -			return;
> -		trim_procedure(context, TRIM_BOTH, trim_set, char_len, char_cnt,
> -			       input_str, input_str_sz);
> -		sql_free(char_len);
> -	}
> +	assert(sql_value_type(arg2) == MP_UINT);
> +	enum mp_type type = sql_value_type(arg1);
> +	if (type == MP_NIL)
> +		return sql_result_null(context);
> +	const unsigned char *input_str = sql_value_text(arg1);
> +	const unsigned char *trim_set;
> +
> +	int input_str_sz = sql_value_bytes(arg1);
> +	uint8_t len_one = 1;
> +	if (type == MP_BIN)
> +		trim_set = (const unsigned char *) "\0";
> +	else
> +		trim_set = (const unsigned char *) " ";
> +	trim_procedure(context, sql_value_int(arg2), trim_set,  &len_one, 1,
> +		       input_str, input_str_sz);

Why did you move handling of the "TRIM(<character_set> FROM <str>)"
case to another function? That breaks the trim functions idea of having
trim_func_one_arg, trim_func_two_args, and trim_func_three_args separated.
After your patch trim_func_three_args handles both 2 and 3 arguments,
and trim_func_two_args handles not both options with 2 arguments.

You either need to keep the different argument count handling inside the
existing functions, or change their names somehow to reflect the new
behaviour.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 04/10] box: add new options for functions
  2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 04/10] box: add new options for functions imeevma
@ 2020-08-22 14:28   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:28 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patch!

On 14.08.2020 17:04, imeevma@tarantool.org wrote:
> The has_vararg option allows us to work with functions with a variable
> number of arguments. This is required for built-in SQL functions.
> 
> Suppose this option is TRUE for a built-in SQL function. Then:
> 1) If param_list is empty, all arguments can be of any type.
> 2) If the length of param_list is not less than the number of the given
> arguments, the types of the given arguments must be compatible with the
> corresponding types described in param_list.
> 3) If the length of param_list is less than the number of given
> arguments, the rest of the arguments must be compatible with the last
> type in param_list.

How is it going to be used? For example, how will it work with printf(),
where tail of the arguments can be of any type? Do you define it as
scalar then?

> The is_overloaded option allows us to deal with functions that can take
> STRING or VARBINARY arguments. In case the first of these arguments is
> of type VARBINARY, we use the overloaded version of the function. By
> default, we use the STRING version of the function.
> 
> The has_overload option indicates that the function has an overloaded
> version that accepts VARBINARY. See an explanation of the is_overloaded
> option.

In addition to this whole idea with overloads being a huge crutch which also
affects the public API with adding some new intricate options, it also looks
like has_overload is entirely a runtime option. Why is it even stored in
_func?

What if I have a function taking STRING, and I want to add VARBINARY, I
need to update 2 records in _func for that? Looks really inconvenient.

I would try to think more into how not to store these options in _func. Because
you are trying to describe some code from the executable's file using the
storage. From there most of the problems arise - new complicated options for
_func, hard upgrade process, and unnecessary changes of the schema.

Why can't you specify all these flags inside sql_builtins array, and
copy them into func_def from there? It already works fine this way.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions imeevma
@ 2020-08-22 14:29   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:29 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patch!

On 14.08.2020 17:05, imeevma@tarantool.org wrote:
> This patch adds overload function definitions for the length(),
> substr(), trim(), and position() functions.

You should describe how the overload works. How name collisions are
resolved.

Also I want to say I am against these 'overloads' for string/varbinary.
The code looks ugly with the new argument 'has_blob_arg' and with changing
the function name. Also it adds **2** new flags to think about, when you
work with the functions.

It should be either more generic with proper function names mangling like
in C++ (this is also bad), or it should not exist and the types should be
resolved inside single function taking both string and blob. Currently the
'overloaded' VARBINARY functions even return exactly the same types as their
non-VARBINARY versions (what is probably a bug - SUBSTR on blob returns
string?) anyway.

Or it should be separated into different functions defined explicitly.
So as user would be aware he needs to use LENGTH for strings, LENGTH_BIN
for byte length of blobs, etc. Otherwise you are trying to invent some
kind of implicit casts, but you cast functions instead of types. Which is
not better.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 07/10] sql: move built-in function definitions in _func
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 07/10] sql: move built-in function definitions in _func imeevma
@ 2020-08-22 14:30   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:30 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patch!

On 14.08.2020 17:05, imeevma@tarantool.org wrote:
> This patch moves SQL built-in function definitions to _func. This helps
> create an unified way to check the types of arguments.

I think this all can be done by fetching more info from sql_builtins array.
As I explained in one of the previous emails.

> It also allows
> users to see these definitions. Also, this patch enables overloading for
> length(), trim(), position() and substr() functions.

Tbh, I can't imagine what a monster user would need to look at these numerous
flags. See no point in exposing it into _func.

> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index ae1842824..1fbffa535 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -2971,12 +2971,6 @@ func_sql_builtin_new(struct func_def *def)
>  	func->flags = sql_builtins[idx].flags;
>  	func->call = sql_builtins[idx].call;
>  	func->finalize = sql_builtins[idx].finalize;
> -	def->param_count = sql_builtins[idx].param_count;
> -	def->is_deterministic = sql_builtins[idx].is_deterministic;
> -	def->returns = sql_builtins[idx].returns;
> -	def->aggregate = sql_builtins[idx].aggregate;
> -	def->exports.sql = sql_builtins[idx].export_to_sql;
> -	def->opts.has_vararg = sql_builtins[idx].param_count == -1;

I think this should be extended, not removed.

>  	return &func->base;
>  }
>  

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 08/10] box: add param_list to 'struct func'
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 08/10] box: add param_list to 'struct func' imeevma
@ 2020-08-22 14:30   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:30 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patch!

> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index ba96d9c62..0914a7615 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -3293,7 +3293,11 @@ func_def_new_from_tuple(struct tuple *tuple)
>  		diag_set(OutOfMemory, def_sz, "malloc", "def");
>  		return NULL;
>  	}
> -	auto def_guard = make_scoped_guard([=] { free(def); });
> +	def->param_list = NULL;
> +	auto def_guard = make_scoped_guard([=] {
> +		free(def->param_list);
> +		free(def);
> +	});

Would be better to patch func_def_sizeof() and allocate in one block.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 10/10] sql: refactor sql/func.c
  2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 10/10] sql: refactor sql/func.c imeevma
@ 2020-08-22 14:31   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 20+ messages in thread
From: Vladislav Shpilevoy @ 2020-08-22 14:31 UTC (permalink / raw)
  To: imeevma, tsafin; +Cc: tarantool-patches

Thanks for the patch!

See 3 comments below.

On 14.08.2020 17:05, imeevma@tarantool.org wrote:
> After changing the way of checking the types of arguments, some of the
> code in sql/func.c is no longer used. This patch removes this code.
> 
> Follow-up of #4159
> ---
>  src/box/sql/func.c | 841 +++++----------------------------------------
>  1 file changed, 87 insertions(+), 754 deletions(-)
> 
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index c289f2de0..de1e4d13d 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c

1. Why didn't you simplify some funtions?

- lengthFunc(). Is it allowed to pass numbers into it legally?
  Perhaps it is, I just don't remember.

- position_func(). It has a some complicated check of argument
  types. I think now you can make it a bit simpler.

- lower/upper(). They still check for non-blob argument.

- like(). Also still checks for non-str types.

> @@ -504,44 +504,21 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
>  {
>  	assert(argc == 1);
>  	UNUSED_PARAMETER(argc);
> -	switch (sql_value_type(argv[0])) {
> -	case MP_UINT: {
> -		sql_result_uint(context, sql_value_uint64(argv[0]));
> -		break;
> -	}
> -	case MP_INT: {
> +	enum mp_type type = sql_value_type(argv[0]);
> +	if (type == MP_NIL)
> +		return sql_result_null(context);
> +	if (type == MP_UINT)
> +		return sql_result_uint(context, sql_value_uint64(argv[0]));
> +	if (type == MP_INT) {

2. Would look better as a switch.

>  		int64_t value = sql_value_int64(argv[0]);
>  		assert(value < 0);
> -		sql_result_uint(context, -value);
> -		break;
> -	}
> @@ -2229,703 +2189,76 @@ static struct {
>  	uint16_t flags;
>  	void (*call)(sql_context *ctx, int argc, sql_value **argv);
>  	void (*finalize)(sql_context *ctx);
> -	/** Members below are related to struct func_def. */
> -	bool is_deterministic;
> -	int param_count;
> -	enum field_type returns;
> -	enum func_aggregate aggregate;
> -	bool export_to_sql;
>  } sql_builtins[] = {
> -	{.name = "ABS",
> -	 .param_count = 1,
> -	 .returns = FIELD_TYPE_NUMBER,
> -	 .aggregate = FUNC_AGGREGATE_NONE,
> -	 .is_deterministic = true,
> -	 .flags = 0,
> -	 .call = absFunc,
> -	 .finalize = NULL,
> -	 .export_to_sql = true,
> -	}, {

3. These removals I don't like. I think we don't need to expose anything
into _func. All could be done in there instead, and would look simpler,
IMO. As I explained in the previous emails in this thread.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-08-22 14:31 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-14 15:04 [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions imeevma
2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 01/10] sql: do not return UNSIGNED in " imeevma
2020-08-22 14:23   ` Vladislav Shpilevoy
2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 02/10] sql: fix functions return types imeevma
2020-08-22 14:24   ` Vladislav Shpilevoy
2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 03/10] sql: change signature of trim() imeevma
2020-08-22 14:26   ` Vladislav Shpilevoy
2020-08-14 15:04 ` [Tarantool-patches] [PATCH v2 04/10] box: add new options for functions imeevma
2020-08-22 14:28   ` Vladislav Shpilevoy
2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 05/10] sql: use has_vararg for built-in functions imeevma
2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 06/10] sql: add overloaded versions of the functions imeevma
2020-08-22 14:29   ` Vladislav Shpilevoy
2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 07/10] sql: move built-in function definitions in _func imeevma
2020-08-22 14:30   ` Vladislav Shpilevoy
2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 08/10] box: add param_list to 'struct func' imeevma
2020-08-22 14:30   ` Vladislav Shpilevoy
2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 09/10] sql: check built-in functions argument types imeevma
2020-08-14 15:05 ` [Tarantool-patches] [PATCH v2 10/10] sql: refactor sql/func.c imeevma
2020-08-22 14:31   ` Vladislav Shpilevoy
2020-08-22 14:25 ` [Tarantool-patches] [PATCH v2 00/10] sql: properly check arguments types of built-in functions Vladislav Shpilevoy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox