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