Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladimir Davydov <vdavydov.dev@gmail.com>
To: kostja@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [PATCH 09/10] ddl: don't use txn_last_stmt on _ck_constraint commit/rollback
Date: Wed,  3 Jul 2019 22:30:11 +0300	[thread overview]
Message-ID: <f5001562361495ad7889b33497a215d4f0d27312.1562181197.git.vdavydov.dev@gmail.com> (raw)
In-Reply-To: <cover.1562181197.git.vdavydov.dev@gmail.com>
In-Reply-To: <cover.1562181197.git.vdavydov.dev@gmail.com>

When we implement transactional DDL, txn_last_stmt won't necessarily
point to the right statement on commit or rollback so we must avoid
using it.

While we are at it, let's also make sure that changes are propagated
to Lua on replace, not on commit, by moving on_alter_space trigger
invocation appropriately.
---
 src/box/alter.cc | 138 +++++++++++++++++++++++++++++++------------------------
 1 file changed, 78 insertions(+), 60 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 3ddddabb..dd66e46f 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -4371,76 +4371,86 @@ ck_constraint_def_new_from_tuple(struct tuple *tuple)
 	return ck_def;
 }
 
-/** Trigger invoked on rollback in the _ck_constraint space. */
+/** Rollback INSERT check constraint. */
 static void
