[tarantool-patches] [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

[tarantool-patches] [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

[imeevma]

This patch creates SQL functions CHARACTER_LENGTH() and
CHAR_LENGTH(). These functions work the same way. Each of them
takes only one argument of type TEXT and returns its length.

Closes #3929

@TarantoolBot document
Title: SQL functions CHAR_LENGTH() and CHARACTER_LENGTH()

The SQL functions CHAR_LENGTH() and CHARACTER_LENGTH() take
exactly one argument of type TEXT and return its length. They
throw an error if the argument is of a different type or if more
than one argument is given.
---
https://github.com/tarantool/tarantool/issues/3929
https://github.com/tarantool/tarantool/tree/imeevma/gh-3929-character-length-functions

 src/box/sql/func.c          |  27 +++++++++
 test/sql-tap/func3.test.lua | 130 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index bb7405e..01c271c 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -133,6 +133,29 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
 }
 
 /*
+ * Implementation of the char_length() and character_length()
+ * functions.
+ */
+static void
+char_length_func(sql_context *context, int argc, sql_value **argv)
+{
+	assert(argc == 1);
+	UNUSED_PARAMETER(argc);
+	if (sql_value_type(argv[0]) != MP_STR) {
+		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT",
+			 mem_type_to_str(argv[0]));
+		context->isError = SQL_TARANTOOL_ERROR;
+		context->fErrorOrAux = 1;
+		return;
+	}
+	const unsigned char *z = sql_value_text(argv[0]);
+	if (z == NULL)
+		return;
+	int len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
+	sql_result_int(context, len);
+}
+
+/*
  * Implementation of the length() function
  */
 static void
@@ -1936,6 +1959,10 @@ sqlRegisterBuiltinFunctions(void)
 			  FIELD_TYPE_STRING),
 		FUNCTION2(length, 1, 0, 0, lengthFunc, SQL_FUNC_LENGTH,
 			  FIELD_TYPE_INTEGER),
+		FUNCTION(char_length, 1, 0, 0, char_length_func,
+			 FIELD_TYPE_INTEGER),
+		FUNCTION(character_length, 1, 0, 0, char_length_func,
+			 FIELD_TYPE_INTEGER),
 		FUNCTION(position, 2, 0, 1, position_func, FIELD_TYPE_INTEGER),
 		FUNCTION(printf, -1, 0, 0, printfFunc, FIELD_TYPE_STRING),
 		FUNCTION(unicode, 1, 0, 0, unicodeFunc, FIELD_TYPE_STRING),
diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
index 6d6411c..a3a548b 100755
--- a/test/sql-tap/func3.test.lua
+++ b/test/sql-tap/func3.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(25)
+test:plan(37)
 
 --!./tcltestrunner.lua
 -- 2010 August 27
@@ -284,6 +284,134 @@ test:do_test(
         return test:execsql "EXPLAIN SELECT likely(min(1.0+'2.0',4*11))"
     end, test:execsql "EXPLAIN SELECT min(1.0+'2.0',4*11)")
 
+--
+-- gh-3929: create SQL functions CHARACTER_LENGTH() and
+-- CHAR_LENGTH().
+--
+test:do_test(
+	"func3-6.1",
+	function()
+		local some_text = 'some text'
+		return test:execsql(string.format([[
+			SELECT LENGTH('%s'), CHAR_LENGTH('%s'), CHARACTER_LENGTH('%s');
+		]], some_text, some_text, some_text))
+	end, {
+		-- <func3-6.1>
+		9, 9, 9
+		-- </func3-6.1>
+	})
+
+test:do_test(
+	"func3-6.2",
+	function()
+		local some_text = 'какой-то текст'
+		return test:execsql(string.format([[
+			SELECT LENGTH('%s'), CHAR_LENGTH('%s'), CHARACTER_LENGTH('%s');
+		]], some_text, some_text, some_text))
+	end, {
+		-- <func3-6.2>
+		14, 14, 14
+		-- </func3-6.2>
+	})
+
+test:do_catchsql_test(
+	"func3-6.3",
+	[[
+		SELECT CHAR_LENGTH(12);
+	]], {
+		-- <func3-6.3>
+		1,"Inconsistent types: expected TEXT got INTEGER"
+		-- </func3-6.3>
+	})
+
+test:do_catchsql_test(
+	"func3-6.4",
+	[[
+		SELECT CHAR_LENGTH(12.34);
+	]], {
+		-- <func3-6.4>
+		1,"Inconsistent types: expected TEXT got REAL"
+		-- </func3-6.4>
+	})
+
+test:do_catchsql_test(
+	"func3-6.5",
+	[[
+		SELECT CHAR_LENGTH(x'12');
+	]], {
+		-- <func3-6.5>
+		1,"Inconsistent types: expected TEXT got BLOB"
+		-- </func3-6.5>
+	})
+
+test:do_catchsql_test(
+	"func3-6.6",
+	[[
+		SELECT CHAR_LENGTH(true);
+	]], {
+		-- <func3-6.6>
+		1,"Inconsistent types: expected TEXT got BOOLEAN"
+		-- </func3-6.6>
+	})
+
+test:do_catchsql_test(
+	"func3-6.7",
+	[[
+		SELECT CHARACTER_LENGTH(12);
+	]], {
+		-- <func3-6.7>
+		1,"Inconsistent types: expected TEXT got INTEGER"
+		-- </func3-6.7>
+	})
+
+test:do_catchsql_test(
+	"func3-6.8",
+	[[
+		SELECT CHARACTER_LENGTH(12.34);
+	]], {
+		-- <func3-6.8>
+		1,"Inconsistent types: expected TEXT got REAL"
+		-- </func3-6.8>
+	})
+
+test:do_catchsql_test(
+	"func3-6.9",
+	[[
+		SELECT CHARACTER_LENGTH(x'12');
+	]], {
+		-- <func3-6.9>
+		1,"Inconsistent types: expected TEXT got BLOB"
+		-- </func3-6.9>
+	})
+
+test:do_catchsql_test(
+	"func3-6.10",
+	[[
+		SELECT CHARACTER_LENGTH(true);
+	]], {
+		-- <func3-6.10>
+		1,"Inconsistent types: expected TEXT got BOOLEAN"
+		-- </func3-6.10>
+	})
 
