[tarantool-patches] Re: [PATCH v2 7/7] sql: store regular identifiers in case-normal form

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the fixes!

See my 5 comments below, review fixes at the end of the
email, and on the branch.

After you've received this bunch of reviews, I ask you ... not,
even beg you not to hurry. Read the comments, the fixes, squash
if you agree. Slowly and accurately fix the other places with
which you do not agree. If such places exist, then send the
patchset as v2. This thread is already dead and unreadable.

If you agree with all the comments, then ask Kirill Y. what to
do next - send it to Nikita, or directly to Kirill Y.

> Introduced a new sql_normalize_name routine performing SQL name
> conversion to case-normal form via unicode character folding.
> For example, ß is converted to SS. The result is similar to SQL
> UPPER function.
> 
> Closes #3931
> ---
>   src/box/lua/lua_sql.c                 | 11 +++--
>   src/box/sql/build.c                   | 32 +++++++++----
>   src/box/sql/expr.c                    | 67 ++++++++++++++++++---------
>   src/box/sql/parse.y                   | 25 ++++++++--
>   src/box/sql/select.c                  | 19 ++++++--
>   src/box/sql/sqlInt.h                  | 28 ++++++++++-
>   src/box/sql/trigger.c                 | 14 ++++--
>   src/box/sql/util.c                    | 46 ++++++++++++------
>   src/lib/core/errinj.h                 |  1 +
>   test/box/errinj.result                |  2 +
>   test/sql-tap/identifier_case.test.lua | 12 +++--
>   test/sql/errinj.result                | 18 +++++++
>   test/sql/errinj.test.lua              |  8 ++++
>   13 files changed, 215 insertions(+), 68 deletions(-)
> 
> diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
> index f5a7b7819..2b3a595af 100644
> --- a/src/box/lua/lua_sql.c
> +++ b/src/box/lua/lua_sql.c
> @@ -176,13 +176,16 @@ lbox_sql_create_function(struct lua_State *L)
>   	}
>   	size_t name_len;
>   	const char *name = lua_tolstring(L, 1, &name_len);
> +	int normalized_name_sz = sql_normalize_name(NULL, 0, name, name_len);
> +	if (normalized_name_sz < 0)
> +		return luaT_error(L);
>   	char *normalized_name = (char *) region_alloc(&fiber()->gc,
> -						      name_len + 1);
> +						      normalized_name_sz + 1);
>   	if (normalized_name == NULL)
>   		return luaL_error(L, "out of memory");
> -	memcpy(normalized_name, name, name_len);
> -	normalized_name[name_len] = '\0';
> -	sqlNormalizeName(normalized_name);
> +	if (sql_normalize_name(normalized_name, normalized_name_sz, name,
> +			       name_len) < 0)
> +		return luaT_error(L);

1. When code duplicates not as 1-2 lines, but as a ~10-15 lines, it is
almost always very bad. After your patch you have 2 completely equal
places where you allocate a normalized name on region. And 3 places
on sql malloc. I've put that code into functions and used them.

