From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 644352F3A3 for ; Wed, 29 May 2019 13:57:56 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Wh3ebJ1Q4k4S for ; Wed, 29 May 2019 13:57:56 -0400 (EDT) Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id EA6B12F3A1 for ; Wed, 29 May 2019 13:57:55 -0400 (EDT) Date: Wed, 29 May 2019 20:57:52 +0300 From: Mergen Imeev Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: create CHAR_LENGTH() and CHARACTER_LENGTH() Message-ID: <20190529175752.GA22262@tarantool.org> References: <7b8049e47b931a2014b92e26436dc264d56231c9.1557321237.git.imeevma@gmail.com> <78A05ED7-1BE9-4DF6-A96C-AD763D7CC969@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <78A05ED7-1BE9-4DF6-A96C-AD763D7CC969@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: "n.pettik" Cc: tarantool-patches@freelists.org 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 or simply contained in a is the null value, then the result of the 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, { + -- + 9, 9, 9 + -- + }) + +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, { + -- + 14, 14, 14 + -- + }) + +test:do_catchsql_test( + "func3-6.3", + [[ + SELECT CHAR_LENGTH(12); + ]], { + -- + 1,"Inconsistent types: expected TEXT got INTEGER" + -- + }) + +test:do_catchsql_test( + "func3-6.4", + [[ + SELECT CHAR_LENGTH(12.34); + ]], { + -- + 1,"Inconsistent types: expected TEXT got REAL" + -- + }) + +test:do_catchsql_test( + "func3-6.5", + [[ + SELECT CHAR_LENGTH(x'12'); + ]], { + -- + 1,"Inconsistent types: expected TEXT got BLOB" + -- + }) + +test:do_catchsql_test( + "func3-6.6", + [[ + SELECT CHAR_LENGTH(true); + ]], { + -- + 1,"Inconsistent types: expected TEXT got BOOLEAN" + -- + }) + +test:do_catchsql_test( + "func3-6.7", + [[ + SELECT CHARACTER_LENGTH(12); + ]], { + -- + 1,"Inconsistent types: expected TEXT got INTEGER" + -- + }) +test:do_catchsql_test( + "func3-6.8", + [[ + SELECT CHARACTER_LENGTH(12.34); + ]], { + -- + 1,"Inconsistent types: expected TEXT got REAL" + -- + }) + +test:do_catchsql_test( + "func3-6.9", + [[ + SELECT CHARACTER_LENGTH(x'12'); + ]], { + -- + 1,"Inconsistent types: expected TEXT got BLOB" + -- + }) + +test:do_catchsql_test( + "func3-6.10", + [[ + SELECT CHARACTER_LENGTH(true); + ]], { + -- + 1,"Inconsistent types: expected TEXT got BOOLEAN" + -- + }) + +test:do_catchsql_test( + "func3-6.11", + [[ + SELECT CHAR_LENGTH('abc', 'efg'); + ]], { + -- + 1,"wrong number of arguments to function CHAR_LENGTH()" + -- + }) + +test:do_catchsql_test( + "func3-6.12", + [[ + SELECT CHARACTER_LENGTH('abc', 'efg'); + ]], { + -- + 1,"wrong number of arguments to function CHARACTER_LENGTH()" + -- + }) + +test:do_execsql_test( + "func3-6.13", + [[ + SELECT CHAR_LENGTH(NULL); + ]], { + -- + "" + -- + }) + +test:do_execsql_test( + "func3-6.14", + [[ + SELECT CHARACTER_LENGTH(NULL); + ]], { + -- + "" + -- + }) + +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, { + -- + "ģ ሴ", 3, 3, 3 + -- + }) test:finish_test()