From: imeevma@tarantool.org To: v.shpilevoy@tarantool.org, sergos@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH v1 1/1] sql: do not reset region on select Date: Mon, 2 Nov 2020 12:49:14 +0300 [thread overview] Message-ID: <de3a3542d3f69017d221a681dd5476f0fd240ba2.1604310090.git.imeevma@gmail.com> (raw) 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
next reply other threads:[~2020-11-02 9:49 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-02 9:49 imeevma [this message] 2020-11-04 22:14 ` Vladislav Shpilevoy 2020-12-16 5:34 ` Mergen Imeev 2020-12-17 22:04 ` Vladislav Shpilevoy 2020-12-22 19:30 ` Mergen Imeev 2020-12-23 14:58 ` Vladislav Shpilevoy 2020-12-23 19:20 ` Mergen Imeev 2020-12-23 19:15 imeevma 2020-12-24 10:48 ` Nikita Pettik 2020-12-24 10:58 ` Nikita Pettik
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=de3a3542d3f69017d221a681dd5476f0fd240ba2.1604310090.git.imeevma@gmail.com \ --to=imeevma@tarantool.org \ --cc=sergos@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v1 1/1] sql: do not reset region on select' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox