From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A811A469719 for ; Mon, 2 Nov 2020 12:49:15 +0300 (MSK) From: imeevma@tarantool.org Date: Mon, 2 Nov 2020 12:49:14 +0300 Message-Id: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v1 1/1] sql: do not reset region on select List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org, sergos@tarantool.org Cc: tarantool-patches@dev.tarantool.org Prior to this patch, region on fiber was reset during select(), get(), count(), max(), or min(). This would result in an error if one of these operations was used in a user-defined function in SQL. After this patch, these functions truncate region instead of resetting it. Closes #5427 --- https://github.com/tarantool/tarantool/issues/5427 https://github.com/tarantool/tarantool/tree/imeevma/gh-5427-lua-func-changes-result @ChangeLog - Region truncated instead of resetting on select() (gh-5427). src/box/box.cc | 5 +- src/box/index.cc | 20 +++ src/box/txn.h | 6 +- .../gh-5427-lua-func-changes-result.result | 153 ++++++++++++++++++ .../gh-5427-lua-func-changes-result.test.lua | 86 ++++++++++ 5 files changed, 264 insertions(+), 6 deletions(-) create mode 100644 test/sql/gh-5427-lua-func-changes-result.result create mode 100644 test/sql/gh-5427-lua-func-changes-result.test.lua diff --git a/src/box/box.cc b/src/box/box.cc index 18568df3b..473ef52ad 100644 --- a/src/box/box.cc +++ b/src/box/box.cc @@ -1388,7 +1388,8 @@ box_select(uint32_t space_id, uint32_t index_id, struct port *port) { (void)key_end; - + struct region *region = &fiber()->gc; + size_t used = region_used(region); rmean_collect(rmean_box, IPROTO_SELECT, 1); if (iterator < 0 || iterator >= iterator_type_MAX) { @@ -1453,6 +1454,8 @@ box_select(uint32_t space_id, uint32_t index_id, return -1; } txn_commit_ro_stmt(txn); + if (txn == NULL) + region_truncate(region, used); return 0; } diff --git a/src/box/index.cc b/src/box/index.cc index c2fc00867..e54a64fdd 100644 --- a/src/box/index.cc +++ b/src/box/index.cc @@ -228,6 +228,8 @@ box_index_get(uint32_t space_id, uint32_t index_id, const char *key, mp_tuple_assert(key, key_end); struct space *space; struct index *index; + struct region *region = &fiber()->gc; + size_t used = region_used(region); if (check_index(space_id, index_id, &space, &index) != 0) return -1; if (!index->def->opts.is_unique) { @@ -246,6 +248,8 @@ box_index_get(uint32_t space_id, uint32_t index_id, const char *key, return -1; } txn_commit_ro_stmt(txn); + if (txn == NULL) + region_truncate(region, used); /* Count statistics. */ rmean_collect(rmean_box, IPROTO_SELECT, 1); if (*result != NULL) @@ -261,6 +265,8 @@ box_index_min(uint32_t space_id, uint32_t index_id, const char *key, mp_tuple_assert(key, key_end); struct space *space; struct index *index; + struct region *region = &fiber()->gc; + size_t used = region_used(region); if (check_index(space_id, index_id, &space, &index) != 0) return -1; if (index->def->type != TREE) { @@ -280,6 +286,8 @@ box_index_min(uint32_t space_id, uint32_t index_id, const char *key, return -1; } txn_commit_ro_stmt(txn); + if (txn == NULL) + region_truncate(region, used); if (*result != NULL) tuple_bless(*result); return 0; @@ -293,6 +301,8 @@ box_index_max(uint32_t space_id, uint32_t index_id, const char *key, assert(result != NULL); struct space *space; struct index *index; + struct region *region = &fiber()->gc; + size_t used = region_used(region); if (check_index(space_id, index_id, &space, &index) != 0) return -1; if (index->def->type != TREE) { @@ -312,6 +322,8 @@ box_index_max(uint32_t space_id, uint32_t index_id, const char *key, return -1; } txn_commit_ro_stmt(txn); + if (txn == NULL) + region_truncate(region, used); if (*result != NULL) tuple_bless(*result); return 0; @@ -331,6 +343,8 @@ box_index_count(uint32_t space_id, uint32_t index_id, int type, enum iterator_type itype = (enum iterator_type) type; struct space *space; struct index *index; + struct region *region = &fiber()->gc; + size_t used = region_used(region); if (check_index(space_id, index_id, &space, &index) != 0) return -1; uint32_t part_count = mp_decode_array(&key); @@ -346,6 +360,8 @@ box_index_count(uint32_t space_id, uint32_t index_id, int type, return -1; } txn_commit_ro_stmt(txn); + if (txn == NULL) + region_truncate(region, used); return count; } @@ -367,6 +383,8 @@ box_index_iterator(uint32_t space_id, uint32_t index_id, int type, enum iterator_type itype = (enum iterator_type) type; struct space *space; struct index *index; + struct region *region = &fiber()->gc; + size_t used = region_used(region); if (check_index(space_id, index_id, &space, &index) != 0) return NULL; assert(mp_typeof(*key) == MP_ARRAY); /* checked by Lua */ @@ -383,6 +401,8 @@ box_index_iterator(uint32_t space_id, uint32_t index_id, int type, return NULL; } txn_commit_ro_stmt(txn); + if (txn == NULL) + region_truncate(region, used); rmean_collect(rmean_box, IPROTO_SELECT, 1); return it; } diff --git a/src/box/txn.h b/src/box/txn.h index a51bdf8e6..e7c8766b7 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -568,11 +568,7 @@ static inline void txn_commit_ro_stmt(struct txn *txn) { assert(txn == in_txn()); - if (txn) { - /* nothing to do */ - } else { - fiber_gc(); - } + (void)txn; } /** diff --git a/test/sql/gh-5427-lua-func-changes-result.result b/test/sql/gh-5427-lua-func-changes-result.result new file mode 100644 index 000000000..f2e777a0e --- /dev/null +++ b/test/sql/gh-5427-lua-func-changes-result.result @@ -0,0 +1,153 @@ +-- test-run result file version 2 +test_run = require('test_run').new() + | --- + | ... +engine = test_run:get_cfg('engine') + | --- + | ... +_ = box.space._session_settings:update('sql_default_engine', {{'=', 2, engine}}) + | --- + | ... + +box.execute([[CREATE TABLE t (b STRING PRIMARY KEY);]]) + | --- + | - row_count: 1 + | ... +box.execute([[INSERT INTO t VALUES ('aaaaaaaaaaaa');]]) + | --- + | - row_count: 1 + | ... +test_run:cmd("setopt delimiter ';'"); + | --- + | - true + | ... +box.schema.func.create('CORRUPT_SELECT', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T:select() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); + | --- + | ... +box.schema.func.create('CORRUPT_GET', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T:get('aaaaaaaaaaaa') + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); + | --- + | ... +box.schema.func.create('CORRUPT_COUNT', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T:count() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); + | --- + | ... +box.schema.func.create('CORRUPT_MAX', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T.index[0]:max() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); + | --- + | ... +box.schema.func.create('CORRUPT_MIN', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T.index[0]:min() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); + | --- + | ... +test_run:cmd("setopt delimiter ''"); + | --- + | - true + | ... + +box.execute([[select CORRUPT_SELECT(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) + | --- + | - metadata: + | - name: COLUMN_1 + | type: integer + | rows: + | - [1] + | ... +box.execute([[select CORRUPT_GET(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) + | --- + | - metadata: + | - name: COLUMN_1 + | type: integer + | rows: + | - [1] + | ... +box.execute([[select CORRUPT_COUNT(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) + | --- + | - metadata: + | - name: COLUMN_1 + | type: integer + | rows: + | - [1] + | ... +box.execute([[select CORRUPT_MAX(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) + | --- + | - metadata: + | - name: COLUMN_1 + | type: integer + | rows: + | - [1] + | ... +box.execute([[select CORRUPT_MIN(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) + | --- + | - metadata: + | - name: COLUMN_1 + | type: integer + | rows: + | - [1] + | ... + +box.execute([[DROP TABLE t;]]) + | --- + | - row_count: 1 + | ... diff --git a/test/sql/gh-5427-lua-func-changes-result.test.lua b/test/sql/gh-5427-lua-func-changes-result.test.lua new file mode 100644 index 000000000..f2699dbe7 --- /dev/null +++ b/test/sql/gh-5427-lua-func-changes-result.test.lua @@ -0,0 +1,86 @@ +test_run = require('test_run').new() +engine = test_run:get_cfg('engine') +_ = box.space._session_settings:update('sql_default_engine', {{'=', 2, engine}}) + +box.execute([[CREATE TABLE t (b STRING PRIMARY KEY);]]) +box.execute([[INSERT INTO t VALUES ('aaaaaaaaaaaa');]]) +test_run:cmd("setopt delimiter ';'"); +box.schema.func.create('CORRUPT_SELECT', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T:select() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); +box.schema.func.create('CORRUPT_GET', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T:get('aaaaaaaaaaaa') + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); +box.schema.func.create('CORRUPT_COUNT', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T:count() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); +box.schema.func.create('CORRUPT_MAX', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T.index[0]:max() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); +box.schema.func.create('CORRUPT_MIN', { + language = 'LUA', + returns = 'integer', + body = [[ + function(x) + box.space.T.index[0]:min() + return 1 + end]], + is_sandboxed = false, + param_list = { "string" }, + exports = { 'LUA', 'SQL' }, + is_deterministic = false, + if_not_exists = true +}); +test_run:cmd("setopt delimiter ''"); + +box.execute([[select CORRUPT_SELECT(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) +box.execute([[select CORRUPT_GET(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) +box.execute([[select CORRUPT_COUNT(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) +box.execute([[select CORRUPT_MAX(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) +box.execute([[select CORRUPT_MIN(t.b) from t where t.b = ? and t.b <= ? order by t.b;]], {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}) + +box.execute([[DROP TABLE t;]]) -- 2.25.1