-on_replace_ck_constraint_rollback(struct trigger *trigger, void *event)
+on_create_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
 {
-	struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
 	struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
 	assert(ck != NULL);
 	struct space *space = space_by_id(ck->def->space_id);
+	assert(space != NULL);
 	struct trigger *ck_trigger = space->ck_constraint_trigger;
-	if (stmt->old_tuple != NULL && stmt->new_tuple == NULL) {
-		/* Rollback DELETE check constraint. */
-		assert(ck != NULL);
-		assert(space != NULL);
-		assert(space_ck_constraint_by_name(space,
-				ck->def->name, strlen(ck->def->name)) == NULL);
-		rlist_add_entry(&space->ck_constraint, ck, link);
-		if (rlist_empty(&ck_trigger->link))
-			trigger_add(&space->on_replace, ck_trigger);
-	}  else if (stmt->new_tuple != NULL && stmt->old_tuple == NULL) {
-		/* Rollback INSERT check constraint. */
-		assert(space != NULL);
-		assert(space_ck_constraint_by_name(space,
-				ck->def->name, strlen(ck->def->name)) != NULL);
-		rlist_del_entry(ck, link);
-		ck_constraint_delete(ck);
-		if (rlist_empty(&space->ck_constraint)) {
-			trigger_clear(ck_trigger);
-			ck_trigger->destroy(ck_trigger);
-			space->ck_constraint_trigger = NULL;
-		}
-	} else {
-		/* Rollback REPLACE check constraint. */
-		assert(space != NULL);
-		const char *name = ck->def->name;
-		struct ck_constraint *new_ck =
-			space_ck_constraint_by_name(space, name, strlen(name));
-		assert(new_ck != NULL);
-		rlist_del_entry(new_ck, link);
-		rlist_add_entry(&space->ck_constraint, ck, link);
-		ck_constraint_delete(new_ck);
+	assert(ck_trigger != NULL);
+	assert(space_ck_constraint_by_name(space, ck->def->name,
+					   strlen(ck->def->name)) != NULL);
+	rlist_del_entry(ck, link);
+	ck_constraint_delete(ck);
+	if (rlist_empty(&space->ck_constraint)) {
+		trigger_clear(ck_trigger);
+		ck_trigger->destroy(ck_trigger);
+		space->ck_constraint_trigger = NULL;
 	}
+	trigger_run_xc(&on_alter_space, space);
 }
 
-/**
- * Trigger invoked on commit in the _ck_constraint space.
- * Drop useless old check constraint object if exists.
- */
+/** Commit DELETE check constraint. */
 static void
-on_replace_ck_constraint_commit(struct trigger *trigger, void *event)
+on_drop_ck_constraint_commit(struct trigger *trigger, void * /* event */)
 {
-	struct txn_stmt *stmt = txn_last_stmt((struct txn *) event);
 	struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
 	assert(ck != NULL);
 	struct space *space = space_by_id(ck->def->space_id);
 	assert(space != NULL);
-	if (stmt->old_tuple != NULL && stmt->new_tuple == NULL) {
-		/* Commit DELETE check constraint. */
-		struct trigger *ck_trigger = space->ck_constraint_trigger;
-		assert(ck_trigger != NULL);
-		if (rlist_empty(&space->ck_constraint)) {
-			ck_trigger->destroy(ck_trigger);
-			space->ck_constraint_trigger = NULL;
-			ck_constraint_delete(ck);
-		}
-	} else if (stmt->old_tuple != NULL) {
-		/* Commit REPLACE check constraint. */
-		assert(stmt->new_tuple != NULL);
+	struct trigger *ck_trigger = space->ck_constraint_trigger;
+	assert(ck_trigger != NULL);
+	if (rlist_empty(&space->ck_constraint)) {
+		ck_trigger->destroy(ck_trigger);
+		space->ck_constraint_trigger = NULL;
 		ck_constraint_delete(ck);
 	}
-	/* Export schema changes to Lua. */
+}
+
+/** Rollback DELETE check constraint. */
+static void
+on_drop_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
+{
+	struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
+	assert(ck != NULL);
+	struct space *space = space_by_id(ck->def->space_id);
+	assert(space != NULL);
+	struct trigger *ck_trigger = space->ck_constraint_trigger;
+	assert(ck_trigger != NULL);
+	assert(space_ck_constraint_by_name(space, ck->def->name,
+					   strlen(ck->def->name)) == NULL);
+	rlist_add_entry(&space->ck_constraint, ck, link);
+	if (rlist_empty(&ck_trigger->link))
+		trigger_add(&space->on_replace, ck_trigger);
+	trigger_run_xc(&on_alter_space, space);
+}
+
+/** Commit REPLACE check constraint. */
+static void
+on_replace_ck_constraint_commit(struct trigger *trigger, void * /* event */)
+{
+	struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
+	if (ck != NULL)
+		ck_constraint_delete(ck);
+}
+
+/** Rollback REPLACE check constraint. */
+static void
+on_replace_ck_constraint_rollback(struct trigger *trigger, void * /* event */)
+{
+	struct ck_constraint *ck = (struct ck_constraint *)trigger->data;
+	assert(ck != NULL);
+	struct space *space = space_by_id(ck->def->space_id);
+	assert(space != NULL);
+	struct ck_constraint *new_ck = space_ck_constraint_by_name(space,
+					ck->def->name, strlen(ck->def->name));
+	assert(new_ck != NULL);
+	rlist_del_entry(new_ck, link);
+	rlist_add_entry(&space->ck_constraint, ck, link);
+	ck_constraint_delete(new_ck);
 	trigger_run_xc(&on_alter_space, space);
 }
 
@@ -4459,10 +4469,8 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
 	struct space *space = space_cache_find_xc(space_id);
 	struct trigger *ck_trigger = space->ck_constraint_trigger;
 	assert(ck_trigger == NULL || !rlist_empty(&ck_trigger->link));
-	struct trigger *on_rollback =
-		txn_alter_trigger_new(on_replace_ck_constraint_rollback, NULL);
-	struct trigger *on_commit =
-		txn_alter_trigger_new(on_replace_ck_constraint_commit, NULL);
+	struct trigger *on_rollback = txn_alter_trigger_new(NULL, NULL);
+	struct trigger *on_commit = txn_alter_trigger_new(NULL, NULL);
 
 	if (new_tuple != NULL) {
 		bool is_deferred =
@@ -4516,9 +4524,15 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
 			space->ck_constraint_trigger = ck_trigger;
 		}
 		ck_guard.is_active = false;
-		on_commit->data = old_tuple == NULL ? new_ck_constraint :
-						      old_ck_constraint;
-		on_rollback->data = on_commit->data;
+		if (old_tuple != NULL) {
+			on_rollback->data = old_ck_constraint;
+			on_rollback->run = on_replace_ck_constraint_rollback;
+		} else {
+			on_rollback->data = new_ck_constraint;
+			on_rollback->run = on_create_ck_constraint_rollback;
+		}
+		on_commit->data = old_ck_constraint;
+		on_commit->run = on_replace_ck_constraint_commit;
 	} else {
 		assert(ck_trigger != NULL);
 		assert(new_tuple == NULL && old_tuple != NULL);
@@ -4535,11 +4549,15 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
 		if (rlist_empty(&space->ck_constraint))
 			trigger_clear(ck_trigger);
 		on_commit->data = old_ck_constraint;
+		on_commit->run = on_drop_ck_constraint_commit;
 		on_rollback->data = old_ck_constraint;
+		on_rollback->run = on_drop_ck_constraint_rollback;
 	}
 
 	txn_on_rollback(txn, on_rollback);
 	txn_on_commit(txn, on_commit);
+
+	trigger_run_xc(&on_alter_space, space);
 }
 
 struct trigger alter_space_on_replace_space = {
-- 
2.11.0

  parent reply	other threads:[~2019-07-03 19:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-03 19:30 [PATCH 00/10] Prepare box/alter.cc for transactional DDL Vladimir Davydov
2019-07-03 19:30 ` [PATCH 01/10] ddl: unreference view on space drop synchronously Vladimir Davydov
2019-07-03 19:37   ` Konstantin Osipov
2019-07-03 19:30 ` [PATCH 02/10] ddl: synchronize user cache with actual data state Vladimir Davydov
2019-07-03 19:43   ` Konstantin Osipov
2019-07-03 20:00     ` Vladimir Davydov
2019-07-04  7:42       ` [tarantool-patches] " Konstantin Osipov
2019-07-03 19:30 ` [PATCH 03/10] ddl: synchronize func " Vladimir Davydov
2019-07-04  8:12   ` Konstantin Osipov
2019-07-03 19:30 ` [PATCH 04/10] ddl: synchronize sequence " Vladimir Davydov
2019-07-04  8:16   ` Konstantin Osipov
2019-07-03 19:30 ` [PATCH 05/10] ddl: fix _space_sequence rollback Vladimir Davydov
2019-07-03 19:30 ` [PATCH 06/10] ddl: restore sequence value if drop is rolled back Vladimir Davydov
2019-07-03 19:30 ` [PATCH 07/10] ddl: don't use txn_last_stmt on _collation commit/rollback Vladimir Davydov
2019-07-03 19:30 ` [PATCH 08/10] ddl: don't use txn_last_stmt on _trigger commit/rollback Vladimir Davydov
2019-07-03 19:30 ` Vladimir Davydov [this message]
2019-07-03 19:30 ` [PATCH 10/10] ddl: don't use txn_last_stmt on _cluster commit/rollback Vladimir Davydov
2019-07-04 15:01 ` [PATCH 00/10] Prepare box/alter.cc for transactional DDL 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=f5001562361495ad7889b33497a215d4f0d27312.1562181197.git.vdavydov.dev@gmail.com \
    --to=vdavydov.dev@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [PATCH 09/10] ddl: don'\''t use txn_last_stmt on _ck_constraint commit/rollback' \
    /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