From ca21e6d55356689f5d9e8f773fc4e614736fb93d Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Fri, 7 Jun 2024 13:29:37 +0300
Subject: [PATCH] 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 #10099
NO_DOC=bug fix
(cherry picked from commit dd0ac8140d39a6ce5888d06f4eaf9a86b5ebba50)
---
...gh-10099-vy-crash-on-invalid-upsert-fix.md | 4 ++
src/box/vy_upsert.c | 23 +++++-----
.../gh_10099_invalid_upsert_test.lua | 42 +++++++++++++++++++
3 files changed, 58 insertions(+), 11 deletions(-)
create mode 100644 changelogs/unreleased/gh-10099-vy-crash-on-invalid-upsert-fix.md
create mode 100644 test/vinyl-luatest/gh_10099_invalid_upsert_test.lua
diff --git a/changelogs/unreleased/gh-10099-vy-crash-on-invalid-upsert-fix.md b/changelogs/unreleased/gh-10099-vy-crash-on-invalid-upsert-fix.md
new file mode 100644
index 0000000000..3dff68a295
--- /dev/null
+++ b/changelogs/unreleased/gh-10099-vy-crash-on-invalid-upsert-fix.md
@@ -0,0 +1,4 @@
+## bugfix/vinyl
+
+* Fixed a bug when an `upsert` statement crashed in case the created tuple had
+ fields conflicting with the primary key definition (gh-10099).
diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c
index fdae931f67..6041843913 100644
--- a/src/box/vy_upsert.c
+++ b/src/box/vy_upsert.c
@@ -132,6 +132,18 @@ vy_apply_upsert_on_terminal_stmt(struct tuple *upsert, struct tuple *stmt,
ups_ops = ups_ops_end;
continue;
}
+ /*
+ * Result statement must satisfy space's format. Since upsert's
+ * tuple correctness is already checked in vy_upsert(), let's
+ * use its format to provide result verification.
+ */
+ struct tuple_format *format = tuple_format(upsert);
+ if (tuple_validate_raw(format, exec_res) != 0) {
+ if (!suppress_error)
+ diag_log();
+ ups_ops = ups_ops_end;
+ continue;
+ }
/*
* If it turns out that resulting tuple modifies primary
* key, then simply ignore this upsert.
@@ -148,17 +160,6 @@ vy_apply_upsert_on_terminal_stmt(struct tuple *upsert, struct tuple *stmt,
continue;
}
ups_ops = ups_ops_end;
- /*
- * Result statement must satisfy space's format. Since upsert's
- * tuple correctness is already checked in vy_upsert(), let's
- * use its format to provide result verification.
- */
- struct tuple_format *format = tuple_format(upsert);
- if (tuple_validate_raw(format, exec_res) != 0) {
- if (! suppress_error)
- diag_log();
- continue;
- }
result_mp = exec_res;
result_mp_end = exec_res + mp_size;
}
diff --git a/test/vinyl-luatest/gh_10099_invalid_upsert_test.lua b/test/vinyl-luatest/gh_10099_invalid_upsert_test.lua
new file mode 100644
index 0000000000..2349f7bbb7
--- /dev/null
+++ b/test/vinyl-luatest/gh_10099_invalid_upsert_test.lua
@@ -0,0 +1,42 @@
+local server = require('luatest.server')
+local t = require('luatest')
+
+local g = t.group()
+
+g.before_all(function(cg)
+ cg.server = server:new()
+ cg.server:start()
+end)
+
+g.after_all(function(cg)
+ cg.server:drop()
+end)
+
+g.after_each(function(cg)
+ cg.server:exec(function()
+ if box.space.test ~= nil then
+ box.space.test:drop()
+ end
+ end)
+end)
+
+g.test_invalid_upsert = function(cg)
+ cg.server:exec(function()
+ local s = box.schema.create_space('test', {engine = 'vinyl'})
+ s:create_index('pk', {parts = {{1, 'integer'}}})
+ s:insert({1})
+ s:insert({2})
+ s:insert({3})
+ box.snapshot()
+ box.begin()
+ s:upsert({1}, {{'#', 1, 1}})
+ s:upsert({2}, {{'=', 1, 's'}})
+ s:upsert({3}, {{'=', 1, 1}})
+ s:upsert({1}, {{'!', 2, 10}})
+ s:upsert({2}, {{'!', 2, 20}})
+ s:upsert({3}, {{'!', 2, 30}})
+ box.commit()
+ t.assert_equals(s:select({}, {fullscan = true}),
+ {{1, 10}, {2, 20}, {3, 30}})
+ end)
+end
--
GitLab