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 1CE322F766 for ; Fri, 2 Nov 2018 09:46:40 -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 RUipGXtr7bN2 for ; Fri, 2 Nov 2018 09:46:40 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 CEB7D2F752 for ; Fri, 2 Nov 2018 09:46:39 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] Re: [PATCH] Return valid lua error for func creation error From: "n.pettik" In-Reply-To: Date: Fri, 2 Nov 2018 16:46:36 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <44B7E08E-B5DA-40FF-8FDF-1D7B196CB2C2@tarantool.org> References: 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: Georgy Kirichenko Thanks a lot for investigating this bug. Patch is OK, only several very minor comments concerning its look. Usually SQL-related patches come with =E2=80=98sql:=E2=80=99 prefix for = commit subject. > On 2 Nov 2018, at 14:30, Georgy Kirichenko = wrote: >=20 > Return valid lua error if something fail while sql function creation. Typo: if something fails during... You forgot to add 'Closes #xxxx=E2=80=99 sign to commit message. > --- > https://github.com/tarantool/tarantool/issues/3724 > = https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-3724-return-er= ror-from-func-creation >=20 > src/box/lua/lua_sql.c | 3 +++ > test/sql/func-recreate.result | 36 +++++++++++++++++++++++++++++++++ > test/sql/func-recreate.test.lua | 17 ++++++++++++++++ > 3 files changed, 56 insertions(+) > create mode 100644 test/sql/func-recreate.result > create mode 100644 test/sql/func-recreate.test.lua >=20 > diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c > index 9d78679fb..100f53b01 100644 > --- a/src/box/lua/lua_sql.c > +++ b/src/box/lua/lua_sql.c > @@ -193,6 +193,9 @@ int lbox_sql_create_function(struct lua_State *L) > NULL, NULL, lua_sql_destroy); >=20 > free(normalized_name); > + if (rc !=3D 0) { > + return luaL_error(L, sqlite3ErrStr(rc)); > + } We don=E2=80=99t put brackets in case if-body contains 1 line: diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c index 100f53b01..bcf2ea0fa 100644 --- a/src/box/lua/lua_sql.c +++ b/src/box/lua/lua_sql.c @@ -193,9 +193,8 @@ int lbox_sql_create_function(struct lua_State *L) NULL, NULL, lua_sql_destroy); =20 free(normalized_name); - if (rc !=3D 0) { + if (rc !=3D 0) return luaL_error(L, sqlite3ErrStr(rc)); - } - return rc; + return 0; } =20 > diff --git a/test/sql/func-recreate.test.lua = b/test/sql/func-recreate.test.lua We also add for regression tests number of issue to its name: test/sql/gh-3724-func-recreate.test.lua