[PATCH 5/5] Allow to execute non-yielding DDL statements in transactions

Vladimir Davydov vdavydov.dev at gmail.com
Tue Jul 9 14:03:10 MSK 2019


On Tue, Jul 09, 2019 at 11:11:50AM +0300, Vladimir Davydov wrote:
> On Tue, Jul 09, 2019 at 01:02:27AM +0300, Konstantin Osipov wrote:
> > * Vladimir Davydov <vdavydov.dev at gmail.com> [19/07/08 22:14]:
> > > What about
> > > 
> > >   DDL does not support long operations, such as building an index or checking a space format, in a transaction
> > 
> > It's not DDL, it's Tarantool :)
> 
> Well, all error codes are about Tarantool. However, this particular one
> is about limitations of our data dictionarly language (DDL).
> 
> > 
> > I think a separate error code is OK, a clear message has more
> > value than sticking to the same error code.
> > 
> > Can not perform %s in a multi-statement transaction. 
> 
> Okay. Sounds good to me.
> 
> > 
> > Is there a workaround? If yes, let's add it to the message.
> 
> No, there's no workaround. The only way to run index build or space
> format check in a transaction is to execute it first, i.e. in fact in
> a single-statement transaction.

Here's the patch with the fixed error message. Please take a look once
time permits. Both remaining patches are available on the branch:

  https://github.com/tarantool/tarantool/commits/dv/gh-4083-transactional-ddl

I can resend them in a separate thread if you wish.
---

>From 19e11c407c95499a757c247903825d8265fec355 Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov.dev at gmail.com>
Date: Fri, 5 Jul 2019 20:13:16 +0300
Subject: [PATCH] Allow to execute non-yielding DDL statements in transactions

The patch is pretty straightforward - all it does is moves checks for
single statement transactions from alter.cc to txn_enable_yield_for_ddl
so that now any DDL request may be executed in a transaction unless it
builds an index or checks the format of a non-empty space (those are the
only two operations that may yield).

There's one thing that must be noted explicitly - it's removal of an
assertion from priv_grant. The assertion ensured that a revoked
privilege was in the cache. The problem is the cache is built from the
contents of the space, see user_reload_privs. On rollback, we first
revert the content of the space to the original state, and only then
start invoking rollback triggers, which call priv_grant. As a result,
we will revert the cache to the original state right after the first
trigger is invoked and the following triggers will have no effect on it.
Thus we have to remove this assertion.

Closes #4083

@TarantoolBot document
Title: Transactional DDL

Now it's possible to group non-yielding DDL statements into
transactions, e.g.

```Lua
box.begin()
box.schema.space.create('my_space')
box.space.my_space:create_index('primary')
box.commit() -- or box.rollback()
```

Most DDL statements don't yield and hence can be run from transactions.
There are just two exceptions: creation of a new index and changing the
format of a non-empty space. Those are long operations that may yield
so as not to block the event loop for too long. Those statements can't
be executed from transactions (to be more exact, such a statement must
go first in any transaction).

Also, just like in case of DML transactions in memtx, it's forbidden to
explicitly yield in a DDL transaction by calling fiber.sleep or any
other yielding function. If this happens, the transaction will be
aborted and an attempt to commit it will fail.

