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 EAC692475D for ; Sun, 28 Jul 2019 20:03:32 -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 Skdp9GvEjd77 for ; Sun, 28 Jul 2019 20:03:32 -0400 (EDT) Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (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 A26E32474D for ; Sun, 28 Jul 2019 20:03:32 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\)) Subject: [tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type From: "n.pettik" In-Reply-To: <49a188eb-dafe-44e7-a0fd-e9244b68e721@tarantool.org> Date: Mon, 29 Jul 2019 03:03:29 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <1293710C-85CC-4F5A-A7CF-4677D901DCE9@tarantool.org> References: <49a188eb-dafe-44e7-a0fd-e9244b68e721@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: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy >=20 > 1. Nit: >=20 > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index d0f0cb4f5..ea7c9f25f 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -2098,8 +2098,7 @@ case OP_Ge: { /* same as TK_GE, = jump, in1, in3 */ > } > break; > } > - } else if (((flags1 | flags3) & MEM_Bool) !=3D 0 || > - ((flags1 | flags3) & MEM_Blob) !=3D 0) { > + } else if (((flags1 | flags3) & (MEM_Bool | MEM_Blob)) !=3D 0) { > /* > * If one of values is of type BOOLEAN or VARBINARY, > * then the second one must be of the same type as Ok, applied. > 2. Please, add tests on varbinary values binding. Could you suggest the way to force MP_BIN format from Lua? I see the only test on VARBINARY type = (test/box/varbinary_type.test.lua), but this approach is unlikely to be applicable for bindings. > 3. BLOB keyword is reserved, but also it is used in parse.y:980. > Should not it be deleted from all the rules, and be just > reserved? It=E2=80=99s rule for declaring blob (aka binary string) literals. I can rename it, but TBO it looks OK to me. > 4. Additionally (I bet, you knew I would ask), what are we going > to do with all mentionings of blob in the code? We have word 'blob' > mentioned 368 times in all the SQL sources, including comments, > parts of names. I=E2=80=99m OK with using BLOB as a synonym to entity of type varbinary. Can manually clean-out all these places, but a bit later (after 2.2 = release). > In wherecode.c:644 we have a CREATE TABLE example, using BLOB - > this one definitely should be fixed. What about other places? > We could replace 'blob' -> 'bin','binary', 'vbin', ... . It > should not take much time. This path has been removed on master branch. So I simply rebased current branch and it disappeared. I=E2=80=99ve also extended patch-set setting correct default trim octet for binary string arguments: sql: fix default trim octet for binary strings =20 According to ANSI specification, if TRIM function accepts binary = string and trim octet is not specified, then it is implicitly set to X'00'. Before this patch trim octet was set to ' ' both for string and = binary string arguments. In turn, ' ' is equal to X'20' in hex = representation. Hence, TRIM function cut wrong characters: =20 TRIM(X'004420') -> X=E2=80=980044' =20 This patch sets default trim octet to X'00' for binary string = arguments. =20 Part of #4206 diff --git a/src/box/sql/func.c b/src/box/sql/func.c index e799f380f..9bea3eda0 100644 --- a/src/box/sql/func.c +++ b/src/box/sql/func.c @@ -1397,15 +1397,20 @@ trim_func_one_arg(struct sql_context *context, = int argc, sql_value **argv) { assert(argc =3D=3D 1); (void) argc; - - const unsigned char *input_str; - if ((input_str =3D sql_value_text(argv[0])) =3D=3D NULL) + /* In case of VARBINARY type default trim octet is X'00'. */ + const unsigned char *default_trim; + enum mp_type val_type =3D sql_value_type(argv[0]); + if (val_type =3D=3D MP_NIL) return; - + if (val_type =3D=3D MP_BIN) + default_trim =3D (const unsigned char *) "\0"; + else + default_trim =3D (const unsigned char *) " "; int input_str_sz =3D sql_value_bytes(argv[0]); - uint8_t len_one =3D 1; - trim_procedure(context, TRIM_BOTH, (const unsigned char *) " ", - &len_one, 1, input_str, input_str_sz); + const unsigned char *input_str =3D sql_value_text(argv[0]); + uint8_t trim_char_len[1] =3D { 1 }; + trim_procedure(context, TRIM_BOTH, default_trim, trim_char_len, = 1, + input_str, input_str_sz); } =20 /** diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua index e073937b8..b81520e33 100755 --- a/test/sql-tap/func.test.lua +++ b/test/sql-tap/func.test.lua @@ -2232,6 +2232,24 @@ test:do_catchsql_test( -- }) =20 +test:do_execsql_test( + "func-22.39", + [[ + SELECT HEX(TRIM(X'004420')) + ]], { "4420" }) + +test:do_execsql_test( + "func-22.40", + [[ + SELECT HEX(TRIM(X'00442000')) + ]], { "4420" }) + +test:do_execsql_test( + "func-22.41", + [[ + SELECT HEX(TRIM(X'442000')) + ]], { "4420" }) + -- This is to test the deprecated sql_aggregate_count() API. -- --test:do_test(