From f95a34da9eb3ba0f09505f00df8fbd043e9fbe21 Mon Sep 17 00:00:00 2001
From: Roman Khabibov <roman.habibov@tarantool.org>
Date: Thu, 28 Mar 2019 14:01:33 +0300
Subject: [PATCH] sql: modify TRIM() function signature

According to the ANSI standard, ltrim, rtrim and trim should
be merged into one unified TRIM() function. The specialization of
trimming (left, right or both and trimming characters) determined
in arguments of this function.

Closes #3879

@TarantoolBot document
Title: TRIM() function

Modify signature of SQL function TRIM(). This function removes
characters included in <trim character> (binary) string from
<trim source> (binary) string until encounter a character that doesn't
belong to <trim character>. Removal occurs on the side, specified by
<trim specification>. Now, syntax is following:
TRIM([ [ <trim specification> ] [ <trim character> ] FROM ] <trim source>).

<trim specification> can be one of the following keywords: LEADING,
TRAILING and BOTH.
<trim character> is the set of trimming characters.
<trim source> is the string, that will be trimmed.
If FROM is specified, then:
1) Either <trim specification> or <trim character> or both shall be
specified.
2) If <trim specification> is not specified, then BOTH is implicit.
3) If <trim character> is not specified, then ' ' is implicit.
---
 extra/mkkeywordhash.c         |   4 +
 src/box/sql/func.c            | 281 ++++++++++++++++++++++------------
 src/box/sql/parse.y           |  55 ++++++-
 src/box/sql/parse_def.c       |   4 +-
 src/box/sql/parse_def.h       |   2 +-
 src/box/sql/sqlInt.h          |  11 ++
 test/sql-tap/badutf1.test.lua |  14 +-
 test/sql-tap/func.test.lua    | 113 +++++++++-----
 test/sql-tap/with1.test.lua   |   2 +-
 9 files changed, 338 insertions(+), 148 deletions(-)

diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index be7bd55452..76e3265e75 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -278,6 +278,10 @@ static Keyword aKeywordTable[] = {
   { "WHILE",                  "TK_STANDARD",    RESERVED,         true  },
   { "TEXT",                   "TK_TEXT",        RESERVED,         true  },
   { "TRUNCATE",               "TK_TRUNCATE",    ALWAYS,           true  },
+  { "TRIM",                   "TK_TRIM",        ALWAYS,           true  },
+  { "LEADING",                "TK_LEADING",     ALWAYS,           true  },
+  { "TRAILING",               "TK_TRAILING",    ALWAYS,           true  },
+  { "BOTH",                   "TK_BOTH",        ALWAYS,           true  },
 };
 
 /* Number of keywords */
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 34b6c79bb4..e3cff86c5d 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1284,108 +1284,196 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv)
 	sql_result_text(context, (char *)zOut, j, sql_free);
 }
 
-/*
- * Implementation of the TRIM(), LTRIM(), and RTRIM() functions.
- * The userdata is 0x1 for left trim, 0x2 for right trim, 0x3 for both.
+/**
+ * Remove characters included in @a trim_set from @a input_str
+ * until encounter a character that doesn't belong to @a trim_set.
+ * Remove from the side specified by @a flags.
+ * @param context SQL context.
+ * @param flags Trim specification: left, right or both.
+ * @param trim_set The set of characters for trimming.
+ * @param char_len Lengths of each UTF-8 character in @a trim_set.
+ * @param char_cnt A number of UTF-8 characters in @a trim_set.
+ * @param input_str Input string for trimming.
+ * @param input_str_sz Input string size in bytes.
  */
 static void