+test:do_catchsql_test(
+	"func3-6.11",
+	[[
+		SELECT CHAR_LENGTH('abc', 'efg');
+	]], {
+		-- <func3-6.6>
+		1,"wrong number of arguments to function CHAR_LENGTH()"
+		-- </func3-6.6>
+	})
+
+test:do_catchsql_test(
+	"func3-6.12",
+	[[
+		SELECT CHARACTER_LENGTH('abc', 'efg');
+	]], {
+		-- <func3-6.7>
+		1,"wrong number of arguments to function CHARACTER_LENGTH()"
+		-- </func3-6.7>
+	})
 
 test:finish_test()
-- 
2.7.4

[tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

[n.pettik]

> On 8 May 2019, at 16:15, imeevma@tarantool.org wrote:
> 
> This patch creates SQL functions CHARACTER_LENGTH() and
> CHAR_LENGTH(). These functions work the same way. Each of them
> takes only one argument of type TEXT and returns its length.

I see your message in issue description:

“
About this function, it was decided that it would be better to
implement it as "{CHAR_LENGTH | CHARACTER_LENGTH}
(character value expression)" (without "[USING CHARACTERS]”)
“

Could you please provide explanation of this decision?
What then is the difference between length() and char_length()?

I see that char_length() in your implementation doesn’t tolerate
NULLs. Is it OK? Could you check this fact using ANSI spec?

> /*
> + * Implementation of the char_length() and character_length()
> + * functions.
> + */

Nit:

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 01c271cab..b5fdc6f20 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -132,7 +132,7 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
        sql_result_text(context, z, -1, SQL_STATIC);
 }
 
-/*
+/**
  * Implementation of the char_length() and character_length()
  * functions.
  */


> diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
> index 6d6411c..a3a548b 100755
> --- a/test/sql-tap/func3.test.lua
> +++ b/test/sql-tap/func3.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> test = require("sqltester")
> -test:plan(25)
> +test:plan(37)
> 
> --!./tcltestrunner.lua
> -- 2010 August 27
> @@ -284,6 +284,134 @@ test:do_test(
>         return test:execsql "EXPLAIN SELECT likely(min(1.0+'2.0',4*11))"
>     end, test:execsql "EXPLAIN SELECT min(1.0+'2.0',4*11)”)

Add test involving invalid utf8 characters.

[tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

[Mergen Imeev]

Hi! Thank you for review! My answers and new version below.

On Tue, May 21, 2019 at 03:58:55PM +0300, n.pettik wrote:
> 
> 
> > On 8 May 2019, at 16:15, imeevma@tarantool.org wrote:
> > 
> > This patch creates SQL functions CHARACTER_LENGTH() and
> > CHAR_LENGTH(). These functions work the same way. Each of them
> > takes only one argument of type TEXT and returns its length.
> 
> I see your message in issue description:
> 
> “
> About this function, it was decided that it would be better to
> implement it as "{CHAR_LENGTH | CHARACTER_LENGTH}
> (character value expression)" (without "[USING CHARACTERS]”)
> “
> 
> Could you please provide explanation of this decision?
Since this issue is not of primary importance, after some
discussion it was decided that it would be too troublesome to
create these functions with the support of “[USING CHARACTERS]”.

> What then is the difference between length() and char_length()?
> 
I think the most noticeable difference is that the CHAR_LENGTH and
CHARACTER_LENGTH functions are described in ANSI, but LENGTH is
not.

> I see that char_length() in your implementation doesn’t tolerate
> NULLs. Is it OK? Could you check this fact using ANSI spec?
> 
You are right:
"General Rules
1) If the value of any <character primary> or <blob primary>
simply contained in a <string value expression> is the null value,
then the result of the <string value expression> is the null
value."
I fixed this.

> > /*
> > + * Implementation of the char_length() and character_length()
> > + * functions.
> > + */
> 
> Nit:
> 
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 01c271cab..b5fdc6f20 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -132,7 +132,7 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
>         sql_result_text(context, z, -1, SQL_STATIC);
>  }
>  
> -/*
> +/**
>   * Implementation of the char_length() and character_length()
>   * functions.
>   */
> 
> 
Thanks, fixed.

> > diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
> > index 6d6411c..a3a548b 100755
> > --- a/test/sql-tap/func3.test.lua
> > +++ b/test/sql-tap/func3.test.lua
> > @@ -1,6 +1,6 @@
> > #!/usr/bin/env tarantool
> > test = require("sqltester")
> > -test:plan(25)
> > +test:plan(37)
> > 
> > --!./tcltestrunner.lua
> > -- 2010 August 27
> > @@ -284,6 +284,134 @@ test:do_test(
> >         return test:execsql "EXPLAIN SELECT likely(min(1.0+'2.0',4*11))"
> >     end, test:execsql "EXPLAIN SELECT min(1.0+'2.0',4*11)”)
> 
> Add test involving invalid utf8 characters.
> 
Sorry, I could not understand what it means. Do you mean
something like this (not exactly invalid characters, but
still):
tarantool> box.execute("SELECT CHAR_LENGTH('\u{12345}');")
---
- metadata:
  - name: !!binary Q0hBUl9MRU5HVEgoJ/CSjYUnKQ==
    type: integer
  rows:
  - [1]
...

I'm not sure if this should be added.

I added a test with utf8 characters.


New patch:

From 150042175efa4e84b420fc90e02c5b70c4151d36 Mon Sep 17 00:00:00 2001
Date: Tue, 7 May 2019 20:33:39 +0300
Subject: [PATCH] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

This patch creates SQL functions CHARACTER_LENGTH() and
CHAR_LENGTH(). These functions work the same way. Each of them
takes only one argument of type TEXT and returns its length.

Closes #3929

@TarantoolBot document
Title: SQL functions CHAR_LENGTH() and CHARACTER_LENGTH()

The SQL functions CHAR_LENGTH() and CHARACTER_LENGTH() take
exactly one argument of type TEXT and return its length. They
throw an error if the argument is of a different type or if more
than one argument is given.

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index bb7405e..210a1de 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -132,6 +132,33 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
 	sql_result_text(context, z, -1, SQL_STATIC);
 }
 
+/**
+ * Implementation of the char_length() and character_length()
+ * functions.
+ */
+static void
+char_length_func(sql_context *context, int argc, sql_value **argv)
+{
+	assert(argc == 1);
+	UNUSED_PARAMETER(argc);
+	if (sql_value_type(argv[0]) == MP_NIL) {
+		sql_result_null(context);
+		return;
+	}
+	if (sql_value_type(argv[0]) != MP_STR) {
+		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT",
+			 mem_type_to_str(argv[0]));
+		context->isError = SQL_TARANTOOL_ERROR;
+		context->fErrorOrAux = 1;
+		return;
+	}
+	const unsigned char *z = sql_value_text(argv[0]);
+	if (z == NULL)
+		return;
+	int len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
+	sql_result_int(context, len);
+}
+
 /*
  * Implementation of the length() function
  */
@@ -1936,6 +1963,10 @@ sqlRegisterBuiltinFunctions(void)
 			  FIELD_TYPE_STRING),
 		FUNCTION2(length, 1, 0, 0, lengthFunc, SQL_FUNC_LENGTH,
 			  FIELD_TYPE_INTEGER),
+		FUNCTION(char_length, 1, 0, 0, char_length_func,
+			 FIELD_TYPE_INTEGER),
+		FUNCTION(character_length, 1, 0, 0, char_length_func,
+			 FIELD_TYPE_INTEGER),
 		FUNCTION(position, 2, 0, 1, position_func, FIELD_TYPE_INTEGER),
 		FUNCTION(printf, -1, 0, 0, printfFunc, FIELD_TYPE_STRING),
 		FUNCTION(unicode, 1, 0, 0, unicodeFunc, FIELD_TYPE_STRING),
diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
index 6d6411c..20f0aa7 100755
--- a/test/sql-tap/func3.test.lua
+++ b/test/sql-tap/func3.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(25)
+test:plan(40)
 
 --!./tcltestrunner.lua
 -- 2010 August 27
@@ -284,6 +284,167 @@ test:do_test(
         return test:execsql "EXPLAIN SELECT likely(min(1.0+'2.0',4*11))"
     end, test:execsql "EXPLAIN SELECT min(1.0+'2.0',4*11)")
 
+--
+-- gh-3929: create SQL functions CHARACTER_LENGTH() and
+-- CHAR_LENGTH().
+--
+test:do_test(
+	"func3-6.1",
+	function()
+		local some_text = 'some text'
+		return test:execsql(string.format([[
+			SELECT LENGTH('%s'), CHAR_LENGTH('%s'), CHARACTER_LENGTH('%s');
+		]], some_text, some_text, some_text))
+	end, {
+		-- <func3-6.1>
+		9, 9, 9
+		-- </func3-6.1>
+	})
+
+test:do_test(
+	"func3-6.2",
+	function()
+		local some_text = 'какой-то текст'
+		return test:execsql(string.format([[
+			SELECT LENGTH('%s'), CHAR_LENGTH('%s'), CHARACTER_LENGTH('%s');
+		]], some_text, some_text, some_text))
+	end, {
+		-- <func3-6.2>
+		14, 14, 14
+		-- </func3-6.2>
+	})
+
+test:do_catchsql_test(
+	"func3-6.3",
+	[[
+		SELECT CHAR_LENGTH(12);
+	]], {
+		-- <func3-6.3>
+		1,"Inconsistent types: expected TEXT got INTEGER"
+		-- </func3-6.3>
+	})
+
+test:do_catchsql_test(
+	"func3-6.4",
+	[[
+		SELECT CHAR_LENGTH(12.34);
+	]], {
+		-- <func3-6.4>
+		1,"Inconsistent types: expected TEXT got REAL"
+		-- </func3-6.4>
+	})
+
+test:do_catchsql_test(
+	"func3-6.5",
+	[[
+		SELECT CHAR_LENGTH(x'12');
+	]], {
+		-- <func3-6.5>
+		1,"Inconsistent types: expected TEXT got BLOB"
+		-- </func3-6.5>
+	})
+
+test:do_catchsql_test(
+	"func3-6.6",
+	[[
+		SELECT CHAR_LENGTH(true);
+	]], {
+		-- <func3-6.6>
+		1,"Inconsistent types: expected TEXT got BOOLEAN"
+		-- </func3-6.6>
+	})
+
+test:do_catchsql_test(
+	"func3-6.7",
+	[[
+		SELECT CHARACTER_LENGTH(12);
+	]], {
+		-- <func3-6.7>
+		1,"Inconsistent types: expected TEXT got INTEGER"
+		-- </func3-6.7>
+	})
 
