Skip to content

Commit dd0ac81

Browse files
committed
vinyl: fix crash on invalid upsert
`vy_apply_result_does_cross_pk()` must be called after the new tuple format is validated, otherwise it may crash in case the new tuple has fields conflicting with the primary key definition. While we are at it, fix the operation cursor (`ups_ops`) not advanced on this kind of error. This resulted in skipped `upsert` statements following an invalid `upsert` statement in a transaction. Closes tarantool#10099 NO_DOC=bug fix
1 parent 32bcea7 commit dd0ac81

File tree

3 files changed

+58
-11
lines changed

3 files changed

+58
-11
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
## bugfix/vinyl
2+
3+
* Fixed a bug when an `upsert` statement crashed in case the created tuple had
4+
fields conflicting with the primary key definition (gh-10099).

src/box/vy_upsert.c

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ vy_apply_upsert_on_terminal_stmt(struct tuple *upsert, struct tuple *stmt,
132132
ups_ops = ups_ops_end;
133133
continue;
134134
}
135+
/*
136+
* Result statement must satisfy space's format. Since upsert's
137+
* tuple correctness is already checked in vy_upsert(), let's
138+
* use its format to provide result verification.
139+
*/
140+
struct tuple_format *format = tuple_format(upsert);
141+
if (tuple_validate_raw(format, exec_res) != 0) {
142+
if (!suppress_error)
143+
diag_log();
144+
ups_ops = ups_ops_end;
145+
continue;
146+
}
135147
/*
136148
* If it turns out that resulting tuple modifies primary
137149
* key, then simply ignore this upsert.
@@ -148,17 +160,6 @@ vy_apply_upsert_on_terminal_stmt(struct tuple *upsert, struct tuple *stmt,
148160
continue;
149161
}
150162
ups_ops = ups_ops_end;
151-
/*
152-
* Result statement must satisfy space's format. Since upsert's
153-
* tuple correctness is already checked in vy_upsert(), let's
154-
* use its format to provide result verification.
155-
*/
156-
struct tuple_format *format = tuple_format(upsert);
157-
if (tuple_validate_raw(format, exec_res) != 0) {
158-
if (! suppress_error)
159-
diag_log();
160-
continue;
161-
}
162163
result_mp = exec_res;
163164
result_mp_end = exec_res + mp_size;
164165
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
local server = require('luatest.server')
2+
local t = require('luatest')
3+
4+
local g = t.group()
5+
6+
g.before_all(function(cg)
7+
cg.server = server:new()
8+
cg.server:start()
9+
end)
10+
11+
g.after_all(function(cg)
12+
cg.server:drop()
13+
end)
14+
15+
g.after_each(function(cg)
16+
cg.server:exec(function()
17+
if box.space.test ~= nil then
18+
box.space.test:drop()
19+
end
20+
end)
21+
end)
22+
23+
g.test_invalid_upsert = function(cg)
24+
cg.server:exec(function()
25+
local s = box.schema.create_space('test', {engine = 'vinyl'})
26+
s:create_index('pk', {parts = {{1, 'integer'}}})
27+
s:insert({1})
28+
s:insert({2})
29+
s:insert({3})
30+
box.snapshot()
31+
box.begin()
32+
s:upsert({1}, {{'#', 1, 1}})
33+
s:upsert({2}, {{'=', 1, 's'}})
34+
s:upsert({3}, {{'=', 1, 1}})
35+
s:upsert({1}, {{'!', 2, 10}})
36+
s:upsert({2}, {{'!', 2, 20}})
37+
s:upsert({3}, {{'!', 2, 30}})
38+
box.commit()
39+
t.assert_equals(s:select({}, {fullscan = true}),
40+
{{1, 10}, {2, 20}, {3, 30}})
41+
end)
42+
end

0 commit comments

Comments
 (0)