From 5a38c5c90072e15e03f9578c0933537c74721334 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@tarantool.org>
Date: Tue, 18 Oct 2022 11:51:33 +0300
Subject: [PATCH] sql: fix another cursor invalidation
This patch fixes the issue described in issue #5310 when the tuple
format has more fields than the space format. This solution is more
general than the solution in 89057a21b.
Follow-up #5310
Closes #4666
NO_DOC=bugfix
---
.../gh-5310-fix-cursor-invalidation.md | 2 +-
src/box/ck_constraint.c | 1 +
src/box/sql.c | 29 ++++++++++---------
src/box/sql.h | 14 ++++++---
src/box/sql/func.c | 1 +
src/box/sql/vdbe.c | 1 +
.../gh_5310_cursor_invalidation_test.lua | 19 +++++++++++-
7 files changed, 47 insertions(+), 20 deletions(-)
diff --git a/changelogs/unreleased/gh-5310-fix-cursor-invalidation.md b/changelogs/unreleased/gh-5310-fix-cursor-invalidation.md
index e4878e2b31..1aa93a77e9 100644
--- a/changelogs/unreleased/gh-5310-fix-cursor-invalidation.md
+++ b/changelogs/unreleased/gh-5310-fix-cursor-invalidation.md
@@ -1,4 +1,4 @@
## bugfix/sql
* Fixed a crash that could occur when tuples longer than specified in
- the format were selected (gh-5310).
+ the space format were selected (gh-5310, gh-4666).
diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
index af26da313a..9648ea2a8a 100644
--- a/src/box/ck_constraint.c
+++ b/src/box/ck_constraint.c
@@ -200,6 +200,7 @@ ck_constraint_on_replace_trigger(struct trigger *trigger, void *event)
"field_ref");
return -1;
}
+ vdbe_field_ref_create(field_ref, space->def->field_count);
vdbe_field_ref_prepare_tuple(field_ref, new_tuple);
struct ck_constraint *ck_constraint;
diff --git a/src/box/sql.c b/src/box/sql.c
index da9ecea80a..7202c25875 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1361,16 +1361,16 @@ sql_template_space_new(Parse *parser, const char *name)
}
/**
- * Initialize a new vdbe_field_ref instance with given tuple
- * data.
+ * Fill vdbe_field_ref instance with given tuple data.
+ *
* @param field_ref The vdbe_field_ref instance to initialize.
* @param tuple The tuple object pointer or NULL when undefined.
* @param data The tuple data (is always defined).
* @param data_sz The size of tuple data (is always defined).
*/
static void
-vdbe_field_ref_create(struct vdbe_field_ref *field_ref, struct tuple *tuple,
- const char *data, uint32_t data_sz)
+vdbe_field_ref_fill(struct vdbe_field_ref *field_ref, struct tuple *tuple,
+ const char *data, uint32_t data_sz)
{
field_ref->tuple = tuple;
field_ref->data = data;
@@ -1379,13 +1379,7 @@ vdbe_field_ref_create(struct vdbe_field_ref *field_ref, struct tuple *tuple,
const char *field0 = data;
field_ref->format = NULL;
uint32_t mp_count = mp_decode_array(&field0);
- if (tuple != NULL) {
- assert(tuple_format(tuple) != NULL);
- uint32_t field_count = tuple_format(tuple)->total_field_count;
- field_ref->field_count = MIN(field_count, mp_count);
- } else {
- field_ref->field_count = mp_count;
- }
+ field_ref->field_count = MIN(field_ref->field_capacity, mp_count);
field_ref->slots[0] = (uint32_t)(field0 - data);
memset(&field_ref->slots[1], 0,
field_ref->field_count * sizeof(field_ref->slots[0]));
@@ -1397,15 +1391,22 @@ void
vdbe_field_ref_prepare_data(struct vdbe_field_ref *field_ref, const char *data,
uint32_t data_sz)
{
- vdbe_field_ref_create(field_ref, NULL, data, data_sz);
+ vdbe_field_ref_fill(field_ref, NULL, data, data_sz);
}
void
vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref,
struct tuple *tuple)
{
- vdbe_field_ref_create(field_ref, tuple, tuple_data(tuple),
- tuple_bsize(tuple));
+ vdbe_field_ref_fill(field_ref, tuple, tuple_data(tuple),
+ tuple_bsize(tuple));
+}
+
+void
+vdbe_field_ref_create(struct vdbe_field_ref *ref, uint32_t capacity)
+{
+ memset(ref, 0, sizeof(*ref) + capacity * sizeof(ref->slots[0]));
+ ref->field_capacity = capacity;
}
ssize_t
diff --git a/src/box/sql.h b/src/box/sql.h
index dc40c32e36..18611c52e2 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -376,6 +376,8 @@ struct vdbe_field_ref {
uint32_t data_sz;
/** Count of fields in tuple. */
uint32_t field_count;
+ /** Number of allocated slots. */
+ uint32_t field_capacity;
/** Format that match data in field data. */
struct tuple_format *format;
/**
@@ -394,8 +396,8 @@ struct vdbe_field_ref {
};
/**
- * Initialize a new vdbe_field_ref instance with given tuple
- * data.
+ * Fill vdbe_field_ref instance with given tuple data.
+ *
* @param field_ref The vdbe_field_ref instance to initialize.
* @param data The tuple data.
* @param data_sz The size of tuple data.
@@ -405,8 +407,8 @@ vdbe_field_ref_prepare_data(struct vdbe_field_ref *field_ref, const char *data,
uint32_t data_sz);
/**
- * Initialize a new vdbe_field_ref instance with given tuple
- * data.
+ * Fill vdbe_field_ref instance with given tuple data.
+ *
* @param field_ref The vdbe_field_ref instance to initialize.
* @param tuple The tuple object pointer.
*/
@@ -414,6 +416,10 @@ void
vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref,
struct tuple *tuple);
+/** Initialize a new vdbe_field_ref instance. */
+void
+vdbe_field_ref_create(struct vdbe_field_ref *ref, uint32_t capacity);
+
#if defined(__cplusplus)
} /* extern "C" { */
#endif
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index a6a0148793..e7079fafb9 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -2443,6 +2443,7 @@ func_sql_expr_call(struct func *func, struct port *args, struct port *ret)
struct vdbe_field_ref *ref;
size_t size = sizeof(ref->slots[0]) * count + sizeof(*ref);
ref = region_aligned_alloc(region, size, alignof(*ref));
+ vdbe_field_ref_create(ref, count);
vdbe_field_ref_prepare_data(ref, data, mp_size);
ref->format = format;
if (sql_bind_ptr(stmt, 1, ref) != 0)
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 29a8be7834..15ed3acce7 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -206,6 +206,7 @@ allocateCursor(
memset(pCx, 0, offsetof(VdbeCursor,uc));
pCx->eCurType = eCurType;
pCx->nField = nField;
+ vdbe_field_ref_create(&pCx->field_ref, nField);
if (eCurType==CURTYPE_TARANTOOL) {
pCx->uc.pCursor = (BtCursor*)&pMem->z[bt_offset];
sqlCursorZero(pCx->uc.pCursor);
diff --git a/test/sql-luatest/gh_5310_cursor_invalidation_test.lua b/test/sql-luatest/gh_5310_cursor_invalidation_test.lua
index 508249ad94..496f05b920 100644
--- a/test/sql-luatest/gh_5310_cursor_invalidation_test.lua
+++ b/test/sql-luatest/gh_5310_cursor_invalidation_test.lua
@@ -11,7 +11,8 @@ g.after_all(function()
g.server:stop()
end)
-g.test_cursor_invalidation = function()
+-- Make sure that tuples with undescribed fields do not cause an error.
+g.test_cursor_invalidation_1 = function()
g.server:exec(function()
local t = require('luatest')
local s = box.schema.space.create('T', {format = {'A'}})
@@ -22,3 +23,19 @@ g.test_cursor_invalidation = function()
s:drop()
end)
end
+
+--
+-- Make sure that tuples with fields described in tuple format but not described
+-- in space format do not cause an error.
+--
+g.test_cursor_invalidation_2 = function()
+ g.server:exec(function()
+ local t = require('luatest')
+ local s = box.schema.space.create('T', {format = {{'A', 'integer'}}})
+ s:create_index('ii', {parts = {{1, 'integer'}, {2, 'integer'},
+ {3, 'integer'}, {4, 'integer'}}})
+ s:insert({1,2,3,4})
+ t.assert_equals(box.execute([[SELECT * FROM t;]]).rows, {{1}})
+ s:drop()
+ end)
+end
--
GitLab