Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: Roman Khabibov <roman.habibov@tarantool.org>,
	tarantool-patches@freelists.org,
	Kirill Yukhin <kyukhin@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH] sql: modify TRIM() function signature
Date: Tue, 23 Apr 2019 11:59:02 +0300	[thread overview]
Message-ID: <67cf8aa1-563f-c9aa-13b2-4d1ce0197e30@tarantool.org> (raw)
In-Reply-To: <ACBE6D8A-54AA-47A1-BD63-B41A2EC2875C@tarantool.org>

LGTM.

On 23/04/2019 04:04, Roman Khabibov wrote:
> Hello! Of course, I agree with you.
> 
>> On Apr 22, 2019, at 9:22 PM, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>> Hi! Thanks for the fixes! I've applied my
>> fixes and pushed on top of the branch. Please,
>> look at them and either squash, or lets discuss
>> where you do not agree. Otherwise it will LGTM.
> 
> commit 368f588a6200653adebf20372e1e64c0fae8b9f3
> Author: Roman Khabibov <roman.habibov@tarantool.org>
> Date:   Thu Mar 28 14:01:33 2019 +0300
> 
>     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.
> 
> diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
> index be7bd5545..76e3265e7 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 abeecefa1..2bbb2ad4a 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -1286,108 +1286,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
> @@ -1818,12 +1906,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 099daf512..a56ce7a10 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 49c76a326..aa1323cb2 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 a1af2bacd..5899a7e4e 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 b322602dc..d5a3e15c1 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -1680,6 +1680,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 d104efaa9..9079dfe25 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 251cc3534..fe9a98191 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 495aa4ee4..ec45e5e76 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>
>    [[                                    ....#
> 
> 

  reply	other threads:[~2019-04-23  8:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 17:33 [tarantool-patches] " Roman Khabibov
2019-04-14 18:01 ` [tarantool-patches] " Vladislav Shpilevoy
2019-04-16  0:14   ` Roman Khabibov
2019-04-16 17:14     ` Vladislav Shpilevoy
2019-04-18 17:11       ` Roman Khabibov
2019-04-19 12:49         ` Vladislav Shpilevoy
2019-04-20  0:48           ` Roman Khabibov
2019-04-21 19:36             ` Vladislav Shpilevoy
2019-04-22 10:43               ` Vladislav Shpilevoy
2019-04-22 16:45               ` Roman Khabibov
2019-04-22 18:22                 ` Vladislav Shpilevoy
2019-04-23  1:04                   ` Roman Khabibov
2019-04-23  8:59                     ` Vladislav Shpilevoy [this message]
2019-04-23 10:21 ` Kirill Yukhin

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=67cf8aa1-563f-c9aa-13b2-4d1ce0197e30@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=kyukhin@tarantool.org \
    --cc=roman.habibov@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH] sql: modify TRIM() function signature' \
    /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