Re: [PATCH v2 3/3] ddl: allow to execute non-yielding DDL statements in transactions

[Konstantin Osipov <kostja@tarantool.org>]

* Vladimir Davydov <vdavydov.dev@gmail.com> [19/07/10 16:13]:

Please write a follow up patch which changes box/lua/schema.lua
operations to use transactions wherever possible.

> 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 two things that must be noted explicitly. The first is 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.
> 
> The second subtlety lays in vinyl_index_commit_modify. Before the commit
> we assumed that if statement lsn is <= vy_lsm::commit_lsn, then it must
> be local recovery from WAL. Now it's not true, because there may be
> several operations for the same index in a transaction, and they all
> will receive the same signature in on_commit trigger. We could, of
> course, try to assign different signatures to them, but that would look
> cumbersome - better simply allow lsn <= vy_lsm::commit_lsn after local
> recovery, there's actually nothing wrong about that.
> 
> 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.
> ---
>  src/box/alter.cc              | 14 --------
>  src/box/errcode.h             |  2 +-
>  src/box/memtx_space.c         |  8 +++--
>  src/box/txn.c                 | 27 +++++++-------
>  src/box/txn.h                 | 25 ++++---------
>  src/box/user.cc               |  1 -
>  src/box/vinyl.c               | 23 ++++++++----
>  test/box/misc.result          |  1 +
>  test/box/on_replace.result    | 53 +++++++++++++---------------
>  test/box/on_replace.test.lua  | 13 +++----
>  test/box/transaction.result   | 82 ++++++++++++++++++++++++++++++++-----------
>  test/box/transaction.test.lua | 56 +++++++++++++++++++++++------
>  test/engine/ddl.result        | 58 ++++++++++++++++++++++++++++--
>  test/engine/ddl.test.lua      | 39 ++++++++++++++++++--
>  14 files changed, 273 insertions(+), 129 deletions(-)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index 612bcd89..c7be6b77 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -1816,7 +1816,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;
> @@ -2113,7 +2112,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;
> @@ -2305,7 +2303,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) {
> @@ -2535,7 +2532,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;
>  
> @@ -2686,7 +2682,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;
> @@ -2880,7 +2875,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 =
> @@ -3178,7 +3172,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;
> @@ -3227,7 +3220,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;
> @@ -3305,7 +3297,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;
> @@ -3458,7 +3449,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;
> @@ -3661,7 +3651,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;
>  
> @@ -3806,7 +3795,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;
> @@ -4189,7 +4177,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;
> @@ -4480,7 +4467,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..af044f0c 100644
> --- a/src/box/vinyl.c
> +++ b/src/box/vinyl.c
> @@ -919,16 +919,17 @@ vinyl_index_commit_modify(struct index *index, int64_t lsn)
>  	       env->status == VINYL_FINAL_RECOVERY_LOCAL ||
>  	       env->status == VINYL_FINAL_RECOVERY_REMOTE);
>  
> -	if (lsn <= lsm->commit_lsn) {
> +	if (env->status == VINYL_FINAL_RECOVERY_LOCAL &&
> +	    lsn <= lsm->commit_lsn) {
>  		/*
> -		 * This must be local recovery from WAL, when
> -		 * the operation has already been committed to
> -		 * vylog.
> +		 * The statement we are recovering from WAL has
> +		 * been successfully written to vylog so we must
> +		 * not replay it.
>  		 */
> -		assert(env->status == VINYL_FINAL_RECOVERY_LOCAL);
>  		return;
>  	}
>  
> +	assert(lsm->commit_lsn <= lsn);
>  	lsm->commit_lsn = lsn;
>  
>  	vy_log_tx_begin();
> @@ -1121,7 +1122,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 +4353,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();
>  
>  --
> -- 
> 2.11.0
> 

-- 
Konstantin Osipov, Moscow, Russia