Tarantool development patches archive
 help / color / mirror / Atom feed
From: Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: kyukhin@tarantool.org, v.ioffe@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH v1 4/9] sql: static type check for SQL built-in functions
Date: Thu, 19 Aug 2021 15:03:04 +0300	[thread overview]
Message-ID: <73e3ea57b144156d563034c00c646a7d65d29cee.1629374448.git.imeevma@gmail.com> (raw)
In-Reply-To: <cover.1629374448.git.imeevma@gmail.com>

This patch introduces a static type checking mechanism for SQL built-in
functions. However, it is currently disabled as the functions themselves
need to be prepared for such changes.

Part of #6105
---
 src/box/sql/expr.c    |   6 ++-
 src/box/sql/func.c    | 119 +++++++++++++++++++++++++++++++++++++++---
 src/box/sql/prepare.c |   2 +-
 src/box/sql/select.c  |   6 ++-
 4 files changed, 123 insertions(+), 10 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 1304710a1..8bd9a858e 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -5394,7 +5394,11 @@ analyzeAggregate(Walker * pWalker, Expr * pExpr)
 						       (pExpr, EP_IntValue));
 						pItem->func =
 							sql_func_find(pExpr);
-						assert(pItem->func != NULL);
+						if (pItem->func == NULL) {
+							pParse->is_aborted =
+								true;
+							return WRC_Abort;
+						}
 						assert(pItem->func->def->
 						       language ==
 						       FUNC_LANGUAGE_SQL_BUILTIN &&
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 4d893affc..93f6cc067 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2090,6 +2090,106 @@ built_in_func_put(struct sql_func_dictionary *dict)
 	}
 }
 
+/**
+ * Check if there is no need to cast argument to accepted type. Also, in some
+ * cases operation 'op' may be important, for example when given argument is
+ * NULL or is variable.
+ *
+ * Returns TRUE when:
+ *  - when operation is NULL;
+ *  - when accepted type and argument type are equal;
+ *  - when accepted type is ANY;
+ *  - when accepted type is INTEGER and argument type is UNSIGNED.
+ */
+static inline bool
+is_exact(int op, enum field_type a, enum field_type b)
+{
+	return op == TK_NULL || a == b || a == FIELD_TYPE_ANY ||
+	       (a == FIELD_TYPE_INTEGER && b == FIELD_TYPE_UNSIGNED);
+}
+
+/**
+ * Check if the argument MEM type will not change during cast. It means that
+ * either is_exact() returns TRUE or accepted type is metatype that includes
+ * argument type.
+ *
+ * Returns TRUE when:
+ *  - is_exact() returns TRUE;
+ *  - when accepted type is NUMBER and argument type is numeric type;
+ *  - when accepted type is SCALAR and argument type is not MAP or ARRAY.
+ */
+static inline bool
+is_upcast(int op, enum field_type a, enum field_type b)
+{
+	return is_exact(op, a, b) ||
+	       (a == FIELD_TYPE_NUMBER && sql_type_is_numeric(b)) ||
+	       (a == FIELD_TYPE_SCALAR && b != FIELD_TYPE_MAP &&
+		b != FIELD_TYPE_ARRAY);
+}
+
+/**
+ * Check if there is a chance that the argument can be cast to accepted type
+ * according to implicit cast rules.
+ *
+ * Returns TRUE when:
+ *  - is_upcast() returns TRUE;
+ *  - when accepted type and argument type are numeric types;
+ *  - when argument is binded value;
+ *  - when argument type is ANY, which means that is was not resolved.
+ */
+static inline bool
+is_castable(int op, enum field_type a, enum field_type b)
+{
+	return is_upcast(op, a, b) || op == TK_VARIABLE ||
+	       (sql_type_is_numeric(a) && sql_type_is_numeric(b)) ||
+	       b == FIELD_TYPE_ANY;
+}
+
+enum check_type {
+	CHECK_TYPE_EXACT,
+	CHECK_TYPE_UPCAST,
+	CHECK_TYPE_CASTABLE,
+};
+
+static struct func *
+find_compatible(struct Expr *expr, struct sql_func_dictionary *dict,
+		enum check_type check)
+{
+	int n = expr->x.pList != NULL ? expr->x.pList->nExpr : 0;
+	for (uint32_t i = 0; i < dict->count; ++i) {
+		struct func_sql_builtin *func = dict->functions[i];
+		int argc = func->base.def->param_count;
+		if (argc != n && argc != -1)
+			continue;
+		if (n == 0)
+			return &func->base;
+
+		enum field_type *types = func->param_list;
+		bool is_match = true;
+		for (int j = 0; j < n && is_match; ++j) {
+			struct Expr *e = expr->x.pList->a[j].pExpr;
+			enum field_type a = types[argc != -1 ? j : 0];
+			enum field_type b = sql_expr_type(e);
+			switch (check) {
+			case CHECK_TYPE_EXACT:
+				is_match = is_exact(e->op, a, b);
+				break;
+			case CHECK_TYPE_UPCAST:
+				is_match = is_upcast(e->op, a, b);
+				break;
+			case CHECK_TYPE_CASTABLE:
+				is_match = is_castable(e->op, a, b);
+				break;
+			default:
+				unreachable();
+			}
+		}
+		if (is_match)
+			return &func->base;
+	}
+	return NULL;
+}
+
 static struct func *
 find_built_in_func(struct Expr *expr, struct sql_func_dictionary *dict)
 {
@@ -2108,13 +2208,18 @@ find_built_in_func(struct Expr *expr, struct sql_func_dictionary *dict)
 		diag_set(ClientError, ER_FUNC_WRONG_ARG_COUNT, name, str, n);
 		return NULL;
 	}
