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 3B0C32474D for ; Sun, 28 Jul 2019 19:59:28 -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 CMEG-73-bZpD for ; Sun, 28 Jul 2019 19:59:28 -0400 (EDT) Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 ED1D924732 for ; Sun, 28 Jul 2019 19:59:27 -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 4/5] sql: make built-ins raise errors for varbin args From: "n.pettik" In-Reply-To: <05d15035-2552-1f05-b7ce-facfbbc3a520@tarantool.org> Date: Mon, 29 Jul 2019 02:59:25 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <61adf9b0d2ce82a927f318c386dd0a004eb3ca45.1563967510.git.korablev@tarantool.org> <05d15035-2552-1f05-b7ce-facfbbc3a520@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 > On 26 Jul 2019, at 01:03, Vladislav Shpilevoy = wrote: > On 24/07/2019 13:42, Nikita Pettik wrote: >> Since values of type 'varbinary' can't be cast to any other type, = let's >> patch built-in functions which are not assumed to accept arguments of >> this type to raise an error in case argument turn out to be of type >> varbinary. >>=20 >> Part of #4206 >> --- >> src/box/sql/func.c | 24 ++++++++++++++++++++++-- >> 1 file changed, 22 insertions(+), 2 deletions(-) >>=20 >> diff --git a/src/box/sql/func.c b/src/box/sql/func.c >> index 4ec591eee..5fd1496fd 100644 >> --- a/src/box/sql/func.c >> +++ b/src/box/sql/func.c >> @@ -506,6 +507,12 @@ roundFunc(sql_context * context, int argc, = sql_value ** argv) >> } >> if (sql_value_is_null(argv[0])) >> return; >> + if (sql_value_type(argv[0]) =3D=3D MP_BIN) { >> + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, >=20 > The patch is ok, but now I see, that we allow 'string' values to = functions, > taking numbers, such as round(). Is it ok? Yep, there was a discussion (not sure whether it was private or in dev mailing list) and it was decided that it is ok. Justification is = following: we allow implicit cast between strings and numeric types. In turn, before passing arguments to functions they should be implicitly casted if it is required. We don=E2=80=99t add explicit OP_ApplyType opcode, = but handle that conversion inside each function separately (due to the sqlite = legacy). I guess it would be better to emit extra OP_ApplyType opcode before each function call, but remove type dispatching from func = implementations.