[tarantool-patches] [PATCH] Return valid lua error for func creation error

[tarantool-patches] [PATCH] Return valid lua error for func creation error

[Georgy Kirichenko]

Return valid lua error if something fail while sql function creation.
---
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));
+	}
 	return rc;
 }
 
diff --git a/test/sql/func-recreate.result b/test/sql/func-recreate.result
new file mode 100644
index 000000000..f12cc8d90
--- /dev/null
+++ b/test/sql/func-recreate.result
@@ -0,0 +1,36 @@
+test_run = require('test_run').new()
+---
+...
+engine = test_run:get_cfg('engine')
+---
+...
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+---
+...
+-- Check errors during function create process
+fiber = require('fiber')
+---
+...
+box.internal.sql_create_function('WAITFOR', function (n) fiber.sleep(n) return n end)
+---
+...
+ch = fiber.channel(1)
+---
+...
+_ = fiber.create(function () ch:put(box.sql.execute('select WAITFOR(0.2)')) end)
+---
+...
+fiber.sleep(0.1)
+---
+...
+box.internal.sql_create_function('WAITFOR', function (n) require('fiber').sleep(n) return n end)
+---
+- error: database is locked
+...
+ch:get()
+---
+- - [0.2]
+...
+box.internal.sql_create_function('WAITFOR', function (n) require('fiber').sleep(n) return n end)
+---
+...
diff --git a/test/sql/func-recreate.test.lua b/test/sql/func-recreate.test.lua
new file mode 100644
index 000000000..b105da8cb
--- /dev/null
+++ b/test/sql/func-recreate.test.lua
@@ -0,0 +1,17 @@
+test_run = require('test_run').new()
+engine = test_run:get_cfg('engine')
+box.sql.execute('pragma sql_default_engine=\''..engine..'\'')
+
+-- Check errors during function create process
+fiber = require('fiber')
+box.internal.sql_create_function('WAITFOR', function (n) fiber.sleep(n) return n end)
+
+ch = fiber.channel(1)
+
+_ = fiber.create(function () ch:put(box.sql.execute('select WAITFOR(0.2)')) end)
+fiber.sleep(0.1)
+
+box.internal.sql_create_function('WAITFOR', function (n) require('fiber').sleep(n) return n end)
+ch:get()
+box.internal.sql_create_function('WAITFOR', function (n) require('fiber').sleep(n) return n end)
+
-- 
2.19.1

[tarantool-patches] Re: [PATCH] Return valid lua error for func creation error

[n.pettik]

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@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

[tarantool-patches] Re: [PATCH] Return valid lua error for func creation error

[Kirill Yukhin]

Hello,
On 02 Nov 14:30, Georgy Kirichenko wrote:
> Return valid lua error if something fail while sql function creation.
> ---
> https://github.com/tarantool/tarantool/issues/3724
> https://github.com/tarantool/tarantool/tree/g.kirichenko/gh-3724-return-error-from-func-creation
I've checked in upddated patch into 2.1 branch.

--
Regards, Kirill Yukhin