diff --git a/src/box/alter.cc b/src/box/alter.cc
index ce0cf2d9..2054ff6d 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1828,7 +1828,6 @@ static void
 on_replace_dd_space(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _space");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -2125,7 +2124,6 @@ static void
 on_replace_dd_index(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _index");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -2318,7 +2316,6 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
 	struct txn_stmt *stmt = txn_current_stmt(txn);
-	txn_check_singlestatement_xc(txn, "Space _truncate");
 	struct tuple *new_tuple = stmt->new_tuple;
 
 	if (new_tuple == NULL) {
@@ -2548,7 +2545,6 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
 	struct txn_stmt *stmt = txn_current_stmt(txn);
-	txn_check_singlestatement_xc(txn, "Space _user");
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
 
@@ -2699,7 +2695,6 @@ static void
 on_replace_dd_func(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _func");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -2893,7 +2888,6 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
-	txn_check_singlestatement_xc(txn, "Space _collation");
 	if (new_tuple == NULL && old_tuple != NULL) {
 		/* DELETE */
 		struct trigger *on_commit =
@@ -3191,7 +3185,6 @@ static void
 on_replace_dd_priv(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _priv");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -3240,7 +3233,6 @@ static void
 on_replace_dd_schema(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _schema");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -3318,7 +3310,6 @@ on_replace_dd_cluster(struct trigger *trigger, void *event)
 {
 	(void) trigger;
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _cluster");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -3471,7 +3462,6 @@ static void
 on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _sequence");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -3674,7 +3664,6 @@ static void
 on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _space_sequence");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *tuple = stmt->new_tuple ? stmt->new_tuple : stmt->old_tuple;
 
@@ -3819,7 +3808,6 @@ static void
 on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _trigger");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -4202,7 +4190,6 @@ static void
 on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _fk_constraint");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
@@ -4493,7 +4480,6 @@ static void
 on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
 {
 	struct txn *txn = (struct txn *) event;
-	txn_check_singlestatement_xc(txn, "Space _ck_constraint");
 	struct txn_stmt *stmt = txn_current_stmt(txn);
 	struct tuple *old_tuple = stmt->old_tuple;
 	struct tuple *new_tuple = stmt->new_tuple;
diff --git a/src/box/errcode.h b/src/box/errcode.h
index be8dab27..97fc9675 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -106,7 +106,7 @@ struct errcode_record {
 	/* 51 */_(ER_NO_SUCH_FUNCTION,		"Function '%s' does not exist") \
 	/* 52 */_(ER_FUNCTION_EXISTS,		"Function '%s' already exists") \
 	/* 53 */_(ER_BEFORE_REPLACE_RET,	"Invalid return value of space:before_replace trigger: expected tuple or nil, got %s") \
-	/* 54 */_(ER_UNUSED2,			"") \
+	/* 54 */_(ER_MULTISTATEMENT_DDL,	"Can not perform %s in a multi-statement transaction") \
 	/* 55 */_(ER_TRIGGER_EXISTS,		"Trigger '%s' already exists") \
 	/* 56 */_(ER_USER_MAX,			"A limit on the total number of users has been reached: %u") \
 	/* 57 */_(ER_NO_SUCH_ENGINE,		"Space engine '%s' does not exist") \
diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
index 921dbcbf..47369994 100644
--- a/src/box/memtx_space.c
+++ b/src/box/memtx_space.c
@@ -888,7 +888,8 @@ memtx_space_check_format(struct space *space, struct tuple_format *format)
 	if (it == NULL)
 		return -1;
 
-	txn_enable_yield_for_ddl();
+	if (txn_enable_yield_for_ddl("space format check") != 0)
+		return -1;
 
 	struct memtx_engine *memtx = (struct memtx_engine *)space->engine;
 	struct memtx_ddl_state state;
@@ -1018,6 +1019,8 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
 	struct index *pk = index_find(src_space, 0);
 	if (pk == NULL)
 		return -1;
+	if (index_size(pk) == 0)
+		return 0;
 
 	struct errinj *inj = errinj(ERRINJ_BUILD_INDEX, ERRINJ_INT);
 	if (inj != NULL && inj->iparam == (int)new_index->def->iid) {
@@ -1030,7 +1033,8 @@ memtx_space_build_index(struct space *src_space, struct index *new_index,
 	if (it == NULL)
 		return -1;
 
-	txn_enable_yield_for_ddl();
+	if (txn_enable_yield_for_ddl("index build") != 0)
+		return -1;
 
 	struct memtx_engine *memtx = (struct memtx_engine *)src_space->engine;
 	struct memtx_ddl_state state;
diff --git a/src/box/txn.c b/src/box/txn.c
index bab98d7e..9ae11aa7 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -592,17 +592,6 @@ txn_abort(struct txn *txn)
 	txn->is_aborted = true;
 }
 
-int
-txn_check_singlestatement(struct txn *txn, const char *where)
-{
-	if (!txn_is_first_statement(txn)) {
-		diag_set(ClientError, ER_UNSUPPORTED,
-			 where, "multi-statement transactions");
-		return -1;
-	}
-	return 0;
-}
-
 void
 txn_disable_yield(struct txn *txn)
 {
@@ -612,12 +601,24 @@ txn_disable_yield(struct txn *txn)
 	trigger_add(&fiber()->on_yield, &txn->fiber_on_yield);
 }
 
-void
-txn_enable_yield_for_ddl(void)
+int
+txn_enable_yield_for_ddl(const char *what)
 {
 	struct txn *txn = in_txn();
 	assert(txn != NULL && txn->is_yield_disabled);
+	/*
+	 * It's okay to yield while executing the first DDL statement
+	 * in a transaction, because the schema hasn't been updated
+	 * yet and so other transactions can't see uncommitted objects.
+	 * Yielding in subsequent statements is not safe, as there
+	 * may be uncommitted objects in the schema cache.
+	 */
+	if (!txn_is_first_statement(txn)) {
+		diag_set(ClientError, ER_MULTISTATEMENT_DDL, what);
+		return -1;
+	}
 	trigger_clear(&txn->fiber_on_yield);
+	return 0;
 }
 
 void
diff --git a/src/box/txn.h b/src/box/txn.h
index 258a8db7..92366038 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -358,13 +358,6 @@ void
 txn_rollback_stmt(struct txn *txn);
 
 /**
- * Raise an error if this is a multi-statement transaction: DDL
- * can not be part of a multi-statement transaction.
- */
-int
-txn_check_singlestatement(struct txn *txn, const char *where);
-
-/**
  * Installs yield-in-transaction trigger: roll back the effects
  * of the transaction and mark the transaction as aborted.
  *
@@ -384,9 +377,13 @@ txn_disable_yield(struct txn *txn);
  * This function temporarily disables the trigger for this purpose.
  * One must call txn_disable_yield_after_ddl() once the DDL request
  * has been complete.
+ *
+ * Before enabling yields, this function checks if it doesn't
+ * violate transaction isolation. If it does, it returns -1 and
+ * sets diag. The 'what' message is used for error reporting.
  */
-void
-txn_enable_yield_for_ddl(void);
+int
+txn_enable_yield_for_ddl(const char *what);
 
 /** See txn_enable_yield_for_ddl(). */
 void
@@ -525,16 +522,6 @@ box_txn_rollback_to_savepoint(box_txn_savepoint_t *savepoint);
 
 #if defined(__cplusplus)
 } /* extern "C" */
-
-#include "diag.h"
-
-static inline void
-txn_check_singlestatement_xc(struct txn *txn, const char *where)
-{
-	if (txn_check_singlestatement(txn, where) != 0)
-		diag_raise();
-}
-
 #endif /* defined(__cplusplus) */
 
 #endif /* TARANTOOL_BOX_TXN_H_INCLUDED */
diff --git a/src/box/user.cc b/src/box/user.cc
index 48bdf18e..c46ff67d 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -709,7 +709,6 @@ priv_grant(struct user *grantee, struct priv_def *priv)
 	if (object == NULL)
 		return;
 	struct access *access = &object[grantee->auth_token];
-	assert(privset_search(&grantee->privs, priv) || access->granted == 0);
 	access->granted = priv->access;
 	rebuild_effective_grants(grantee);
 }
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 8629aa8e..dadaa230 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -1121,7 +1121,11 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format)
 	bool need_wal_sync;
 	tx_manager_abort_writers_for_ddl(env->xm, space, &need_wal_sync);
 
-	txn_enable_yield_for_ddl();
+	if (!need_wal_sync && vy_lsm_is_empty(pk))
+		return 0; /* space is empty, nothing to do */
+
+	if (txn_enable_yield_for_ddl("space format check") != 0)
+		return -1;
 
 	struct trigger on_replace;
 	struct vy_check_format_ctx ctx;
@@ -4348,7 +4352,11 @@ vinyl_space_build_index(struct space *src_space, struct index *new_index,
 	bool need_wal_sync;
 	tx_manager_abort_writers_for_ddl(env->xm, src_space, &need_wal_sync);
 
-	txn_enable_yield_for_ddl();
+	if (!need_wal_sync && vy_lsm_is_empty(pk))
+		return 0; /* space is empty, nothing to do */
+
+	if (txn_enable_yield_for_ddl("index build") != 0)
+		return -1;
 
 	/*
 	 * Iterate over all tuples stored in the space and insert
diff --git a/test/box/misc.result b/test/box/misc.result
index dab8549b..3c8e6994 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -385,6 +385,7 @@ t;
   51: box.error.NO_SUCH_FUNCTION
   52: box.error.FUNCTION_EXISTS
   53: box.error.BEFORE_REPLACE_RET
+  54: box.error.MULTISTATEMENT_DDL
   55: box.error.TRIGGER_EXISTS
   56: box.error.USER_MAX
   57: box.error.NO_SUCH_ENGINE
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
index b71c9878..6334d9a2 100644
--- a/test/box/on_replace.result
+++ b/test/box/on_replace.result
@@ -465,86 +465,83 @@ s = box.schema.space.create('test_on_repl_ddl')
 _ = s:create_index('pk')
 ---
 ...
+_ = s:create_index('sk', {parts = {2, 'unsigned'}})
+---
+...
 t = s:on_replace(function () box.schema.space.create('some_space') end)
 ---
 ...
 s:replace({1, 2})
 ---
-- error: Space _schema does not support multi-statement transactions
+- [1, 2]
 ...
-t = s:on_replace(function () s:create_index('sec') end, t)
+t = s:on_replace(function () s:format({{'a', 'unsigned'}, {'b', 'unsigned'}}) end, t)
 ---
 ...
 s:replace({2, 3})
 ---
-- error: Space _index does not support multi-statement transactions
+- [2, 3]
 ...
-t = s:on_replace(function () box.schema.user.create('newu') end, t)
+t = s:on_replace(function () box.schema.user.create('newu') box.schema.role.create('newr') end, t)
 ---
 ...
 s:replace({3, 4})
 ---
-- error: Space _user does not support multi-statement transactions
+- [3, 4]
 ...
-t = s:on_replace(function () box.schema.role.create('newr') end, t)
+t = s:on_replace(function () box.schema.user.drop('newu') box.schema.role.drop('newr') end, t)
 ---
 ...
 s:replace({4, 5})
 ---
-- error: Space _user does not support multi-statement transactions
+- [4, 5]
 ...
-t = s:on_replace(function () box.space._user:delete{box.schema.GUEST_ID} end, t)
----
-...
-s:replace({4, 5})
----
-- error: Space _user does not support multi-statement transactions
-...
-t = s:on_replace(function () box.space._user:delete{box.schema.SUPER_ROLE_ID} end, t)
----
-...
-s:replace({4, 5})
----
-- error: Space _user does not support multi-statement transactions
-...
-t = s:on_replace(function () s:drop() end, t)
+t = s:on_replace(function () s.index.sk:drop() end, t)
 ---
 ...
 s:replace({5, 6})
 ---
-- error: Space _index does not support multi-statement transactions
+- [5, 6]
 ...
 t = s:on_replace(function () box.schema.func.create('newf') end, t)
 ---
 ...
 s:replace({6, 7})
 ---
-- error: Space _func does not support multi-statement transactions
+- [6, 7]
 ...
 t = s:on_replace(function () box.schema.user.grant('guest', 'read,write', 'space', 'test_on_repl_ddl') end, t)
 ---
 ...
 s:replace({7, 8})
 ---
-- error: Space _priv does not support multi-statement transactions
+- [7, 8]
 ...
 t = s:on_replace(function () s:rename('newname') end, t)
 ---
 ...
 s:replace({8, 9})
 ---
-- error: Space _space does not support multi-statement transactions
+- [8, 9]
 ...
 t = s:on_replace(function () s.index.pk:rename('newname') end, t)
 ---
 ...
 s:replace({9, 10})
 ---
-- error: Space _index does not support multi-statement transactions
+- [9, 10]
 ...
 s:select()
 ---
-- []
+- - [1, 2]
+  - [2, 3]
+  - [3, 4]
+  - [4, 5]
+  - [5, 6]
+  - [6, 7]
+  - [7, 8]
+  - [8, 9]
+  - [9, 10]
 ...
 s:drop() -- test_on_repl_ddl
 ---
diff --git a/test/box/on_replace.test.lua b/test/box/on_replace.test.lua
index 7fffc1e0..79c828da 100644
--- a/test/box/on_replace.test.lua
+++ b/test/box/on_replace.test.lua
@@ -181,19 +181,16 @@ second:drop()
 
 s = box.schema.space.create('test_on_repl_ddl')
 _ = s:create_index('pk')
+_ = s:create_index('sk', {parts = {2, 'unsigned'}})
 t = s:on_replace(function () box.schema.space.create('some_space') end)
 s:replace({1, 2})
-t = s:on_replace(function () s:create_index('sec') end, t)
+t = s:on_replace(function () s:format({{'a', 'unsigned'}, {'b', 'unsigned'}}) end, t)
 s:replace({2, 3})
-t = s:on_replace(function () box.schema.user.create('newu') end, t)
+t = s:on_replace(function () box.schema.user.create('newu') box.schema.role.create('newr') end, t)
 s:replace({3, 4})
-t = s:on_replace(function () box.schema.role.create('newr') end, t)
+t = s:on_replace(function () box.schema.user.drop('newu') box.schema.role.drop('newr') end, t)
 s:replace({4, 5})
-t = s:on_replace(function () box.space._user:delete{box.schema.GUEST_ID} end, t)
-s:replace({4, 5})
-t = s:on_replace(function () box.space._user:delete{box.schema.SUPER_ROLE_ID} end, t)
-s:replace({4, 5})
-t = s:on_replace(function () s:drop() end, t)
+t = s:on_replace(function () s.index.sk:drop() end, t)
 s:replace({5, 6})
 t = s:on_replace(function () box.schema.func.create('newf') end, t)
 s:replace({6, 7})
diff --git a/test/box/transaction.result b/test/box/transaction.result
index ad2d650c..9a48f2f7 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -84,22 +84,12 @@ while f:status() ~= 'dead' do fiber.sleep(0) end;
 ---
 ...
 -- transactions and system spaces
--- some operation involves more than one ddl spaces, so they should fail
-box.begin() box.schema.space.create('test');
+box.begin() box.schema.space.create('test') box.rollback();
 ---
-- error: Space _space does not support multi-statement transactions
 ...
-box.rollback();
+box.begin() box.schema.user.create('test') box.rollback();
 ---
 ...
-box.begin() box.schema.user.create('test');
----
-- error: Space _priv does not support multi-statement transactions
-...
-box.rollback();
----
-...
--- but this is Ok now
 box.begin() box.schema.func.create('test') box.rollback();
 ---
 ...
@@ -657,21 +647,14 @@ box.space.vinyl:select{};
 -- Two DDL satements in a row
 box.begin()
 box.space.test:truncate()
-box.space.test:truncate();
----
-- error: Space _truncate does not support multi-statement transactions
-...
--- A transaction is left open due to an exception in the above fragment
+box.space.test:truncate()
 box.rollback();
 ---
 ...
 -- Two DDL stateemnts on different engines
 box.begin()
 box.space.memtx:truncate()
-box.space.vinyl:truncate();
----
-- error: Space _truncate does not support multi-statement transactions
-...
+box.space.vinyl:truncate()
 box.rollback();
 ---
 ...
@@ -738,3 +721,60 @@ box.commit() -- error
 s:drop()
 ---
 ...
+--
+-- Multiple DDL statements in the same transaction.
+--
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+function create()
+    box.schema.role.create('my_role')
+    box.schema.user.create('my_user')
+    box.schema.user.grant('my_user', 'my_role')
+    box.schema.space.create('memtx_space', {engine = 'memtx'})
+    box.schema.space.create('vinyl_space', {engine = 'vinyl'})
+    box.schema.role.grant('my_role', 'read,write', 'space', 'vinyl_space')
+    box.schema.user.grant('my_user', 'read,write', 'space', 'memtx_space')
+    box.space.memtx_space:create_index('pk', {sequence = true})
+    box.space.memtx_space:create_index('sk', {parts = {2, 'unsigned'}})
+    box.space.vinyl_space:create_index('pk', {sequence = true})
+    box.space.vinyl_space:create_index('sk', {parts = {2, 'unsigned'}})
+    box.space.memtx_space:truncate()
+    box.space.vinyl_space:truncate()
+    box.space.memtx_space:format({{'a', 'unsigned'}, {'b', 'unsigned'}})
+    box.space.vinyl_space:format({{'a', 'unsigned'}, {'b', 'unsigned'}})
+    box.schema.func.create('my_func')
+end;
+---
+...
+function drop()
+    box.schema.func.drop('my_func')
+    box.space.memtx_space:truncate()
+    box.space.vinyl_space:truncate()
+    box.schema.user.revoke('my_user', 'read,write', 'space', 'memtx_space')
+    box.schema.role.revoke('my_role', 'read,write', 'space', 'vinyl_space')
+    box.space.memtx_space:drop()
+    box.space.vinyl_space:drop()
+    box.schema.user.revoke('my_user', 'my_role')
+    box.schema.user.drop('my_user')
+    box.schema.role.drop('my_role')
+end;
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+box.begin() create() box.rollback()
+---
+...
+box.begin() create() box.commit()
+---
+...
+box.begin() drop() box.rollback()
+---
+...
+box.begin() drop() box.commit()
+---
+...
diff --git a/test/box/transaction.test.lua b/test/box/transaction.test.lua
index 8cd3e8ba..0792cc3c 100644
--- a/test/box/transaction.test.lua
+++ b/test/box/transaction.test.lua
@@ -41,12 +41,8 @@ f = fiber.create(sloppy);
 -- ensure it's rolled back automatically
 while f:status() ~= 'dead' do fiber.sleep(0) end;
 -- transactions and system spaces
--- some operation involves more than one ddl spaces, so they should fail
-box.begin() box.schema.space.create('test');
-box.rollback();
-box.begin() box.schema.user.create('test');
-box.rollback();
--- but this is Ok now
+box.begin() box.schema.space.create('test') box.rollback();
+box.begin() box.schema.user.create('test') box.rollback();
 box.begin() box.schema.func.create('test') box.rollback();
 box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv') box.rollback();
 space = box.schema.space.create('test');
@@ -341,16 +337,13 @@ box.space.vinyl:select{};
 -- Two DDL satements in a row
 box.begin()
 box.space.test:truncate()
-box.space.test:truncate();
-
--- A transaction is left open due to an exception in the above fragment
+box.space.test:truncate()
 box.rollback();
 
 -- Two DDL stateemnts on different engines
 box.begin()
 box.space.memtx:truncate()
-box.space.vinyl:truncate();
-
+box.space.vinyl:truncate()
 box.rollback();
 
 box.space.memtx:select{};
@@ -381,3 +374,44 @@ s = box.schema.space.create('test')
 box.begin() s:create_index('pk') fiber.sleep(0)
 box.commit() -- error
 s:drop()
+
+--
+-- Multiple DDL statements in the same transaction.
+--
+test_run:cmd("setopt delimiter ';'")
+function create()
+    box.schema.role.create('my_role')
+    box.schema.user.create('my_user')
+    box.schema.user.grant('my_user', 'my_role')
+    box.schema.space.create('memtx_space', {engine = 'memtx'})
+    box.schema.space.create('vinyl_space', {engine = 'vinyl'})
+    box.schema.role.grant('my_role', 'read,write', 'space', 'vinyl_space')
+    box.schema.user.grant('my_user', 'read,write', 'space', 'memtx_space')
+    box.space.memtx_space:create_index('pk', {sequence = true})
+    box.space.memtx_space:create_index('sk', {parts = {2, 'unsigned'}})
+    box.space.vinyl_space:create_index('pk', {sequence = true})
+    box.space.vinyl_space:create_index('sk', {parts = {2, 'unsigned'}})
+    box.space.memtx_space:truncate()
+    box.space.vinyl_space:truncate()
+    box.space.memtx_space:format({{'a', 'unsigned'}, {'b', 'unsigned'}})
+    box.space.vinyl_space:format({{'a', 'unsigned'}, {'b', 'unsigned'}})
+    box.schema.func.create('my_func')
+end;
+function drop()
+    box.schema.func.drop('my_func')
+    box.space.memtx_space:truncate()
+    box.space.vinyl_space:truncate()
+    box.schema.user.revoke('my_user', 'read,write', 'space', 'memtx_space')
+    box.schema.role.revoke('my_role', 'read,write', 'space', 'vinyl_space')
+    box.space.memtx_space:drop()
+    box.space.vinyl_space:drop()
+    box.schema.user.revoke('my_user', 'my_role')
+    box.schema.user.drop('my_user')
+    box.schema.role.drop('my_role')
+end;
+test_run:cmd("setopt delimiter ''");
+
+box.begin() create() box.rollback()
+box.begin() create() box.commit()
+box.begin() drop() box.rollback()
+box.begin() drop() box.commit()
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 30f305e9..c59c85f1 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -2215,8 +2215,62 @@ box.schema.user.revoke('guest', 'write', 'space', 'ddl_test')
 box.commit();
 ---
 ...
--- index and space drop are not currently supported (because of truncate)
-s.index.pk:drop();
+box.begin()
+s.index.pk:drop()
+s:drop()
+box.commit();
+---
+...
+--
+-- Only the first statement in a transaction is allowed to be
+-- a yielding DDL statement (index build, space format check).
+--
+s = box.schema.space.create('test', {engine = engine});
+---
+...
+_ = s:create_index('pk');
+---
+...
+s:insert{1, 1};
+---
+- [1, 1]
+...
+-- ok
+box.begin()
+s:create_index('sk', {parts = {2, 'unsigned'}})
+box.commit();
+---
+...
+s.index.sk:drop();
+---
+...
+-- ok
+box.begin()
+s:format({{'a', 'unsigned'}, {'b', 'unsigned'}})
+box.commit();
+---
+...
+s:format({});
+---
+...
+-- error
+box.begin()
+s.index.pk:alter{sequence = true}
+s:create_index('sk', {parts = {2, 'unsigned'}});
+---
+- error: Can not perform index build in a multi-statement transaction
+...
+box.rollback();
+---
+...
+-- error
+box.begin()
+s.index.pk:alter{sequence = true}
+s:format({{'a', 'unsigned'}, {'b', 'unsigned'}});
+---
+- error: Can not perform space format check in a multi-statement transaction
+...
+box.rollback();
 ---
 ...
 s:drop();
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index 25ce8ee6..1670b548 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -827,8 +827,43 @@ box.begin()
 box.schema.user.revoke('guest', 'write', 'space', 'ddl_test')
 box.commit();
 
--- index and space drop are not currently supported (because of truncate)
-s.index.pk:drop();
+box.begin()
+s.index.pk:drop()
+s:drop()
+box.commit();
+
+--
+-- Only the first statement in a transaction is allowed to be
+-- a yielding DDL statement (index build, space format check).
+--
+s = box.schema.space.create('test', {engine = engine});
+_ = s:create_index('pk');
+s:insert{1, 1};
+
+-- ok
+box.begin()
+s:create_index('sk', {parts = {2, 'unsigned'}})
+box.commit();
+s.index.sk:drop();
+
+-- ok
+box.begin()
+s:format({{'a', 'unsigned'}, {'b', 'unsigned'}})
+box.commit();
+s:format({});
+
+-- error
+box.begin()
+s.index.pk:alter{sequence = true}
+s:create_index('sk', {parts = {2, 'unsigned'}});
+box.rollback();
+
+-- error
+box.begin()
+s.index.pk:alter{sequence = true}
+s:format({{'a', 'unsigned'}, {'b', 'unsigned'}});
+box.rollback();
+
 s:drop();
 
 --



More information about the Tarantool-patches mailing list