-	struct func *func = NULL;
-	for (uint32_t i = 0; i < dict->count; ++i) {
-		func = &dict->functions[i]->base;
-		if (func->def->param_count == n)
-			break;
-	}
-	return func;
+	struct func *func = find_compatible(expr, dict, CHECK_TYPE_EXACT);
+	if (func != NULL)
+		return func;
+	func = find_compatible(expr, dict, CHECK_TYPE_UPCAST);
+	if (func != NULL)
+		return func;
+	func = find_compatible(expr, dict, CHECK_TYPE_CASTABLE);
+	if (func != NULL)
+		return func;
+	diag_set(ClientError, ER_SQL_EXECUTE,
+		 tt_sprintf("wrong arguments for function %s()", name));
+	return NULL;
 }
 
 struct func *
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index d859a0243..b986630e9 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -89,7 +89,7 @@ sql_stmt_compile(const char *zSql, int nBytes, struct Vdbe *pReprepare,
 	} else {
 		sqlRunParser(&sParse, zSql);
 	}
-	assert(0 == sParse.nQueryLoop);
+	assert(0 == sParse.nQueryLoop || sParse.is_aborted);
 
 	if (db->mallocFailed)
 		sParse.is_aborted = true;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 021e0ebd5..6ae0cebe7 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -6279,7 +6279,7 @@ sqlSelect(Parse * pParse,		/* The parser context */
 			sNC.ncFlags &= ~NC_InAggFunc;
 		}
 		sAggInfo.mxReg = pParse->nMem;
-		if (db->mallocFailed)
+		if (pParse->is_aborted)
 			goto select_end;
 
 		/* Processing for aggregates with GROUP BY is very different and
@@ -6488,6 +6488,8 @@ sqlSelect(Parse * pParse,		/* The parser context */
 			 */
 			sqlVdbeJumpHere(v, addr1);
 			updateAccumulator(pParse, &sAggInfo);
+			if (pParse->is_aborted)
+				goto select_end;
 			sqlVdbeAddOp2(v, OP_Integer, 1, iUseFlag);
 			VdbeComment((v, "indicate data in accumulator"));
 
@@ -6651,6 +6653,8 @@ sqlSelect(Parse * pParse,		/* The parser context */
 					goto select_end;
 				}
 				updateAccumulator(pParse, &sAggInfo);
+				if (pParse->is_aborted)
+					goto select_end;
 				assert(pMinMax == 0 || pMinMax->nExpr == 1);
 				if (sqlWhereIsOrdered(pWInfo) > 0) {
 					sqlVdbeGoto(v,
-- 
2.25.1


  parent reply	other threads:[~2021-08-19 12:04 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-19 12:02 [Tarantool-patches] [PATCH v1 0/9] Check types of SQL built-in functions arguments Mergen Imeev via Tarantool-patches
2021-08-19 12:02 ` [Tarantool-patches] [PATCH v1 1/9] sql: modify signature of TRIM() Mergen Imeev via Tarantool-patches
2021-08-19 12:02 ` [Tarantool-patches] [PATCH v1 2/9] sql: rework SQL built-in functions hash table Mergen Imeev via Tarantool-patches
2021-08-19 12:03 ` [Tarantool-patches] [PATCH v1 3/9] sql: check number of arguments during parsing Mergen Imeev via Tarantool-patches
2021-08-19 12:03 ` Mergen Imeev via Tarantool-patches [this message]
2021-08-19 12:03 ` [Tarantool-patches] [PATCH v1 5/9] sql: runtime type check for SQL built-in functions Mergen Imeev via Tarantool-patches
2021-08-19 12:03 ` [Tarantool-patches] [PATCH v1 6/9] sql: enable types checking for some functions Mergen Imeev via Tarantool-patches
2021-08-19 12:03 ` [Tarantool-patches] [PATCH v1 7/9] sql: fix result type of min() and max() functions Mergen Imeev via Tarantool-patches
2021-08-19 12:03 ` [Tarantool-patches] [PATCH v1 8/9] sql: check argument types of sum(), avg(), total() Mergen Imeev via Tarantool-patches
2021-08-19 12:03 ` [Tarantool-patches] [PATCH v1 9/9] sql: arguments check for string value functions Mergen Imeev via Tarantool-patches
2021-08-19 12:26 ` [Tarantool-patches] [PATCH v1 0/9] Check types of SQL built-in functions arguments Kirill Yukhin via Tarantool-patches
2021-08-19 15:50   ` Vitaliia Ioffe via Tarantool-patches
2021-08-19 16:16 ` Kirill Yukhin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=73e3ea57b144156d563034c00c646a7d65d29cee.1629374448.git.imeevma@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=kyukhin@tarantool.org \
    --cc=v.ioffe@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v1 4/9] sql: static type check for SQL built-in functions' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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