Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko <sergepetrenko@tarantool.org>
To: vdavydov.dev@gmail.com
Cc: tarantool-patches@freelists.org,
	Serge Petrenko <sergepetrenko@tarantool.org>
Subject: [PATCH] lua: fix operation type passing to on_replace triggers
Date: Mon,  3 Jun 2019 17:24:36 +0300	[thread overview]
Message-ID: <20190603142436.60683-1-sergepetrenko@tarantool.org> (raw)

This commit fixes a regression introduced by commit
5ab0763b856d495d1d61055f946f6c0cf87dd065
(pass operation type to lua triggers)

When passing operation type to the trigger, we relied on a corresponding
xrow_header, which would later be written to the log. When before_replace
triggers are fired, there is no row connected to the txn statement yet,
so we faked one to pass operations. We, however, didn't account for
temporary spaces, which never have a row associated with a corresponding
txn stmt. This lead to a segfault on an attemt to use on_replace triggers
with temporary spaces.

Add a fake row for temporary spaces to pass operation type in on_replace
triggers and add some tests.

Closes #4266
---
https://github.com/tarantool/tarantool/issues/4266
https://github.com/tarantool/tarantool/tree/sp/gh-4266-temp-space-insert

 src/box/txn.c                    | 19 ++++++++++++++++++-
 test/box/before_replace.result   | 25 +++++++++++++++++++++++++
 test/box/before_replace.test.lua | 15 +++++++++++++++
 test/box/on_replace.result       | 25 +++++++++++++++++++++++++
 test/box/on_replace.test.lua     | 15 +++++++++++++++
 5 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/src/box/txn.c b/src/box/txn.c
index da749d7cc..88e61baea 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -312,7 +312,24 @@ txn_commit_stmt(struct txn *txn, struct request *request)
 	 */
 	if (stmt->space != NULL && !rlist_empty(&stmt->space->on_replace) &&
 	    stmt->space->run_triggers && (stmt->old_tuple || stmt->new_tuple)) {
-		if (trigger_run(&stmt->space->on_replace, txn) != 0)
+		int rc = 0;
+		if(!space_is_temporary(stmt->space)) {
+			rc = trigger_run(&stmt->space->on_replace, txn);
+		} else {
+			/*
+			 * There is no row attached to txn_stmt for
+			 * temporary spaces, since DML operations on them
+			 * are not written to WAL. Fake a row to pass operation
+			 * type to lua on_replace triggers.
+			 */
+			assert(stmt->row == NULL);
+			struct xrow_header temp_header;
+			temp_header.type = request->type;
+			stmt->row = &temp_header;
+			rc = trigger_run(&stmt->space->on_replace, txn);
+			stmt->row = NULL;
+		}
+		if (rc != 0)
 			goto fail;
 	}
 	--txn->in_sub_stmt;
diff --git a/test/box/before_replace.result b/test/box/before_replace.result
index 9580f05af..5054fdefd 100644
--- a/test/box/before_replace.result
+++ b/test/box/before_replace.result
@@ -833,3 +833,28 @@ save_type
 s:drop()
 ---
 ...
+--
+-- gh-4266 triggers on temporary space fail
+--
+s = box.schema.space.create('test', {temporary = true})
+---
+...
+_ = s:create_index('pk')
+---
+...
+save_type = ''
+---
+...
+_ = s:before_replace(function(old,new, name, type) save_type = type return new end)
+---
+...
+_ = s:insert{1}
+---
+...
+save_type
+---
+- INSERT
+...
+s:drop()
+---
+...
diff --git a/test/box/before_replace.test.lua b/test/box/before_replace.test.lua
index 656e20661..c46ec25a0 100644
--- a/test/box/before_replace.test.lua
+++ b/test/box/before_replace.test.lua
@@ -291,3 +291,18 @@ _ = s:upsert({3,4,5}, {{'+', 2, 1}})
 save_type
 
 s:drop()
+
+--
+-- gh-4266 triggers on temporary space fail
+--
+s = box.schema.space.create('test', {temporary = true})
+
+_ = s:create_index('pk')
+
+save_type = ''
+
+_ = s:before_replace(function(old,new, name, type) save_type = type return new end)
+_ = s:insert{1}
+save_type
+
+s:drop()
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
index 963dac2cb..b2a4b2179 100644
--- a/test/box/on_replace.result
+++ b/test/box/on_replace.result
@@ -752,3 +752,28 @@ save_type
 s:drop()
 ---
 ...
+--
+-- gh-4266 triggers on temporary space fail
+--
+s = box.schema.space.create('test', {temporary = true})
+---
+...
+_ = s:create_index('pk')
+---
+...
+save_type = ''
+---
+...
+_ = s:on_replace(function(old,new, name, type) save_type = type end)
+---
+...
+_ = s:insert{1}
+---
+...
+save_type
+---
+- INSERT
+...
+s:drop()
+---
+...
diff --git a/test/box/on_replace.test.lua b/test/box/on_replace.test.lua
index edb0ad65e..a6150b0ee 100644
--- a/test/box/on_replace.test.lua
+++ b/test/box/on_replace.test.lua
@@ -298,3 +298,18 @@ _ = s:upsert({3,4,5}, {{'+', 2, 1}})
 save_type
 
 s:drop()
+
+--
+-- gh-4266 triggers on temporary space fail
+--
+s = box.schema.space.create('test', {temporary = true})
+
+_ = s:create_index('pk')
+
+save_type = ''
+
+_ = s:on_replace(function(old,new, name, type) save_type = type end)
+_ = s:insert{1}
+save_type
+
+s:drop()
-- 
2.20.1 (Apple Git-117)

             reply	other threads:[~2019-06-03 14:24 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-03 14:24 Serge Petrenko [this message]
2019-06-03 14:33 ` Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190603142436.60683-1-sergepetrenko@tarantool.org \
    --to=sergepetrenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH] lua: fix operation type passing to on_replace triggers' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox