Skip to content

Commit

Permalink
sql: do not reset region on select
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ImeevMA authored and Korablev77 committed Dec 24, 2020
1 parent 2a3a0d1 commit d0d668f
Show file tree
Hide file tree
Showing 6 changed files with 258 additions and 17 deletions.
5 changes: 3 additions & 2 deletions src/box/box.cc
Expand Up @@ -1413,7 +1413,8 @@ box_select(uint32_t space_id, uint32_t index_id,
});

struct txn *txn;
if (txn_begin_ro_stmt(space, &txn) != 0)
struct txn_ro_savepoint svp;
if (txn_begin_ro_stmt(space, &txn, &svp) != 0)
return -1;

struct iterator *it = index_create_iterator(index, type,
Expand Down Expand Up @@ -1447,7 +1448,7 @@ box_select(uint32_t space_id, uint32_t index_id,
txn_rollback_stmt(txn);
return -1;
}
txn_commit_ro_stmt(txn);
txn_commit_ro_stmt(txn, &svp);
return 0;
}

Expand Down
25 changes: 15 additions & 10 deletions src/box/index.cc
Expand Up @@ -239,13 +239,14 @@ box_index_get(uint32_t space_id, uint32_t index_id, const char *key,
return -1;
/* Start transaction in the engine. */
struct txn *txn;
if (txn_begin_ro_stmt(space, &txn) != 0)
struct txn_ro_savepoint svp;
if (txn_begin_ro_stmt(space, &txn, &svp) != 0)
return -1;
if (index_get(index, key, part_count, result) != 0) {
txn_rollback_stmt(txn);
return -1;
}
txn_commit_ro_stmt(txn);
txn_commit_ro_stmt(txn, &svp);
/* Count statistics. */
rmean_collect(rmean_box, IPROTO_SELECT, 1);
if (*result != NULL)
Expand Down Expand Up @@ -273,13 +274,14 @@ box_index_min(uint32_t space_id, uint32_t index_id, const char *key,
return -1;
/* Start transaction in the engine. */
struct txn *txn;
if (txn_begin_ro_stmt(space, &txn) != 0)
struct txn_ro_savepoint svp;
if (txn_begin_ro_stmt(space, &txn, &svp) != 0)
return -1;
if (index_min(index, key, part_count, result) != 0) {
txn_rollback_stmt(txn);
return -1;
}
txn_commit_ro_stmt(txn);
txn_commit_ro_stmt(txn, &svp);
if (*result != NULL)
tuple_bless(*result);
return 0;
Expand All @@ -305,13 +307,14 @@ box_index_max(uint32_t space_id, uint32_t index_id, const char *key,
return -1;
/* Start transaction in the engine. */
struct txn *txn;
if (txn_begin_ro_stmt(space, &txn) != 0)
struct txn_ro_savepoint svp;
if (txn_begin_ro_stmt(space, &txn, &svp) != 0)
return -1;
if (index_max(index, key, part_count, result) != 0) {
txn_rollback_stmt(txn);
return -1;
}
txn_commit_ro_stmt(txn);
txn_commit_ro_stmt(txn, &svp);
if (*result != NULL)
tuple_bless(*result);
return 0;
Expand All @@ -338,14 +341,15 @@ box_index_count(uint32_t space_id, uint32_t index_id, int type,
return -1;
/* Start transaction in the engine. */
struct txn *txn;
if (txn_begin_ro_stmt(space, &txn) != 0)
struct txn_ro_savepoint svp;
if (txn_begin_ro_stmt(space, &txn, &svp) != 0)
return -1;
ssize_t count = index_count(index, itype, key, part_count);
if (count < 0) {
txn_rollback_stmt(txn);
return -1;
}
txn_commit_ro_stmt(txn);
txn_commit_ro_stmt(txn, &svp);
return count;
}

Expand Down Expand Up @@ -374,15 +378,16 @@ box_index_iterator(uint32_t space_id, uint32_t index_id, int type,
if (key_validate(index->def, itype, key, part_count))
return NULL;
struct txn *txn;
if (txn_begin_ro_stmt(space, &txn) != 0)
struct txn_ro_savepoint svp;
if (txn_begin_ro_stmt(space, &txn, &svp) != 0)
return NULL;
struct iterator *it = index_create_iterator(index, itype,
key, part_count);
if (it == NULL) {
txn_rollback_stmt(txn);
return NULL;
}
txn_commit_ro_stmt(txn);
txn_commit_ro_stmt(txn, &svp);
rmean_collect(rmean_box, IPROTO_SELECT, 1);
return it;
}
Expand Down
5 changes: 3 additions & 2 deletions src/box/sql.c
Expand Up @@ -901,7 +901,8 @@ cursor_seek(BtCursor *pCur, int *pRes)

struct space *space = pCur->space;
struct txn *txn = NULL;
if (space->def->id != 0 && txn_begin_ro_stmt(space, &txn) != 0)
struct txn_ro_savepoint svp;
if (space->def->id != 0 && txn_begin_ro_stmt(space, &txn, &svp) != 0)
return -1;
struct iterator *it =
index_create_iterator(pCur->index, pCur->iter_type, key,
Expand All @@ -913,7 +914,7 @@ cursor_seek(BtCursor *pCur, int *pRes)
return -1;
}
if (txn != NULL)
txn_commit_ro_stmt(txn);
txn_commit_ro_stmt(txn, &svp);
pCur->iter = it;
pCur->eState = CURSOR_VALID;

Expand Down
23 changes: 20 additions & 3 deletions src/box/txn.h
Expand Up @@ -258,6 +258,20 @@ struct txn_savepoint {
char name[1];
};

/**
* Read-only transaction savepoint object. After completing a read-only
* transaction, we must clear the region. However, if we just reset the region,
* we may corrupt the data that was placed in the region before the read-only
* transaction began. To avoid this, we should use truncation. This structure
* contains the information required for truncation.
*/
struct txn_ro_savepoint {
/** Region used during this transaction. */
struct region *region;
/** Savepoint for region. */
size_t region_used;
};

extern double too_long_threshold;

/**
Expand Down Expand Up @@ -557,24 +571,27 @@ txn_begin_in_engine(struct engine *engine, struct txn *txn);
* select.
*/
static inline int
txn_begin_ro_stmt(struct space *space, struct txn **txn)
txn_begin_ro_stmt(struct space *space, struct txn **txn,
struct txn_ro_savepoint *svp)
{
*txn = in_txn();
if (*txn != NULL) {
struct engine *engine = space->engine;
return txn_begin_in_engine(engine, *txn);
}
svp->region = &fiber()->gc;
svp->region_used = region_used(svp->region);
return 0;
}

static inline void
txn_commit_ro_stmt(struct txn *txn)
txn_commit_ro_stmt(struct txn *txn, struct txn_ro_savepoint *svp)
{
assert(txn == in_txn());
if (txn) {
/* nothing to do */
} else {
fiber_gc();
region_truncate(svp->region, svp->region_used);
}
}

Expand Down
149 changes: 149 additions & 0 deletions test/sql/gh-5427-lua-func-changes-result.result
@@ -0,0 +1,149 @@
-- 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', {
returns = 'integer',
body = [[
function()
box.space.T:select()
return 1
end]],
exports = {'LUA', 'SQL'},
});
| ---
| ...
box.schema.func.create('CORRUPT_GET', {
returns = 'integer',
body = [[
function()
box.space.T:get('aaaaaaaaaaaa')
return 1
end]],
exports = {'LUA', 'SQL'},
});
| ---
| ...
box.schema.func.create('CORRUPT_COUNT', {
returns = 'integer',
body = [[
function()
box.space.T:count()
return 1
end]],
exports = {'LUA', 'SQL'},
});
| ---
| ...
box.schema.func.create('CORRUPT_MAX', {
returns = 'integer',
body = [[
function()
box.space.T.index[0]:max()
return 1
end]],
exports = {'LUA', 'SQL'},
});
| ---
| ...
box.schema.func.create('CORRUPT_MIN', {
returns = 'integer',
body = [[
function()
box.space.T.index[0]:min()
return 1
end]],
exports = {'LUA', 'SQL'},
});
| ---
| ...
test_run:cmd("setopt delimiter ''");
| ---
| - true
| ...

values = {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}
| ---
| ...
query = [[select %s() from t where t.b = ? and t.b <= ? order by t.b;]]
| ---
| ...
box.execute(string.format(query, 'CORRUPT_SELECT'), values)
| ---
| - metadata:
| - name: COLUMN_1
| type: integer
| rows:
| - [1]
| ...
box.execute(string.format(query, 'CORRUPT_GET'), values)
| ---
| - metadata:
| - name: COLUMN_1
| type: integer
| rows:
| - [1]
| ...
box.execute(string.format(query, 'CORRUPT_COUNT'), values)
| ---
| - metadata:
| - name: COLUMN_1
| type: integer
| rows:
| - [1]
| ...
box.execute(string.format(query, 'CORRUPT_MAX'), values)
| ---
| - metadata:
| - name: COLUMN_1
| type: integer
| rows:
| - [1]
| ...
box.execute(string.format(query, 'CORRUPT_MIN'), values)
| ---
| - metadata:
| - name: COLUMN_1
| type: integer
| rows:
| - [1]
| ...
box.execute([[DROP TABLE t;]])
| ---
| - row_count: 1
| ...

box.func.CORRUPT_SELECT:drop()
| ---
| ...
box.func.CORRUPT_GET:drop()
| ---
| ...
box.func.CORRUPT_COUNT:drop()
| ---
| ...
box.func.CORRUPT_MAX:drop()
| ---
| ...
box.func.CORRUPT_MIN:drop()
| ---
| ...
68 changes: 68 additions & 0 deletions test/sql/gh-5427-lua-func-changes-result.test.lua
@@ -0,0 +1,68 @@
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', {
returns = 'integer',
body = [[
function()
box.space.T:select()
return 1
end]],
exports = {'LUA', 'SQL'},
});
box.schema.func.create('CORRUPT_GET', {
returns = 'integer',
body = [[
function()
box.space.T:get('aaaaaaaaaaaa')
return 1
end]],
exports = {'LUA', 'SQL'},
});
box.schema.func.create('CORRUPT_COUNT', {
returns = 'integer',
body = [[
function()
box.space.T:count()
return 1
end]],
exports = {'LUA', 'SQL'},
});
box.schema.func.create('CORRUPT_MAX', {
returns = 'integer',
body = [[
function()
box.space.T.index[0]:max()
return 1
end]],
exports = {'LUA', 'SQL'},
});
box.schema.func.create('CORRUPT_MIN', {
returns = 'integer',
body = [[
function()
box.space.T.index[0]:min()
return 1
end]],
exports = {'LUA', 'SQL'},
});
test_run:cmd("setopt delimiter ''");

values = {"aaaaaaaaaaaa", "aaaaaaaaaaaa"}
query = [[select %s() from t where t.b = ? and t.b <= ? order by t.b;]]
box.execute(string.format(query, 'CORRUPT_SELECT'), values)
box.execute(string.format(query, 'CORRUPT_GET'), values)
box.execute(string.format(query, 'CORRUPT_COUNT'), values)
box.execute(string.format(query, 'CORRUPT_MAX'), values)
box.execute(string.format(query, 'CORRUPT_MIN'), values)
box.execute([[DROP TABLE t;]])

box.func.CORRUPT_SELECT:drop()
box.func.CORRUPT_GET:drop()
box.func.CORRUPT_COUNT:drop()
box.func.CORRUPT_MAX:drop()
box.func.CORRUPT_MIN:drop()

0 comments on commit d0d668f

Please sign in to comment.