>   	struct lua_sql_func_info *func_info =
>   		(struct lua_sql_func_info *) malloc(sizeof(*func_info));
>   	if (func_info == NULL)
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index dad4ce3a6..55ca100c1 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -883,7 +883,15 @@ sql_expr_new(struct sql *db, int op, const Token *token, bool dequote)
>   	if (token != NULL) {
>   		if (op != TK_INTEGER || token->z == NULL ||
>   		    sqlGetInt32(token->z, &val) == 0) {
> -			extra_sz = token->n + 1;
> +			if (op == TK_ID || op == TK_COLLATE ||
> +			    op == TK_FUNCTION) {
> +				extra_sz = sql_normalize_name(NULL, 0, token->z,
> +							      token->n);
> +				if (extra_sz < 0)
> +					return NULL;
> +			} else {
> +				extra_sz = token->n + 1;
> +			}
>   			assert(val >= 0);
>   		}
>   	}
> @@ -909,15 +917,20 @@ sql_expr_new(struct sql *db, int op, const Token *token, bool dequote)
>   	} else {
>   		expr->u.zToken = (char *)&expr[1];
>   		assert(token->z != NULL || token->n == 0);
> -		memcpy(expr->u.zToken, token->z, token->n);
> -		expr->u.zToken[token->n] = '\0';
> -		if (dequote) {
> -			if (expr->u.zToken[0] == '"')
> -				expr->flags |= EP_DblQuoted;
> -			if (expr->op == TK_ID || expr->op == TK_COLLATE ||
> -			    expr->op == TK_FUNCTION)
> -				sqlNormalizeName(expr->u.zToken);
> -			else
> +		if (dequote && expr->u.zToken[0] == '"')
> +			expr->flags |= EP_DblQuoted;
> +		if (dequote &&
> +		    (expr->op == TK_ID || expr->op == TK_COLLATE ||
> +		     expr->op == TK_FUNCTION)) {
> +			if (sql_normalize_name(expr->u.zToken, extra_sz,
> +					       token->z, token->n) < 0) {
> +				sqlDbFree(db, expr);
> +				return NULL;
> +			}
> +		} else {
> +			memcpy(expr->u.zToken, token->z, token->n);
> +			expr->u.zToken[token->n] = '\0';
> +			if (dequote)
>   				sqlDequote(expr->u.zToken);
>   		}

2. Two hunks above make sql_expr_new overcomplicated. You check for
'dequote' 3 times. And at the same time you do not check it when you
decide whether to increase extra_sz in the hunk above. In fixes for the
previous commit I split sql_expr_new into sql_expr_new and
sql_expr_new_dequoted - it solves the problem here as well.

>   	}
> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 13d0b16f8..120eedfa6 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -3195,7 +3195,33 @@ void sqlTreeViewWith(TreeView *, const With *);
>   void sqlSetString(char **, sql *, const char *);
>   void sqlErrorMsg(Parse *, const char *, ...);
>   void sqlDequote(char *);
> -void sqlNormalizeName(char *z);
> +
> +/**
> + * Perform SQL name normalization: cast name to the upper-case
> + * (via Unicode Character Folding). Casing is locale-dependent
> + * and context-sensitive. The result may be longer or shorter
> + * than the original. The source string and the destination buffer
> + * must not overlap.
> + * For example, ß is converted to SS.
> + * The result is similar to SQL UPPER function.
> + *
> + * @param dst A buffer for the result string. The result will be
> + *            NUL-terminated if the buffer is large enough. The
> + *            contents is undefined in case of failure.

3. I understand that you just copy-pasted comment from ICU
docs, but usually we do not write 'NUL-terminated'. We use
either NULL, or 0.

> + * @param dst_size The size of the buffer (number of bytes).
> + *                 If it is 0, then dest may be NULL and the
> + *                 function will only return the corresponding
> + *                 dst_size - this size of buffer that may fit
> + *                 result.

4. Here you decided not to copy though here the original
comment would be correct. In your version you said, that
in case of too small buffer you return dst_size. But it is
wrong. What would be a sense of such a function, when on
a too small buffer it returns the same too small size? In
fact it returns a new size, not presented among the
arguments.

> + * @param src The original string.
> + * @param src_len The length of the original string.
> + * @retval The count of bytes written(or need to be written) on
> + *         success.
> + * @retval < 0 Otherwise. The diag message is set.
> + */
> +int
> +sql_normalize_name(char *dst, int dst_size, const char *src, int src_len);
> +
>   void sqlTokenInit(Token *, char *);
>   int sqlKeywordCode(const unsigned char *, int);
>   int sqlRunParser(Parse *, const char *);
> diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
> index 544ba2a5a..16d0f7fe6 100644
> --- a/src/box/sql/trigger.c
> +++ b/src/box/sql/trigger.c
> @@ -279,15 +279,23 @@ sql_trigger_select_step(struct sql *db, struct Select *select)
>   static struct TriggerStep *
>   sql_trigger_step_new(struct sql *db, u8 op, struct Token *target_name)
>   {
> +	int name_sz =
> +		sql_normalize_name(NULL, 0, target_name->z, target_name->n);
> +	if (name_sz < 0)
> +		return NULL;
>   	int size = sizeof(struct TriggerStep) + target_name->n + 1;
>   	struct TriggerStep *trigger_step = sqlDbMallocZero(db, size);

5. What frustrated me the most in the patch is that place. Because it is
easy visible on a very first self- even not a review, but just screening.
You calculated name_sz, which can be bigger than target_name->n + 1, but
left the old size as it was. Below it could lead to a bad memory access.

>   	if (trigger_step == NULL) {
> -		diag_set(OutOfMemory, size, "sqlDbMallocZero", "trigger_step");
> +		diag_set(OutOfMemory, name_sz, "sqlDbMallocZero",
> +			 "trigger_step");
>   		return NULL;
>   	}
>   	char *z = (char *)&trigger_step[1];
> -	memcpy(z, target_name->z, target_name->n);
> -	sqlNormalizeName(z);
> +	if (sql_normalize_name(z, name_sz, target_name->z,
> +			       target_name->n) < 0) {
> +		sqlDbFree(db, trigger_step);
> +		return NULL;
> +	}
>   	trigger_step->zTarget = z;
>   	trigger_step->op = op;
>   	return trigger_step;

=========================================================================

commit 1dfc7de80f8de06348d5b0f035d705918c2e2dfa
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date:   Tue Mar 26 19:03:20 2019 +0300

     Review fixes

diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index 2b3a595af..3d0047e16 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -176,15 +176,9 @@ lbox_sql_create_function(struct lua_State *L)
  	}
  	size_t name_len;
  	const char *name = lua_tolstring(L, 1, &name_len);
-	int normalized_name_sz = sql_normalize_name(NULL, 0, name, name_len);
-	if (normalized_name_sz < 0)
-		return luaT_error(L);
-	char *normalized_name = (char *) region_alloc(&fiber()->gc,
-						      normalized_name_sz + 1);
+	char *normalized_name =
+		sql_normalized_name_region_new(&fiber()->gc, name, name_len);
  	if (normalized_name == NULL)
-		return luaL_error(L, "out of memory");
-	if (sql_normalize_name(normalized_name, normalized_name_sz, name,
-			       name_len) < 0)
  		return luaT_error(L);
  	struct lua_sql_func_info *func_info =
  		(struct lua_sql_func_info *) malloc(sizeof(*func_info));
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 76801cd94..9c288b427 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -229,27 +229,6 @@ sql_space_column_is_in_pk(struct space *space, uint32_t column)
  	return false;
  }
  
-char *
-sql_name_from_token(struct sql *db, struct Token *name_token)
-{
-	assert(name_token != NULL && name_token->z != NULL);
-	int name_sz =
-		sql_normalize_name(NULL, 0, name_token->z, name_token->n);
-	if (name_sz < 0)
-		return NULL;
-	char *name = sqlDbMallocRawNN(db, name_sz);
-	if (name == NULL) {
-		diag_set(OutOfMemory, name_sz, "sqlDbMallocRawNN", "name");
-		return NULL;
-	}
-	if (sql_normalize_name(name, name_sz, name_token->z,
-			       name_token->n) < 0) {
-		sqlDbFree(db, name);
-		return NULL;
-	}
-	return name;
-}
-
  /*
   * This routine is used to check if the UTF-8 string zName is a legal
   * unqualified name for an identifier.
@@ -441,18 +420,8 @@ sqlAddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
  	if (sql_field_retrieve(pParse, def, def->field_count) == NULL)
  		return;
  	struct region *region = &pParse->region;
-	int name_sz = sql_normalize_name(NULL, 0, pName->z, pName->n);
-	if (name_sz < 0) {
-		pParse->is_aborted = true;
-		return;
-	}
-	z = region_alloc(region, name_sz);
+	z = sql_normalized_name_region_new(region, pName->z, pName->n);
  	if (z == NULL) {
-		diag_set(OutOfMemory, name_sz, "region_alloc", "z");
-		pParse->is_aborted = true;
-		return;
-	}
-	if (sql_normalize_name(z, name_sz, pName->z, pName->n) < 0) {
  		pParse->is_aborted = true;
  		return;
  	}
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 4d5464617..660110660 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -879,65 +879,113 @@ sqlExprSetHeightAndFlags(Parse * pParse, Expr * p)
  #define exprSetHeight(y)
  #endif				/* SQL_MAX_EXPR_DEPTH>0 */
  
-struct Expr *
-sql_expr_new(struct sql *db, int op, const struct Token *token)
+/**
+ * Allocate a new empty expression object with reserved extra
+ * memory.
+ * @param db SQL context.
+ * @param op Expression value type.
+ * @param extra_size Extra size, needed to be allocated together
+ *        with the expression.
+ * @retval Not NULL Success. An empty expression.
+ * @retval NULL Error. A diag message is set.
+ */
+static struct Expr *
+sql_expr_new_empty(struct sql *db, int op, int extra_size)
  {
-	int extra_sz = 0;
-	int val = 0;
-	if (token != NULL) {
-		if (op != TK_INTEGER || token->z == NULL ||
-		    sqlGetInt32(token->z, &val) == 0) {
-			if (op == TK_ID || op == TK_COLLATE ||
-			    op == TK_FUNCTION) {
-				extra_sz = sql_normalize_name(NULL, 0, token->z,
-							      token->n);
-				if (extra_sz < 0)
-					return NULL;
-			} else {
-				extra_sz = token->n + 1;
-			}
-			assert(val >= 0);
-		}
-	}
-	struct Expr *expr = sqlDbMallocRawNN(db, sizeof(*expr) + extra_sz);
-	if (expr == NULL) {
-		diag_set(OutOfMemory, sizeof(*expr), "sqlDbMallocRawNN",
-			 "expr");
+	struct Expr *e = sqlDbMallocRawNN(db, sizeof(*e) + extra_size);
+	if (e == NULL) {
+		diag_set(OutOfMemory, sizeof(*e), "sqlDbMallocRawNN", "e");
  		return NULL;
  	}
-
-	memset(expr, 0, sizeof(*expr));
-	expr->op = (u8)op;
-	expr->iAgg = -1;
+	memset(e, 0, sizeof(*e));
+	e->op = (u8)op;
+	e->iAgg = -1;
  #if SQL_MAX_EXPR_DEPTH > 0
-	expr->nHeight = 1;
+	e->nHeight = 1;
  #endif
-	if (token == NULL)
-		return expr;
+	return e;
+}
  
-	if (extra_sz == 0) {
-		expr->flags |= EP_IntValue;
-		expr->u.iValue = val;
-	} else {
-		expr->u.zToken = (char *)&expr[1];
-		assert(token->z != NULL || token->n == 0);
-		memcpy(expr->u.zToken, token->z, token->n);
-		expr->u.zToken[token->n] = '\0';
+/**
+ * Try to convert a token of a specified type to integer.
+ * @param op Token type.
+ * @param token Token itself.
+ * @param[out] res Result integer.
+ * @retval 0 Success. @A res stores a result.
+ * @retval -1 Error. Can not be converted. No diag.
+ */
+static inline int
+sql_expr_token_to_int(int op, const struct Token *token, int *res)
+{
+	if (op == TK_INTEGER && token->z != NULL &&
+	    sqlGetInt32(token->z, res) > 0)
+		return 0;
+	return -1;
+}
+
+/** Create an expression of a constant integer. */
+static inline struct Expr *
+sql_expr_new_int(struct sql *db, int value)
+{
+	struct Expr *e = sql_expr_new_empty(db, TK_INTEGER, 0);
+	if (e != NULL) {
+		e->flags |= EP_IntValue;
+		e->u.iValue = value;
  	}
-	return expr;
+	return e;
+}
+
+struct Expr *
+sql_expr_new(struct sql *db, int op, const struct Token *token)
+{
+	int extra_sz = 0;
+	if (token != NULL) {
+		int val;
+		if (sql_expr_token_to_int(op, token, &val) == 0)
+			return sql_expr_new_int(db, val);
+		extra_sz = token->n + 1;
+	}
+	struct Expr *e = sql_expr_new_empty(db, op, extra_sz);
+	if (e == NULL || token == NULL)
+		return e;
+	e->u.zToken = (char *) &e[1];
+	assert(token->z != NULL || token->n == 0);
+	memcpy(e->u.zToken, token->z, token->n);
+	e->u.zToken[token->n] = '\0';
+	return e;
  }
  
  struct Expr *
  sql_expr_new_dequoted(struct sql *db, int op, const struct Token *token)
  {
-	struct Expr *e = sql_expr_new(db, op, token);
-	if (e == NULL || (e->flags & EP_IntValue) != 0 || e->u.zToken == NULL)
+	int extra_size = 0;
+	bool is_name = false;
+	if (token != NULL) {
+		int val;
+		assert(token->z != NULL || token->n == 0);
+		if (sql_expr_token_to_int(op, token, &val) == 0)
+			return sql_expr_new_int(db, val);
+		is_name = op == TK_ID || op == TK_COLLATE || op == TK_FUNCTION;
+		if (is_name) {
+			extra_size = sql_normalize_name(NULL, 0, token->z,
+							token->n);
+			if (extra_size < 0)
+				return NULL;
+		} else {
+			extra_size = token->n + 1;
+		}
+	}
+	struct Expr *e = sql_expr_new_empty(db, op, extra_size);
+	if (e == NULL || token == NULL || token->n == 0)
  		return e;
-	if (e->u.zToken[0] == '"')
+	e->u.zToken = (char *) &e[1];
+	if (token->z[0] == '"')
  		e->flags |= EP_DblQuoted;
-	if (e->op != TK_ID && e->op != TK_COLLATE && e->op != TK_FUNCTION) {
+	if (! is_name) {
+		memcpy(e->u.zToken, token->z, token->n);
+		e->u.zToken[token->n] = '\0';
  		sqlDequote(e->u.zToken);
-	} else if (sql_normalize_name(e->u.zToken, strlen(e->u.zToken), token->z,
+	} else if (sql_normalize_name(e->u.zToken, extra_size, token->z,
  				      token->n) < 0) {
  		sql_expr_delete(db, e, false);
  		return NULL;
@@ -1824,31 +1872,22 @@ sqlExprListSetName(Parse * pParse,	/* Parsing context */
  		       int dequote	/* True to cause the name to be dequoted */
      )
  {
-	assert(pList != 0 || pParse->db->mallocFailed != 0);
+	struct sql *db = pParse->db;
+	assert(pList != 0 || db->mallocFailed != 0);
  	if (pList == NULL || pName->n == 0)
  		return;
  	assert(pList->nExpr > 0);
  	struct ExprList_item *item = &pList->a[pList->nExpr - 1];
  	assert(item->zName == NULL);
  	if (dequote) {
-		int name_sz = sql_normalize_name(NULL, 0, pName->z, pName->n);
-		if (name_sz < 0)
-			goto tarantool_error;
-		item->zName = sqlDbMallocRawNN(pParse->db, name_sz);
+		item->zName = sql_normalized_name_db_new(db, pName->z, pName->n);
  		if (item->zName == NULL)
-			return;
-		if (sql_normalize_name(item->zName, name_sz, pName->z,
-				       pName->n) < 0)
-			goto tarantool_error;
+			pParse->is_aborted = true;
  	} else {
-		item->zName = sqlDbStrNDup(pParse->db, pName->z, pName->n);
-		if (item->zName == NULL)
-			return;
+		item->zName = sqlDbStrNDup(db, pName->z, pName->n);
  	}
-	sqlCheckIdentifierName(pParse, item->zName);
-	return;
-tarantool_error:
-	pParse->is_aborted = true;
+	if (item->zName != NULL)
+		sqlCheckIdentifierName(pParse, item->zName);
  }
  
  /*
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index bc52451aa..c7e1879ca 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -4187,15 +4187,11 @@ flattenSubquery(Parse * pParse,		/* Parsing context */
  			if (pList->a[i].zName == 0) {
  				char *str = pList->a[i].zSpan;
  				int len = strlen(str);
-				int name_sz =
-					sql_normalize_name(NULL, 0, str, len);
-				if (name_sz < 0)
-					goto tarantool_error;
-				char *name = sqlDbMallocRawNN(db, name_sz);
-				if (name != NULL &&
-				    sql_normalize_name(name, name_sz, str,
-						       len) < 0)
-					goto tarantool_error;
+				char *name =
+					sql_normalized_name_db_new(db, str,
+								   len);
+				if (name == NULL)
+					pParse->is_aborted = true;
  				pList->a[i].zName = name;
  			}
  		}
@@ -4274,9 +4270,6 @@ flattenSubquery(Parse * pParse,		/* Parsing context */
  	}
  #endif
  
-	return 1;
-tarantool_error:
-	pParse->is_aborted = true;
  	return 1;
  }
  
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 74c3a98be..168b4b47f 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -3206,13 +3206,12 @@ void sqlDequote(char *);
   * The result is similar to SQL UPPER function.
   *
   * @param dst A buffer for the result string. The result will be
- *            NUL-terminated if the buffer is large enough. The
- *            contents is undefined in case of failure.
- * @param dst_size The size of the buffer (number of bytes).
- *                 If it is 0, then dest may be NULL and the
- *                 function will only return the corresponding
- *                 dst_size - this size of buffer that may fit
- *                 result.
+ *        0-terminated if the buffer is large enough. The contents
+ *        is undefined in case of failure.
+ * @param dst_size The size of the buffer (number of bytes). If it
+ *        is 0, then dest may be NULL and the function will only
+ *        return the length of the result without writing any of
+ *        the result string
   * @param src The original string.
   * @param src_len The length of the original string.
   * @retval The count of bytes written(or need to be written) on
@@ -3222,6 +3221,31 @@ void sqlDequote(char *);
  int
  sql_normalize_name(char *dst, int dst_size, const char *src, int src_len);
  
+/**
+ * Duplicate a normalized version of @a name onto an sqlMalloc.
+ * For normalization rules @sa sql_normalize_name().
+ * @param db SQL context.
+ * @param name Source string.
+ * @param len Length of @a name.
+ * @retval Not NULL Success. A normalized string is returned.
+ * @retval NULL Error. A diag message is set.
+ */
+char *
+sql_normalized_name_db_new(struct sql *db, const char *name, int len);
+
+/**
+ * Duplicate a normalized version of @a name onto a region @a r.
+ * For normalization rules @sa sql_normalize_name().
+ * @param r Region allocator.
+ * @param name Source string.
+ * @param len Length of @a name.
+ * @retval Not NULL Success. A normalized string is returned.
+ * @retval NULL Error. A diag message is set. Region is not
+ *         truncated back.
+ */
+char *
+sql_normalized_name_region_new(struct region *r, const char *name, int len);
+
  void sqlTokenInit(Token *, char *);
  int sqlKeywordCode(const unsigned char *, int);
  int sqlRunParser(Parse *, const char *);
@@ -3768,12 +3792,16 @@ void sqlExprIfFalse(Parse *, Expr *, int, int);
   * string is \000 terminated and is persistent.
   *
   * @param db The database connection.
- * @param name_token The source token with text.
+ * @param t The source token with text.
   * @retval Not NULL Formatted name on new memory.
   * @retval NULL Error. Diag message is set.
   */
-char *
-sql_name_from_token(struct sql *db, struct Token *name_token);
+static inline char *
+sql_name_from_token(struct sql *db, struct Token *t)
+{
+	assert(t != NULL && t->z != NULL);
+	return sql_normalized_name_db_new(db, t->z, t->n);
+}
  
  int sqlExprCompare(Expr *, Expr *, int);
  int sqlExprListCompare(ExprList *, ExprList *, int);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 681bd55e4..ec3b3e172 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -279,19 +279,18 @@ sql_trigger_select_step(struct sql *db, struct Select *select)
  static struct TriggerStep *
  sql_trigger_step_new(struct sql *db, u8 op, struct Token *target_name)
  {
-	int name_sz =
+	int name_size =
  		sql_normalize_name(NULL, 0, target_name->z, target_name->n);
-	if (name_sz < 0)
+	if (name_size < 0)
  		return NULL;
-	int size = sizeof(struct TriggerStep) + target_name->n + 1;
+	int size = sizeof(struct TriggerStep) + name_size;
  	struct TriggerStep *trigger_step = sqlDbMallocZero(db, size);
  	if (trigger_step == NULL) {
-		diag_set(OutOfMemory, name_sz, "sqlDbMallocZero",
-			 "trigger_step");
+		diag_set(OutOfMemory, size, "sqlDbMallocZero", "trigger_step");
  		return NULL;
  	}
  	char *z = (char *)&trigger_step[1];
-	if (sql_normalize_name(z, name_sz, target_name->z,
+	if (sql_normalize_name(z, name_size, target_name->z,
  			       target_name->n) < 0) {
  		sqlDbFree(db, trigger_step);
  		return NULL;
diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index 85da88ab6..bca32ad9a 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -320,6 +320,40 @@ error:
  	return -1;
  }
  
+char *
+sql_normalized_name_db_new(struct sql *db, const char *name, int len)
+{
+	int size = sql_normalize_name(NULL, 0, name, len);
+	if (size < 0)
+		return NULL;
+	char *res = sqlDbMallocRawNN(db, size);
+	if (res == NULL) {
+		diag_set(OutOfMemory, size, "sqlDbMallocRawNN", "res");
+		return NULL;
+	}
+	if (sql_normalize_name(res, size, name, len) < 0) {
+		sqlDbFree(db, res);
+		return NULL;
+	}
+	return res;
+}
+
+char *
+sql_normalized_name_region_new(struct region *r, const char *name, int len)
+{
+	int size = sql_normalize_name(NULL, 0, name, len);
+	if (size < 0)
+		return NULL;
+	char *res = (char *) region_alloc(r, size);
+	if (res == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc", "res");
+		return NULL;
+	}
+	if (sql_normalize_name(res, size, name, len) < 0)
+		return NULL;
+	return res;
+}
+
  /*
   * Generate a Token object from a string
   */