From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B06D8469710 for ; Thu, 21 May 2020 04:15:33 +0300 (MSK) From: Nikita Pettik Date: Thu, 21 May 2020 04:15:31 +0300 Message-Id: Subject: [Tarantool-patches] [PATCH] vinyl: fix assert in vy_tx_write() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org Assert in vy_tx_write() validates that upsert applied on the top of cached statement is always replace. In fact, it is not always so. If vy_apply_upsert() fails due to the fact that PK is modified, it returns old statement (i.e. statement which upsert is applied on). In this regard, if tuple cache contains insert and invalid upsert is executed, vy_apply_upsert() will return insert. As a result, assert will fire. Let's fix it and take into account that vy_apply_upsert() is able to return inserts as well. Closes #5005 --- Branch: https://github.com/tarantool/tarantool/tree/np/gh-5005-upsert-changing-pk-crash Issue: https://github.com/tarantool/tarantool/issues/5005 @ChangeLog: * Fix crash in debug build due to invalid upsert applied on cached INSERT statement. src/box/vy_tx.c | 7 +++-- test/vinyl/gh-5005-upsert-affecting-pk.result | 30 +++++++++++++++++++ .../gh-5005-upsert-affecting-pk.test.lua | 12 ++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 test/vinyl/gh-5005-upsert-affecting-pk.result create mode 100644 test/vinyl/gh-5005-upsert-affecting-pk.test.lua diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c index d092e0cdb..846c63294 100644 --- a/src/box/vy_tx.c +++ b/src/box/vy_tx.c @@ -506,8 +506,11 @@ vy_tx_write(struct vy_lsm *lsm, struct vy_mem *mem, mem->cmp_def, false); tuple_unref(deleted.stmt); if (applied.stmt != NULL) { - assert(vy_stmt_type(applied.stmt) == - IPROTO_REPLACE); + enum iproto_type applied_type = + vy_stmt_type(applied.stmt); + assert(applied_type == IPROTO_REPLACE || + applied_type == IPROTO_INSERT); + (void) applied_type; int rc = vy_lsm_set(lsm, mem, applied, region_stmt); tuple_unref(applied.stmt); diff --git a/test/vinyl/gh-5005-upsert-affecting-pk.result b/test/vinyl/gh-5005-upsert-affecting-pk.result new file mode 100644 index 000000000..e2fea3501 --- /dev/null +++ b/test/vinyl/gh-5005-upsert-affecting-pk.result @@ -0,0 +1,30 @@ +-- test-run result file version 2 +-- Make sure that applying invalid upsert (which modifies PK) +-- after read (so that key populates tuple cache) does not result +-- in crash. +-- +s = box.schema.create_space('test', {engine = 'vinyl'}) + | --- + | ... +pk = s:create_index('pk') + | --- + | ... +s:insert{1, 1} + | --- + | - [1, 1] + | ... +s:select() + | --- + | - - [1, 1] + | ... +s:upsert({1}, {{'+', 1, 1}}) + | --- + | ... +s:select() + | --- + | - - [1, 1] + | ... + +s:drop() + | --- + | ... diff --git a/test/vinyl/gh-5005-upsert-affecting-pk.test.lua b/test/vinyl/gh-5005-upsert-affecting-pk.test.lua new file mode 100644 index 000000000..23dc46ecd --- /dev/null +++ b/test/vinyl/gh-5005-upsert-affecting-pk.test.lua @@ -0,0 +1,12 @@ +-- Make sure that applying invalid upsert (which modifies PK) +-- after read (so that key populates tuple cache) does not result +-- in crash. +-- +s = box.schema.create_space('test', {engine = 'vinyl'}) +pk = s:create_index('pk') +s:insert{1, 1} +s:select() +s:upsert({1}, {{'+', 1, 1}}) +s:select() + +s:drop() -- 2.17.1