+test:do_catchsql_test(
+	"func3-6.8",
+	[[
+		SELECT CHARACTER_LENGTH(12.34);
+	]], {
+		-- <func3-6.8>
+		1,"Inconsistent types: expected TEXT got REAL"
+		-- </func3-6.8>
+	})
+
+test:do_catchsql_test(
+	"func3-6.9",
+	[[
+		SELECT CHARACTER_LENGTH(x'12');
+	]], {
+		-- <func3-6.9>
+		1,"Inconsistent types: expected TEXT got BLOB"
+		-- </func3-6.9>
+	})
+
+test:do_catchsql_test(
+	"func3-6.10",
+	[[
+		SELECT CHARACTER_LENGTH(true);
+	]], {
+		-- <func3-6.10>
+		1,"Inconsistent types: expected TEXT got BOOLEAN"
+		-- </func3-6.10>
+	})
+
+test:do_catchsql_test(
+	"func3-6.11",
+	[[
+		SELECT CHAR_LENGTH('abc', 'efg');
+	]], {
+		-- <func3-6.11>
+		1,"wrong number of arguments to function CHAR_LENGTH()"
+		-- </func3-6.11>
+	})
+
+test:do_catchsql_test(
+	"func3-6.12",
+	[[
+		SELECT CHARACTER_LENGTH('abc', 'efg');
+	]], {
+		-- <func3-6.12>
+		1,"wrong number of arguments to function CHARACTER_LENGTH()"
+		-- </func3-6.12>
+	})
+
+test:do_execsql_test(
+	"func3-6.13",
+	[[
+		SELECT CHAR_LENGTH(NULL);
+	]], {
+		-- <func3-6.13>
+		""
+		-- </func3-6.13>
+	})
+
+test:do_execsql_test(
+	"func3-6.14",
+	[[
+		SELECT CHARACTER_LENGTH(NULL);
+	]], {
+		-- <func3-6.14>
+		""
+		-- </func3-6.14>
+	})
+
+test:do_test(
+	"func3-6.15",
+	function()
+		local some_text = '\u{123} \u{1234}'
+		return test:execsql(string.format([[
+			SELECT '%s', LENGTH('%s'), CHAR_LENGTH('%s'), CHARACTER_LENGTH('%s');
+		]], some_text, some_text, some_text, some_text))
+	end, {
+		-- <func3-6.15>
+		"ģ ሴ", 3, 3, 3
+		-- </func3-6.15>
+	})
 
 test:finish_test()

[tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

[n.pettik]

> On 29 May 2019, at 20:57, Mergen Imeev <imeevma@tarantool.org> wrote:
> 
> Hi! Thank you for review! My answers and new version below.
> 
> On Tue, May 21, 2019 at 03:58:55PM +0300, n.pettik wrote:
>> 
>> 
>>> On 8 May 2019, at 16:15, imeevma@tarantool.org wrote:
>>> 
>>> This patch creates SQL functions CHARACTER_LENGTH() and
>>> CHAR_LENGTH(). These functions work the same way. Each of them
>>> takes only one argument of type TEXT and returns its length.
>> 
>> I see your message in issue description:
>> 
>> “
>> About this function, it was decided that it would be better to
>> implement it as "{CHAR_LENGTH | CHARACTER_LENGTH}
>> (character value expression)" (without "[USING CHARACTERS]”)
>> “
>> 
>> Could you please provide explanation of this decision?
> Since this issue is not of primary importance, after some
> discussion it was decided that it would be too troublesome to
> create these functions with the support of “[USING CHARACTERS]”.
> 
>> What then is the difference between length() and char_length()?
>> 
> I think the most noticeable difference is that the CHAR_LENGTH and
> CHARACTER_LENGTH functions are described in ANSI, but LENGTH is
> not.

So the name is the only difference? Why don’t you use lengthFunc then
as an implementation? I guess the main purpose of the ticket not to add
just another one synonym, but follow ANSI syntax of function invocation
including USING clause.