-trimFunc(sql_context * context, int argc, sql_value ** argv)
+trim_procedure(struct sql_context *context, enum trim_side_mask flags,
+	       const unsigned char *trim_set, const uint8_t *char_len,
+	       int char_cnt, const unsigned char *input_str, int input_str_sz)
 {
-	const unsigned char *zIn;	/* Input string */
-	const unsigned char *zCharSet;	/* Set of characters to trim */
-	int nIn;		/* Number of bytes in input */
-	int flags;		/* 1: trimleft  2: trimright  3: trim */
-	int i;			/* Loop counter */
-	unsigned char *aLen = 0;	/* Length of each character in zCharSet */
-	unsigned char **azChar = 0;	/* Individual characters in zCharSet */
-	int nChar;		/* Number of characters in zCharSet */
-
-	if (sql_value_type(argv[0]) == SQL_NULL) {
-		return;
-	}
-	zIn = sql_value_text(argv[0]);
-	if (zIn == 0)
-		return;
-	nIn = sql_value_bytes(argv[0]);
-	assert(zIn == sql_value_text(argv[0]));
-	if (argc == 1) {
-		static const unsigned char lenOne[] = { 1 };
-		static unsigned char *const azOne[] = { (u8 *) " " };
-		nChar = 1;
-		aLen = (u8 *) lenOne;
-		azChar = (unsigned char **)azOne;
-		zCharSet = 0;
-	} else if ((zCharSet = sql_value_text(argv[1])) == 0) {
-		return;
-	} else {
-		const unsigned char *z = zCharSet;
-		int trim_set_sz = sql_value_bytes(argv[1]);
-		/*
-		* Count the number of UTF-8 characters passing
-		* through the entire char set, but not up
-		* to the '\0' or X'00' character. This allows
-		* to handle trimming set containing such
-		* characters.
-		*/
-		nChar = sql_utf8_char_count(z, trim_set_sz);
-		if (nChar > 0) {
-			azChar =
-			    contextMalloc(context,
-					  ((i64) nChar) * (sizeof(char *) + 1));
-			if (azChar == 0) {
-				return;
-			}
-			aLen = (unsigned char *)&azChar[nChar];
-			z = zCharSet;
-			i = 0;
-			nChar = 0;
-			int handled_bytes_cnt = trim_set_sz;
-			while(handled_bytes_cnt > 0) {
-				azChar[nChar] = (unsigned char *)(z + i);
-				SQL_UTF8_FWD_1(z, i, trim_set_sz);
-				aLen[nChar] = (u8) (z + i - azChar[nChar]);
-				handled_bytes_cnt -= aLen[nChar];
-				nChar++;
-			}
-		}
-	}
-	if (nChar > 0) {
-		flags = SQL_PTR_TO_INT(sql_user_data(context));
-		if (flags & 1) {
-			while (nIn > 0) {
-				int len = 0;
-				for (i = 0; i < nChar; i++) {
-					len = aLen[i];
-					if (len <= nIn
-					    && memcmp(zIn, azChar[i], len) == 0)
-						break;
-				}
-				if (i >= nChar)
+	if (char_cnt == 0)
+		goto finish;
+	int i, len;
+	const unsigned char *z;
+	if ((flags & TRIM_LEADING) != 0) {
+		while (input_str_sz > 0) {
+			z = trim_set;
+			for (i = 0; i < char_cnt; ++i, z += len) {
+				len = char_len[i];
+				if (len <= input_str_sz
+				    && memcmp(input_str, z, len) == 0)
 					break;
-				zIn += len;
-				nIn -= len;
 			}
+			if (i >= char_cnt)
+				break;
+			input_str += len;
+			input_str_sz -= len;
 		}
-		if (flags & 2) {
-			while (nIn > 0) {
-				int len = 0;
-				for (i = 0; i < nChar; i++) {
-					len = aLen[i];
-					if (len <= nIn
-					    && memcmp(&zIn[nIn - len],
-						      azChar[i], len) == 0)
-						break;
-				}
-				if (i >= nChar)
+	}
+	if ((flags & TRIM_TRAILING) != 0) {
+		while (input_str_sz > 0) {
+			z = trim_set;
+			for (i = 0; i < char_cnt; ++i, z += len) {
+				len = char_len[i];
+				if (len <= input_str_sz
+				    && memcmp(&input_str[input_str_sz - len],
+					      z, len) == 0)
 					break;
-				nIn -= len;
 			}
+			if (i >= char_cnt)
+				break;
+			input_str_sz -= len;
 		}
-		if (zCharSet) {
-			sql_free(azChar);
-		}
 	}
-	sql_result_text(context, (char *)zIn, nIn, SQL_TRANSIENT);
+finish:
+	sql_result_text(context, (char *)input_str, input_str_sz,
+			SQL_TRANSIENT);
+}
+
+/**
+ * Prepare arguments for trimming procedure. Allocate memory for
+ * @a char_len (array of lengths each character in @a trim_set)
+ * and fill it.
+ *
+ * @param context SQL context.
+ * @param trim_set The set of characters for trimming.
+ * @param[out] char_len Lengths of each character in @ trim_set.
+ * @retval >=0 A number of UTF-8 characters in @a trim_set.
+ * @retval -1 Memory allocation error.
+ */
+static int
+trim_prepare_char_len(struct sql_context *context,
+		      const unsigned char *trim_set, int trim_set_sz,
+		      uint8_t **char_len)
+{
+	/*
+	 * Count the number of UTF-8 characters passing through
+	 * the entire char set, but not up to the '\0' or X'00'
+	 * character. This allows to handle trimming set
+	 * containing such characters.
+	 */
+	int char_cnt = sql_utf8_char_count(trim_set, trim_set_sz);
+	if (char_cnt == 0) {
+		*char_len = NULL;
+		return 0;
+	}
+
+	if ((*char_len = (uint8_t *)contextMalloc(context, char_cnt)) == NULL)
+		return -1;
+
+	int i = 0, j = 0;
+	while(j < char_cnt) {
+		int old_i = i;
+		SQL_UTF8_FWD_1(trim_set, i, trim_set_sz);
+		(*char_len)[j++] = i - old_i;
+	}
+
+	return char_cnt;
+}
+
+/**
+ * Normalize args from @a argv input array when it has one arg
+ * only.
+ *
+ * Case: TRIM(<str>)
+ * Call trimming procedure with TRIM_BOTH as the flags and " " as
+ * the trimming set.
+ */
+static void
+trim_func_one_arg(struct sql_context *context, int argc, sql_value **argv)
+{
+	assert(argc == 1);
+	(void) argc;
+
+	const unsigned char *input_str;
+	if ((input_str = sql_value_text(argv[0])) == NULL)
+		return;
+
+	int input_str_sz = sql_value_bytes(argv[0]);
+	uint8_t len_one = 1;
+	trim_procedure(context, TRIM_BOTH, (const unsigned char *) " ",
+		       &len_one, 1, input_str, input_str_sz);
+}
+
+/**
+ * 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.
+ */
+static void
+trim_func_two_args(struct sql_context *context, int argc, sql_value **argv)
+{
+	assert(argc == 2);
+	(void) argc;
+
+	const unsigned char *input_str, *trim_set;
+	if ((input_str = sql_value_text(argv[1])) == NULL)
+		return;
+
+	int input_str_sz = sql_value_bytes(argv[1]);
+	if (sql_value_type(argv[0]) == SQL_INTEGER) {
+		uint8_t len_one = 1;
+		trim_procedure(context, sql_value_int(argv[0]),
+			       (const unsigned char *) " ", &len_one, 1,
+			       input_str, input_str_sz);
+	} else if ((trim_set = sql_value_text(argv[0])) != NULL) {
+		int trim_set_sz = sql_value_bytes(argv[0]);
+		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);
+	}
+}
+
+/**
+ * Normalize args from @a argv input array when it has three args.
+ *
+ * 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.
+ */
+static void
+trim_func_three_args(struct sql_context *context, int argc, sql_value **argv)
+{
+	assert(argc == 3);
+	(void) argc;
+
+	assert(sql_value_type(argv[0]) == SQL_INTEGER);
+	const unsigned char *input_str, *trim_set;
+	if ((input_str = sql_value_text(argv[2])) == NULL ||
+	    (trim_set = sql_value_text(argv[1])) == NULL)
+		return;
+
+	int trim_set_sz = sql_value_bytes(argv[1]);
+	int input_str_sz = sql_value_bytes(argv[2]);
+	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(argv[0]), trim_set, char_len,
+		       char_cnt, input_str, input_str_sz);
+	sql_free(char_len);
 }
 
 #ifdef SQL_ENABLE_UNKNOWN_SQL_FUNCTION
@@ -1810,12 +1898,9 @@ sqlRegisterBuiltinFunctions(void)
 			  FIELD_TYPE_INTEGER),
 		FUNCTION2(likely, 1, 0, 0, noopFunc, SQL_FUNC_UNLIKELY,
 			  FIELD_TYPE_INTEGER),
-		FUNCTION_COLL(ltrim, 1, 1, 0, trimFunc),
-		FUNCTION_COLL(ltrim, 2, 1, 0, trimFunc),
-		FUNCTION_COLL(rtrim, 1, 2, 0, trimFunc),
-		FUNCTION_COLL(rtrim, 2, 2, 0, trimFunc),
-		FUNCTION_COLL(trim, 1, 3, 0, trimFunc),
-		FUNCTION_COLL(trim, 2, 3, 0, trimFunc),
+		FUNCTION_COLL(trim, 1, 3, 0, trim_func_one_arg),
+		FUNCTION_COLL(trim, 2, 3, 0, trim_func_two_args),
+		FUNCTION_COLL(trim, 3, 3, 0, trim_func_three_args),
 		FUNCTION(min, -1, 0, 1, minmaxFunc, FIELD_TYPE_SCALAR),
 		FUNCTION(min, 0, 0, 1, 0, FIELD_TYPE_SCALAR),
 		AGGREGATE2(min, 1, 0, 1, minmaxStep, minMaxFinalize,
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 099daf512f..a56ce7a101 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1032,6 +1032,55 @@ expr(A) ::= CAST(X) LP expr(E) AS typedef(T) RP(Y). {
   sqlExprAttachSubtrees(pParse->db, A.pExpr, E.pExpr, 0);
 }
 %endif  SQL_OMIT_CAST
+
+expr(A) ::= TRIM(X) LP trim_operands(Y) RP(E). {
+  A.pExpr = sqlExprFunction(pParse, Y, &X);
+  spanSet(&A, &X, &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) ::= expr(Y). {
+  A = sql_expr_list_append(pParse->db, NULL, Y.pExpr);
+}
+
+%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. {
+  A = sql_expr_list_append(pParse->db, NULL, Y.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);
+}
+
+%type expr_optional {struct Expr *}
+%destructor expr_optional {sql_expr_delete(pParse->db, $$, false);}
+
+expr_optional(A) ::= .        { A = NULL; }
+expr_optional(A) ::= expr(X). { A = X.pExpr; }
+
+%type trim_specification {enum trim_side_mask}
+
+trim_specification(A) ::= LEADING.  { A = TRIM_LEADING; }
+trim_specification(A) ::= TRAILING. { A = TRIM_TRAILING; }
+trim_specification(A) ::= BOTH.     { A = TRIM_BOTH; }
+
 expr(A) ::= id(X) LP distinct(D) exprlist(Y) RP(E). {
   if( Y && Y->nExpr>pParse->db->aLimit[SQL_LIMIT_FUNCTION_ARG] ){
     const char *err =
@@ -1294,7 +1343,7 @@ expr(A) ::= EXISTS(B) LP select(Y) RP(E). {
 }
 
 /* CASE expressions */
-expr(A) ::= CASE(C) case_operand(X) case_exprlist(Y) case_else(Z) END(E). {
+expr(A) ::= CASE(C) expr_optional(X) case_exprlist(Y) case_else(Z) END(E). {
   spanSet(&A,&C,&E);  /*A-overwrites-C*/
   A.pExpr = sqlPExpr(pParse, TK_CASE, X, 0);
   if( A.pExpr ){
@@ -1319,10 +1368,6 @@ case_exprlist(A) ::= WHEN expr(Y) THEN expr(Z). {
 %destructor case_else {sql_expr_delete(pParse->db, $$, false);}
 case_else(A) ::=  ELSE expr(X).         {A = X.pExpr;}
 case_else(A) ::=  .                     {A = 0;} 
-%type case_operand {Expr*}
-%destructor case_operand {sql_expr_delete(pParse->db, $$, false);}
-case_operand(A) ::= expr(X).            {A = X.pExpr; /*A-overwrites-X*/} 
-case_operand(A) ::= .                   {A = 0;} 
 
 %type exprlist {ExprList*}
 %destructor exprlist {sql_expr_list_delete(pParse->db, $$);}
diff --git a/src/box/sql/parse_def.c b/src/box/sql/parse_def.c
index 49c76a3267..aa1323cb21 100644
--- a/src/box/sql/parse_def.c
+++ b/src/box/sql/parse_def.c
@@ -34,7 +34,9 @@
 
 const struct Token sqlIntTokens[] = {
 	{"0", 1, false},
-	{"1", 1, false}
+	{"1", 1, false},
+	{"2", 1, false},
+	{"3", 1, false},
 };
 
 void
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index a1af2bacd3..5899a7e4e4 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -87,7 +87,7 @@ struct Token {
 	bool isReserved;
 };
 
-/** Constant tokens for values 0 and 1. */
+/** Constant tokens for integer values. */
 extern const struct Token sqlIntTokens[];
 
 /** Generate a Token object from a string. */
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 828ab839ee..8c8d0be1cc 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -1634,6 +1634,17 @@ struct FuncDestructor {
 					 * single query - might change over time
 					 */
 
+/*
+ * Trim side mask components. TRIM_LEADING means to trim left side
+ * only. TRIM_TRAILING is to trim right side only. TRIM_BOTH is to
+ * trim both sides.
+ */
+enum trim_side_mask {
+	TRIM_LEADING = 1,
+	TRIM_TRAILING = 2,
+	TRIM_BOTH = TRIM_LEADING | TRIM_TRAILING
+};
+
 /*
  * The following three macros, FUNCTION(), LIKEFUNC() and AGGREGATE() are
  * used to create the initializers for the FuncDef structures.
diff --git a/test/sql-tap/badutf1.test.lua b/test/sql-tap/badutf1.test.lua
index d104efaa98..9079dfe258 100755
--- a/test/sql-tap/badutf1.test.lua
+++ b/test/sql-tap/badutf1.test.lua
@@ -302,7 +302,7 @@ test:do_test(
 test:do_test(
     "badutf-4.1",
     function()
-        return test:execsql2("SELECT hex(trim('\x80\x80\x80\xf0\x80\x80\x80\xff','\x80\xff')) AS x")
+        return test:execsql2("SELECT hex(TRIM('\x80\xff' FROM '\x80\x80\x80\xf0\x80\x80\x80\xff')) AS x")
     end, {
         -- <badutf-4.1>
         "X", "F0"
@@ -312,7 +312,7 @@ test:do_test(
 test:do_test(
     "badutf-4.2",
     function()
-        return test:execsql2("SELECT hex(ltrim('\x80\x80\x80\xf0\x80\x80\x80\xff','\x80\xff')) AS x")
+        return test:execsql2("SELECT hex(TRIM(LEADING '\x80\xff' FROM '\x80\x80\x80\xf0\x80\x80\x80\xff')) AS x")
     end, {
         -- <badutf-4.2>
         "X", "F0808080FF"
@@ -322,7 +322,7 @@ test:do_test(
 test:do_test(
     "badutf-4.3",
     function()
-        return test:execsql2("SELECT hex(rtrim('\x80\x80\x80\xf0\x80\x80\x80\xff','\x80\xff')) AS x")
+        return test:execsql2("SELECT hex(TRIM(TRAILING '\x80\xff' FROM '\x80\x80\x80\xf0\x80\x80\x80\xff')) AS x")
     end, {
         -- <badutf-4.3>
         "X", "808080F0"
@@ -332,7 +332,7 @@ test:do_test(
 test:do_test(
     "badutf-4.4",
     function()
-        return test:execsql2("SELECT hex(trim('\x80\x80\x80\xf0\x80\x80\x80\xff','\xff\x80')) AS x")
+        return test:execsql2("SELECT hex(TRIM('\xff\x80' FROM '\x80\x80\x80\xf0\x80\x80\x80\xff')) AS x")
     end, {
         -- <badutf-4.4>
         "X", "808080F0808080FF"
@@ -342,7 +342,7 @@ test:do_test(
 test:do_test(
     "badutf-4.5",
     function()
-        return test:execsql2("SELECT hex(trim('\xff\x80\x80\xf0\x80\x80\x80\xff','\xff\x80')) AS x")
+        return test:execsql2("SELECT hex(TRIM('\xff\x80' FROM '\xff\x80\x80\xf0\x80\x80\x80\xff')) AS x")
     end, {
         -- <badutf-4.5>
         "X", "80F0808080FF"
@@ -352,7 +352,7 @@ test:do_test(
 test:do_test(
     "badutf-4.6",
     function()
-        return test:execsql2("SELECT hex(trim('\xff\x80\xf0\x80\x80\x80\xff','\xff\x80')) AS x")
+        return test:execsql2("SELECT hex(TRIM('\xff\x80' FROM '\xff\x80\xf0\x80\x80\x80\xff')) AS x")
     end, {
         -- <badutf-4.6>
         "X", "F0808080FF"
@@ -362,7 +362,7 @@ test:do_test(
 test:do_test(
     "badutf-4.7",
     function()
-        return test:execsql2("SELECT hex(trim('\xff\x80\xf0\x80\x80\x80\xff','\xff\x80\x80')) AS x")
+        return test:execsql2("SELECT hex(TRIM('\xff\x80\x80' FROM '\xff\x80\xf0\x80\x80\x80\xff')) AS x")
     end, {
         -- <badutf-4.7>
         "X", "FF80F0808080FF"
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index 251cc35341..fe9a98191c 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(14586)
+test:plan(14590)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -1915,7 +1915,7 @@ test:do_catchsql_test(
         SELECT trim(1,2,3)
     ]], {
         -- <func-22.1>
-        1, "wrong number of arguments to function TRIM()"
+        1, "Syntax error near ','"
         -- </func-22.1>
     })
 
@@ -1925,7 +1925,7 @@ test:do_catchsql_test(
         SELECT ltrim(1,2,3)
     ]], {
         -- <func-22.2>
-        1, "wrong number of arguments to function LTRIM()"
+        1, "Function 'LTRIM' does not exist"
         -- </func-22.2>
     })
 
@@ -1935,7 +1935,7 @@ test:do_catchsql_test(
         SELECT rtrim(1,2,3)
     ]], {
         -- <func-22.3>
-        1, "wrong number of arguments to function RTRIM()"
+        1, "Function 'RTRIM' does not exist"
         -- </func-22.3>
     })
 
@@ -1952,7 +1952,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.5",
     [[
-        SELECT ltrim('  hi  ');
+        SELECT TRIM(LEADING FROM '  hi  ');
     ]], {
         -- <func-22.5>
         "hi  "
@@ -1962,7 +1962,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.6",
     [[
-        SELECT rtrim('  hi  ');
+        SELECT TRIM(TRAILING FROM '  hi  ');
     ]], {
         -- <func-22.6>
         "  hi"
@@ -1972,7 +1972,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.7",
     [[
-        SELECT trim('  hi  ','xyz');
+        SELECT TRIM('xyz' FROM '  hi  ');
     ]], {
         -- <func-22.7>
         "  hi  "
@@ -1982,7 +1982,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.8",
     [[
-        SELECT ltrim('  hi  ','xyz');
+        SELECT TRIM(LEADING 'xyz' FROM '  hi  ');
     ]], {
         -- <func-22.8>
         "  hi  "
@@ -1992,7 +1992,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.9",
     [[
-        SELECT rtrim('  hi  ','xyz');
+        SELECT TRIM(TRAILING 'xyz' FROM '  hi  ');
     ]], {
         -- <func-22.9>
         "  hi  "
@@ -2002,7 +2002,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.10",
     [[
-        SELECT trim('xyxzy  hi  zzzy','xyz');
+        SELECT TRIM('xyz' FROM 'xyxzy  hi  zzzy');
     ]], {
         -- <func-22.10>
         "  hi  "
@@ -2012,7 +2012,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.11",
     [[
-        SELECT ltrim('xyxzy  hi  zzzy','xyz');
+        SELECT TRIM(LEADING 'xyz' FROM 'xyxzy  hi  zzzy');
     ]], {
         -- <func-22.11>
         "  hi  zzzy"
@@ -2022,7 +2022,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.12",
     [[
-        SELECT rtrim('xyxzy  hi  zzzy','xyz');
+        SELECT TRIM(TRAILING 'xyz' FROM 'xyxzy  hi  zzzy');
     ]], {
         -- <func-22.12>
         "xyxzy  hi  "
@@ -2032,7 +2032,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.13",
     [[
-        SELECT trim('  hi  ','');
+        SELECT TRIM('' FROM '  hi  ');
     ]], {
         -- <func-22.13>
         "  hi  "
@@ -2043,7 +2043,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.14",
     [[
-        SELECT hex(trim(x'c280e1bfbff48fbfbf6869',x'6162e1bfbfc280'))
+        SELECT hex(TRIM(x'6162e1bfbfc280' FROM x'c280e1bfbff48fbfbf6869'))
     ]], {
         -- <func-22.14>
         "F48FBFBF6869"
@@ -2052,8 +2052,8 @@ test:do_execsql_test(
 
 test:do_execsql_test(
     "func-22.15",
-    [[SELECT hex(trim(x'6869c280e1bfbff48fbfbf61',
-                         x'6162e1bfbfc280f48fbfbf'))]], {
+    [[SELECT hex(TRIM(x'6162e1bfbfc280f48fbfbf'
+                      FROM x'6869c280e1bfbff48fbfbf61'))]], {
         -- <func-22.15>
         "6869"
         -- </func-22.15>
@@ -2062,7 +2062,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.16",
     [[
-        SELECT hex(trim(x'ceb1ceb2ceb3',x'ceb1'));
+        SELECT hex(TRIM(x'ceb1' FROM x'ceb1ceb2ceb3'));
     ]], {
         -- <func-22.16>
         "CEB2CEB3"
@@ -2083,7 +2083,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.21",
     [[
-        SELECT typeof(trim(NULL,'xyz'));
+        SELECT typeof(TRIM('xyz' FROM NULL));
     ]], {
         -- <func-22.21>
         "null"
@@ -2093,7 +2093,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.22",
     [[
-        SELECT typeof(trim('hello',NULL));
+        SELECT typeof(TRIM(NULL FROM 'hello'));
     ]], {
         -- <func-22.22>
         "null"
@@ -2105,7 +2105,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.23",
     [[
-        SELECT TRIM(X'004100', X'00');
+        SELECT TRIM(X'00' FROM X'004100');
     ]], {
         -- <func-22.23>
         "A"
@@ -2115,7 +2115,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.24",
     [[
-        SELECT TRIM(X'004100', X'0000');
+        SELECT TRIM(X'0000' FROM X'004100');
     ]], {
         -- <func-22.24>
         "A"
@@ -2125,7 +2125,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.25",
     [[
-        SELECT TRIM(X'004100', X'0042');
+        SELECT TRIM(X'0042' FROM X'004100');
     ]], {
         -- <func-22.25>
         "A"
@@ -2135,7 +2135,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.26",
     [[
-        SELECT TRIM(X'00004100420000', X'00');
+        SELECT TRIM(X'00' FROM X'00004100420000');
     ]], {
         -- <func-22.26>
         "A\0B"
@@ -2145,7 +2145,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.27",
     [[
-        SELECT LTRIM(X'004100', X'00');
+        SELECT TRIM(LEADING X'00' FROM X'004100');
     ]], {
         -- <func-22.27>
         "A\0"
@@ -2155,7 +2155,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.28",
     [[
-        SELECT LTRIM(X'004100', X'0000');
+        SELECT TRIM(LEADING X'0000' FROM X'004100');
     ]], {
         -- <func-22.28>
         "A\0"
@@ -2165,7 +2165,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.29",
     [[
-        SELECT LTRIM(X'004100', X'0042');
+        SELECT TRIM(LEADING X'0042' FROM X'004100');
     ]], {
         -- <func-22.29>
         "A\0"
@@ -2175,7 +2175,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.30",
     [[
-        SELECT LTRIM(X'00004100420000', X'00');
+        SELECT TRIM(LEADING X'00' FROM X'00004100420000');
     ]], {
         -- <func-22.30>
         "A\0B\0\0"
@@ -2185,7 +2185,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.31",
     [[
-        SELECT RTRIM(X'004100', X'00');
+        SELECT TRIM(TRAILING X'00' FROM X'004100');
     ]], {
         -- <func-22.31>
         "\0A"
@@ -2195,7 +2195,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.32",
     [[
-        SELECT RTRIM(X'004100', X'0000');
+        SELECT TRIM(TRAILING X'0000' FROM X'004100');
     ]], {
         -- <func-22.32>
         "\0A"
@@ -2205,7 +2205,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.33",
     [[
-        SELECT RTRIM(X'004100', X'0042');
+        SELECT TRIM(TRAILING X'0042' FROM X'004100');
     ]], {
         -- <func-22.33>
         "\0A"
@@ -2215,13 +2215,56 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-22.34",
     [[
-        SELECT RTRIM(X'00004100420000', X'00');
+        SELECT TRIM(TRAILING X'00' FROM X'00004100420000');
     ]], {
         -- <func-22.34>
         "\0\0A\0B"
         -- </func-22.34>
     })
 
+-- gh-3879 Check new TRIM() grammar, particularly BOTH keyword and
+-- FROM without any agrs before. LEADING and TRAILING keywords is
+-- checked above.
+
+test:do_execsql_test(
+    "func-22.35",
+    [[
+        SELECT TRIM(BOTH FROM '  hi  ');
+    ]], {
+        -- <func-22.35>
+        "hi"
+        -- </func-22.35>
+    })
+test:do_execsql_test(
+    "func-22.36",
+    [[
+        SELECT TRIM(BOTH 'xyz' FROM '  hi  ');
+    ]], {
+        -- <func-22.36>
+        "  hi  "
+        -- </func-22.36>
+    })
+
+test:do_execsql_test(
+    "func-22.37",
+    [[
+        SELECT TRIM(BOTH 'xyz' FROM 'xyxzy  hi  zzzy');
+    ]], {
+        -- <func-22.37>
+        "  hi  "
+        -- </func-22.37>
+    })
+
+test:do_catchsql_test(
+    "func-22.38",
+    [[
+        SELECT TRIM(FROM 'xyxzy');
+    ]], {
+        -- <func-22.38>
+        1, "Syntax error near 'FROM'"
+        -- </func-22.38>
+    })
+
 -- This is to test the deprecated sql_aggregate_count() API.
 --
 --test:do_test(
@@ -2838,16 +2881,16 @@ test:do_execsql_test(
     "SELECT TRIM(CHAR(32,00,32,00,32));",
     {string.char(00,32,00)})
 
--- LTRIM
+-- LEFT TRIM
 test:do_execsql_test(
     "func-70",
-    "SELECT LTRIM(CHAR(32,00,32,00,32));",
+    "SELECT TRIM(LEADING FROM CHAR(32,00,32,00,32));",
     {string.char(00,32,00,32)})
 
--- RTRIM
+-- RIGHT TRIM
 test:do_execsql_test(
     "func-71",
-    "SELECT RTRIM(CHAR(32,00,32,00,32));",
+    "SELECT TRIM(TRAILING FROM CHAR(32,00,32,00,32));",
     {string.char(32,00,32,00)})
 
 -- GROUP_CONCAT
diff --git a/test/sql-tap/with1.test.lua b/test/sql-tap/with1.test.lua
index 495aa4ee47..ec45e5e76b 100755
--- a/test/sql-tap/with1.test.lua
+++ b/test/sql-tap/with1.test.lua
@@ -550,7 +550,7 @@ test:do_execsql_test("8.1-mandelbrot", [[
       SELECT group_concat( substr(' .+*#', 1+min(iter/7,4), 1), '') 
       FROM m2 GROUP BY cy
     )
-  SELECT group_concat(rtrim(t),x'0a') FROM a;
+  SELECT group_concat(TRIM(TRAILING FROM t),x'0a') FROM a;
 ]], {
   -- <8.1-mandelbrot>
   [[                                    ....#
-- 
GitLab