[Tarantool-patches] [PATCH v2 03/10] sql: change signature of trim()

[imeevma@tarantool.org]

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