>>> diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
>>> index 6d6411c..a3a548b 100755
>>> --- a/test/sql-tap/func3.test.lua
>>> +++ b/test/sql-tap/func3.test.lua
>>> @@ -1,6 +1,6 @@
>>> #!/usr/bin/env tarantool
>>> test = require("sqltester")
>>> -test:plan(25)
>>> +test:plan(37)
>>> 
>>> --!./tcltestrunner.lua
>>> -- 2010 August 27
>>> @@ -284,6 +284,134 @@ test:do_test(
>>>        return test:execsql "EXPLAIN SELECT likely(min(1.0+'2.0',4*11))"
>>>    end, test:execsql "EXPLAIN SELECT min(1.0+'2.0',4*11)”)
>> 
>> Add test involving invalid utf8 characters.
>> 
> Sorry, I could not understand what it means. Do you mean
> something like this (not exactly invalid characters, but
> still):
> tarantool> box.execute("SELECT CHAR_LENGTH('\u{12345}');")
> ---
> - metadata:
>  - name: !!binary Q0hBUl9MRU5HVEgoJ/CSjYUnKQ==
>    type: integer
>  rows:
>  - [1]
> ...
> 
> I'm not sure if this should be added.
> I added a test with utf8 characters.

See sql-tap/badutf1.test.lua

[tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

[Mergen Imeev]

Hi! Thank you for review! I divided path into two. Both
patches and my answers below.

On Thu, Jun 06, 2019 at 09:55:31PM +0300, n.pettik wrote:
> 
> 
> > On 29 May 2019, at 20:57, Mergen Imeev <imeevma@tarantool.org> wrote:
> > 
> > Hi! Thank you for review! My answers and new version below.
> > 
> > On Tue, May 21, 2019 at 03:58:55PM +0300, n.pettik wrote:
> >> 
> >> 
> >>> On 8 May 2019, at 16:15, imeevma@tarantool.org wrote:
> >>> 
> >>> This patch creates SQL functions CHARACTER_LENGTH() and
> >>> CHAR_LENGTH(). These functions work the same way. Each of them
> >>> takes only one argument of type TEXT and returns its length.
> >> 
> >> I see your message in issue description:
> >> 
> >> “
> >> About this function, it was decided that it would be better to
> >> implement it as "{CHAR_LENGTH | CHARACTER_LENGTH}
> >> (character value expression)" (without "[USING CHARACTERS]”)
> >> “
> >> 
> >> Could you please provide explanation of this decision?
It seems that now is not the time for this.

> > Since this issue is not of primary importance, after some
> > discussion it was decided that it would be too troublesome to
> > create these functions with the support of “[USING CHARACTERS]”.
> > 
> >> What then is the difference between length() and char_length()?
> >> 
> > I think the most noticeable difference is that the CHAR_LENGTH and
> > CHARACTER_LENGTH functions are described in ANSI, but LENGTH is
> > not.
> 
> So the name is the only difference? Why don’t you use lengthFunc then
> as an implementation? I guess the main purpose of the ticket not to add
> just another one synonym, but follow ANSI syntax of function invocation
> including USING clause.
> 
Fixed by changes in lengthFunc.

> >>> diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
> >>> index 6d6411c..a3a548b 100755
> >>> --- a/test/sql-tap/func3.test.lua
> >>> +++ b/test/sql-tap/func3.test.lua
> >>> @@ -1,6 +1,6 @@
> >>> #!/usr/bin/env tarantool
> >>> test = require("sqltester")
> >>> -test:plan(25)
> >>> +test:plan(37)
> >>> 
> >>> --!./tcltestrunner.lua
> >>> -- 2010 August 27
> >>> @@ -284,6 +284,134 @@ test:do_test(
> >>>        return test:execsql "EXPLAIN SELECT likely(min(1.0+'2.0',4*11))"
> >>>    end, test:execsql "EXPLAIN SELECT min(1.0+'2.0',4*11)”)
> >> 
> >> Add test involving invalid utf8 characters.
> >> 
> > Sorry, I could not understand what it means. Do you mean
> > something like this (not exactly invalid characters, but
> > still):
> > tarantool> box.execute("SELECT CHAR_LENGTH('\u{12345}');")
> > ---
> > - metadata:
> >  - name: !!binary Q0hBUl9MRU5HVEgoJ/CSjYUnKQ==
> >    type: integer
> >  rows:
> >  - [1]
> > ...
> > 
> > I'm not sure if this should be added.
> > I added a test with utf8 characters.
> 
> See sql-tap/badutf1.test.lua
> 
Added.


First patch:

From 3702718fcf9da52a27bebd5db67a3428b98a093d Mon Sep 17 00:00:00 2001
Date: Fri, 14 Jun 2019 18:55:51 +0300
Subject: [PATCH] sql: make LENGTH() to accept only strings

Prior to this patch, LENGTH() could accept arguments other than
strings. Since this function is considered to be the CHAR_LENGTH()
function from the ANSI standard, this is incorrect. This patch
corrects the behavior of the LENGTH() function.

Part of #3929
Part of #4159

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index ec4f76d..da820f5 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -138,31 +138,23 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
 static void
 lengthFunc(sql_context * context, int argc, sql_value ** argv)
 {
-	int len;
-
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
-	switch (sql_value_type(argv[0])) {
-	case MP_BIN:
-	case MP_INT:
-	case MP_DOUBLE:{
-			sql_result_int(context,
-					   sql_value_bytes(argv[0]));
-			break;
-		}
-	case MP_STR:{
-			const unsigned char *z = sql_value_text(argv[0]);
-			if (z == 0)
-				return;
-			len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
-			sql_result_int(context, len);
-			break;
-		}
-	default:{
-			sql_result_null(context);
-			break;
-		}
+	if (sql_value_type(argv[0]) == MP_NIL) {
+		sql_result_null(context);
+		return;
 	}
+	if (sql_value_type(argv[0]) != MP_STR) {
+		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT",
+			 mem_type_to_str(argv[0]));
+		context->is_aborted = true;
+		return;
+	}
+	const unsigned char *z = sql_value_text(argv[0]);
+	if (z == NULL)
+		return;
+	int len = sql_utf8_char_count(z, sql_value_bytes(argv[0]));
+	sql_result_int(context, len);
 }
 
 /*
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index f9044ad..328e9b0 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(14590)
+test:plan(14589)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -95,10 +95,10 @@ test:do_execsql_test(
 test:do_execsql_test(
     "func-1.4",
     [[
-        SELECT coalesce(length(a),-1) FROM t2
+        SELECT coalesce(a,-1) FROM t2
     ]], {
         -- <func-1.4>
-        1, -1, 3, -1, 5
+        1, -1, 345, -1, 67890
         -- </func-1.4>
     })
 
@@ -982,17 +982,6 @@ test:do_execsql_test(
         -- </func-9.4>
     })
 
-test:do_execsql_test(
-    "func-9.5",
-    [[
-        SELECT length(randomblob(32)), length(randomblob(-5)),
-               length(randomblob(2000))
-    ]], {
-        -- <func-9.5>
-        32, "", 2000
-        -- </func-9.5>
-    })
-
 -- The "hex()" function was added in order to be able to render blobs
 -- generated by randomblob().  So this seems like a good place to test
 -- hex().
diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
index 6d6411c..3832cac 100755
--- a/test/sql-tap/func3.test.lua
+++ b/test/sql-tap/func3.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(25)
+test:plan(30)
 
 --!./tcltestrunner.lua
 -- 2010 August 27
@@ -284,6 +284,58 @@ test:do_test(
         return test:execsql "EXPLAIN SELECT likely(min(1.0+'2.0',4*11))"
     end, test:execsql "EXPLAIN SELECT min(1.0+'2.0',4*11)")
 
+ --
+-- gh-4159: sql: length takes not-string values.
+--
+test:do_catchsql_test(
+    "func3-6.1",
+    [[
+        SELECT LENGTH(12);
+    ]], {
+        -- <func3-6.1>
+        1,"Inconsistent types: expected TEXT got INTEGER"
+        -- </func3-6.1>
+    })
+
+test:do_catchsql_test(
+    "func3-6.2",
+    [[
+        SELECT LENGTH(12.34);
+    ]], {
+        -- <func3-6.2>
+        1,"Inconsistent types: expected TEXT got REAL"
+        -- </func3-6.2>
+    })
+
+test:do_catchsql_test(
+    "func3-6.3",
+    [[
+        SELECT LENGTH(x'12');
+    ]], {
+        -- <func3-6.3>
+        1,"Inconsistent types: expected TEXT got BLOB"
+        -- </func3-6.3>
+    })
+
+test:do_catchsql_test(
+    "func3-6.4",
+    [[
+        SELECT LENGTH(true);
+    ]], {
+        -- <func3-6.4>
+        1,"Inconsistent types: expected TEXT got BOOLEAN"
+        -- </func3-6.4>
+    })
+
+test:do_execsql_test(
+    "func3-6.5",
+    [[
+        SELECT LENGTH('123456789');
+    ]], {
+        -- <func3-6.5>
+        9
+        -- </func3-6.5>
+    })
 
 
 test:finish_test()
diff --git a/test/sql-tap/orderby1.test.lua b/test/sql-tap/orderby1.test.lua
index 51e8d30..2cbb8ef 100755
--- a/test/sql-tap/orderby1.test.lua
+++ b/test/sql-tap/orderby1.test.lua
@@ -735,7 +735,7 @@ test:do_execsql_test(
         SELECT (
           SELECT 'hardware' FROM ( 
             SELECT 'software' ORDER BY 'firmware' ASC, 'sportswear' DESC
-          ) GROUP BY 1 HAVING length(b) <> 0
+          ) GROUP BY 1 HAVING b <> 0
         )
         FROM abc;
     ]], {



Second patch:

From 2db336d071f5ab2c1380961df6c0234d16af77d3 Mon Sep 17 00:00:00 2001
Date: Sat, 15 Jun 2019 13:51:58 +0300
Subject: [PATCH] sql: ANSI aliases for LENGTH().

This patch creates CHARACTER_LENGTH() and CHAR_LENGTH() aliases
for LENGTH().

Closes #3929

@TarantoolBot document
Title: SQL functions LENGTH(), CHAR_LENGTH() and CHARACTER_LENGTH()

The SQL functions LENGTH(), CHAR_LENGTH() and CHARACTER_LENGTH()
take exactly one argument. If the argument is of type TEXT, then
they return its length. If the argument is NULL, they return NULL.
They throw an error if the argument is of a different type or if
more than one argument is given.

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index da820f5..21e86bb 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1891,6 +1891,9 @@ sqlRegisterBuiltinFunctions(void)
 			  FIELD_TYPE_STRING),
 		FUNCTION2(length, 1, 0, 0, lengthFunc, SQL_FUNC_LENGTH,
 			  FIELD_TYPE_INTEGER),
+		FUNCTION(char_length, 1, 0, 0, lengthFunc, FIELD_TYPE_INTEGER),
+		FUNCTION(character_length, 1, 0, 0, lengthFunc,
+			 FIELD_TYPE_INTEGER),
 		FUNCTION(position, 2, 0, 1, position_func, FIELD_TYPE_INTEGER),
 		FUNCTION(printf, -1, 0, 0, printfFunc, FIELD_TYPE_STRING),
 		FUNCTION(unicode, 1, 0, 0, unicodeFunc, FIELD_TYPE_STRING),
diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
index 3832cac..f5f5bba 100755
--- a/test/sql-tap/func3.test.lua
+++ b/test/sql-tap/func3.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(30)
+test:plan(58)
 
 --!./tcltestrunner.lua
 -- 2010 August 27
@@ -337,5 +337,269 @@ test:do_execsql_test(
         -- </func3-6.5>
     })
 
+ --
+-- gh-3929: sql: ANSI aliases for LENGTH().
+--
+test:do_catchsql_test(
+    "func3-7.1",
+    [[
+        SELECT CHAR_LENGTH(12);
+    ]], {
+        -- <func3-7.1>
+        1,"Inconsistent types: expected TEXT got INTEGER"
+        -- </func3-7.1>
+    })
+
+test:do_catchsql_test(
+    "func3-7.2",
+    [[
+        SELECT CHAR_LENGTH(12.34);
+    ]], {
+        -- <func3-7.2>
+        1,"Inconsistent types: expected TEXT got REAL"
+        -- </func3-7.2>
+    })
+
+test:do_catchsql_test(
+    "func3-7.3",
+    [[
+        SELECT CHAR_LENGTH(x'12');
+    ]], {
+        -- <func3-7.3>
+        1,"Inconsistent types: expected TEXT got BLOB"
+        -- </func3-7.3>
+    })
+
+test:do_catchsql_test(
+    "func3-7.4",
+    [[
+        SELECT CHAR_LENGTH(true);
+    ]], {
+        -- <func3-7.4>
+        1,"Inconsistent types: expected TEXT got BOOLEAN"
+        -- </func3-7.4>
+    })
+
+test:do_execsql_test(
+    "func3-7.5",
+    [[
+        SELECT CHAR_LENGTH('123456789');
+    ]], {
+        -- <func3-7.5>
+        9
+        -- </func3-7.5>
+    })
+
+test:do_execsql_test(
+    "func3-7.6",
+    "SELECT CHAR_LENGTH('\x80');",
+    {
+        -- <func3-7.6>
+        1
+        -- </func3-7.6>
+    })
+
+test:do_execsql_test(
+    "func3-7.7",
+    "SELECT CHAR_LENGTH('\x61\x62\x63');",
+    {
+        -- <func3-7.7>
+        3
+        -- </func3-7.7>
+    })
+
+test:do_execsql_test(
+    "func3-7.8",
+    "SELECT CHAR_LENGTH('\x7f\x80\x81');",
+    {
+        -- <func3-7.8>
+        3
+        -- </func3-7.8>
+    })
+
+test:do_execsql_test(
+    "func3-7.9",
+    "SELECT CHAR_LENGTH('\x61\xc0');",
+    {
+        -- <func3-7.9>
+        2
+        -- </func3-7.9>
+    })
+
+test:do_execsql_test(
+    "func3-7.10",
+    "SELECT CHAR_LENGTH('\x61\xc0\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80');",
+    {
+        -- <func3-7.10>
+        12
+        -- </func3-7.10>
+    })
+
+test:do_execsql_test(
+    "func3-7.11",
+    "SELECT CHAR_LENGTH('\xc0\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80');",
+    {
+        -- <func3-7.11>
+        11
+        -- </func3-7.11>
+    })
+
+test:do_execsql_test(
+    "func3-7.12",
+    "SELECT CHAR_LENGTH('\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80');",
+    {
+        -- <func3-7.12>
+        10
+        -- </func3-7.12>
+    })
+
+test:do_execsql_test(
+    "func3-7.13",
+    "SELECT CHAR_LENGTH('\x80\x80\x80\x80\x80\xf0\x80\x80\x80\x80');",
+    {
+        -- <func3-7.13>
+        7
+        -- </func3-7.13>
+    })
+
+test:do_execsql_test(
+    "func3-7.14",
+    "SELECT CHAR_LENGTH('\x80\x80\x80\x80\x80\xf0\x80\x80\x80\xff');",
+    {
+        -- <func3-7.14>
+        7
+        -- </func3-7.14>
+    })
+
+test:do_catchsql_test(
+    "func3-7.15",
+    [[
+        SELECT CHARACTER_LENGTH(12);
+    ]], {
+        -- <func3-7.15>
+        1,"Inconsistent types: expected TEXT got INTEGER"
+        -- </func3-7.15>
+    })
+
+test:do_catchsql_test(
+    "func3-7.16",
+    [[
+        SELECT CHARACTER_LENGTH(12.34);
+    ]], {
+        -- <func3-7.16>
+        1,"Inconsistent types: expected TEXT got REAL"
+        -- </func3-7.16>
+    })
+
+test:do_catchsql_test(
+    "func3-7.17",
+    [[
+        SELECT CHARACTER_LENGTH(x'12');
+    ]], {
+        -- <func3-7.17>
+        1,"Inconsistent types: expected TEXT got BLOB"
+        -- </func3-7.17>
+    })
+
+test:do_catchsql_test(
+    "func3-7.18",
+    [[
+        SELECT CHARACTER_LENGTH(true);
+    ]], {
+        -- <func3-7.18>
+        1,"Inconsistent types: expected TEXT got BOOLEAN"
+        -- </func3-7.18>
+    })
+
+test:do_execsql_test(
+    "func3-7.19",
+    [[
+        SELECT CHARACTER_LENGTH('123456789');
+    ]], {
+        -- <func3-7.19>
+        9
+        -- </func3-7.19>
+    })
+
+test:do_execsql_test(
+    "func3-7.20",
+    "SELECT CHARACTER_LENGTH('\x80');",
+    {
+        -- <func3-7.20>
+        1
+        -- </func3-7.20>
+    })
+
+test:do_execsql_test(
+    "func3-7.21",
+    "SELECT CHARACTER_LENGTH('\x61\x62\x63');",
+    {
+        -- <func3-7.21>
+        3
+        -- </func3-7.21>
+    })
+
+test:do_execsql_test(
+    "func3-7.22",
+    "SELECT CHARACTER_LENGTH('\x7f\x80\x81');",
+    {
+        -- <func3-7.22>
+        3
+        -- </func3-7.22>
+    })
+
+test:do_execsql_test(
+    "func3-7.23",
+    "SELECT CHARACTER_LENGTH('\x61\xc0');",
+    {
+        -- <func3-7.23>
+        2
+        -- </func3-7.23>
+    })
+
+test:do_execsql_test(
+    "func3-7.24",
+    "SELECT CHARACTER_LENGTH('\x61\xc0\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80');",
+    {
+        -- <func3-7.24>
+        12
+        -- </func3-7.24>
+    })
+
+test:do_execsql_test(
+    "func3-7.25",
+    "SELECT CHARACTER_LENGTH('\xc0\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80');",
+    {
+        -- <func3-7.25>
+        11
+        -- </func3-7.25>
+    })
+
+test:do_execsql_test(
+    "func3-7.26",
+    "SELECT CHARACTER_LENGTH('\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80');",
+    {
+        -- <func3-7.26>
+        10
+        -- </func3-7.26>
+    })
+
+test:do_execsql_test(
+    "func3-7.27",
+    "SELECT CHARACTER_LENGTH('\x80\x80\x80\x80\x80\xf0\x80\x80\x80\x80');",
+    {
+        -- <func3-7.27>
+        7
+        -- </func3-7.27>
+    })
+
+test:do_execsql_test(
+    "func3-7.28",
+    "SELECT CHARACTER_LENGTH('\x80\x80\x80\x80\x80\xf0\x80\x80\x80\xff');",
+    {
+        -- <func3-7.28>
+        7
+        -- </func3-7.28>
+    })
 
 test:finish_test()

[tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH()

[n.pettik]

[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]


> First patch:
> 
> From 3702718fcf9da52a27bebd5db67a3428b98a093d Mon Sep 17 00:00:00 2001
> Date: Fri, 14 Jun 2019 18:55:51 +0300
> Subject: [PATCH] sql: make LENGTH() to accept only strings
> 
> Prior to this patch, LENGTH() could accept arguments other than
> strings. Since this function is considered to be the CHAR_LENGTH()
> function from the ANSI standard, this is incorrect. This patch
> corrects the behavior of the LENGTH() function.
> 
> Part of #3929
> Part of #4159

This is OK.

> Second patch:
> 
> From 2db336d071f5ab2c1380961df6c0234d16af77d3 Mon Sep 17 00:00:00 2001
> Date: Sat, 15 Jun 2019 13:51:58 +0300
> Subject: [PATCH] sql: ANSI aliases for LENGTH().
> 
> This patch creates CHARACTER_LENGTH() and CHAR_LENGTH() aliases
> for LENGTH().
> 
> Closes #3929
> 
> @TarantoolBot document
> Title: SQL functions LENGTH(), CHAR_LENGTH() and CHARACTER_LENGTH()
> 
> The SQL functions LENGTH(), CHAR_LENGTH() and CHARACTER_LENGTH()
> take exactly one argument. If the argument is of type TEXT, then
> they return its length. If the argument is NULL, they return NULL.
> They throw an error if the argument is of a different type or if
> more than one argument is given.
> 
> diff --git a/test/sql-tap/func3.test.lua b/test/sql-tap/func3.test.lua
> index 3832cac..f5f5bba 100755
> --- a/test/sql-tap/func3.test.lua
> +++ b/test/sql-tap/func3.test.lua
> @@ -1,6 +1,6 @@
> #!/usr/bin/env tarantool
> test = require("sqltester")
> -test:plan(30)
> +test:plan(58)
> 
> --!./tcltestrunner.lua
> -- 2010 August 27
> @@ -337,5 +337,269 @@ test:do_execsql_test(
>         -- </func3-6.5>
>     })
> 
> + --
> +-- gh-3929: sql: ANSI aliases for LENGTH().
> +—

I’d (IMHO) say you added too many tests. Especially taking into
consideration the fact that under the hood these functions feature
the same implementation.


[-- Attachment #2: Type: text/html, Size: 35167 bytes --]