[tarantool-patches] Re: [PATCH] Return valid lua error for func creation error
n.pettik
korablev at tarantool.org
Fri Nov 2 16:46:36 MSK 2018
Thanks a lot for investigating this bug.
Patch is OK, only several very minor comments concerning its look.
Usually SQL-related patches come with ‘sql:’ prefix for commit subject.
> On 2 Nov 2018, at 14:30, Georgy Kirichenko <georgy at tarantool.org> wrote:
>
> Return valid lua error if something fail while sql function creation.
Typo: if something fails during...
You forgot to add 'Closes #xxxx’ sign to commit message.
> ---
> https://github.com/tarantool/tarantool/issues/3724
> https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-3724-return-error-from-func-creation
>
> 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
>
> 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);
>
> free(normalized_name);
> + if (rc != 0) {
> + return luaL_error(L, sqlite3ErrStr(rc));
> + }
We don’t 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);
free(normalized_name);
- if (rc != 0) {
+ if (rc != 0)
return luaL_error(L, sqlite3ErrStr(rc));
- }
- return rc;
+ return 0;
}
> 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
More information about the Tarantool-patches
mailing list