* [tarantool-patches] [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() @ 2019-05-08 13:15 imeevma 2019-05-21 12:58 ` [tarantool-patches] " n.pettik 0 siblings, 1 reply; 6+ messages in thread From: imeevma @ 2019-05-08 13:15 UTC (permalink / raw) To: korablev; +Cc: tarantool-patches 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() 2019-05-08 13:15 [tarantool-patches] [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() imeevma @ 2019-05-21 12:58 ` n.pettik 2019-05-29 17:57 ` Mergen Imeev 0 siblings, 1 reply; 6+ messages in thread From: n.pettik @ 2019-05-21 12:58 UTC (permalink / raw) To: tarantool-patches; +Cc: Imeev Mergen > 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() 2019-05-21 12:58 ` [tarantool-patches] " n.pettik @ 2019-05-29 17:57 ` Mergen Imeev 2019-06-06 18:55 ` n.pettik 0 siblings, 1 reply; 6+ messages in thread From: Mergen Imeev @ 2019-05-29 17:57 UTC (permalink / raw) To: n.pettik; +Cc: tarantool-patches 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() ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() 2019-05-29 17:57 ` Mergen Imeev @ 2019-06-06 18:55 ` n.pettik 2019-06-15 14:45 ` Mergen Imeev 0 siblings, 1 reply; 6+ messages in thread From: n.pettik @ 2019-06-06 18:55 UTC (permalink / raw) To: tarantool-patches; +Cc: Imeev Mergen > 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() 2019-06-06 18:55 ` n.pettik @ 2019-06-15 14:45 ` Mergen Imeev 2019-06-25 21:40 ` n.pettik 0 siblings, 1 reply; 6+ messages in thread From: Mergen Imeev @ 2019-06-15 14:45 UTC (permalink / raw) To: n.pettik; +Cc: tarantool-patches 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() ^ permalink raw reply [flat|nested] 6+ messages in thread
* [tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() 2019-06-15 14:45 ` Mergen Imeev @ 2019-06-25 21:40 ` n.pettik 0 siblings, 0 replies; 6+ messages in thread From: n.pettik @ 2019-06-25 21:40 UTC (permalink / raw) To: tarantool-patches; +Cc: Imeev Mergen [-- 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 --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-06-25 21:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-08 13:15 [tarantool-patches] [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() imeevma 2019-05-21 12:58 ` [tarantool-patches] " n.pettik 2019-05-29 17:57 ` Mergen Imeev 2019-06-06 18:55 ` n.pettik 2019-06-15 14:45 ` Mergen Imeev 2019-06-25 21:40 ` n.pettik
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox