[PATCH v2 3/3] ddl: allow to execute non-yielding DDL statements in transactions
Konstantin Osipov
kostja at tarantool.org
Wed Jul 10 23:43:58 MSK 2019
* Vladimir Davydov <vdavydov.dev at 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
More information about the Tarantool-patches
mailing list