From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Wed, 10 Jul 2019 23:43:58 +0300 From: Konstantin Osipov Subject: Re: [PATCH v2 3/3] ddl: allow to execute non-yielding DDL statements in transactions Message-ID: <20190710204358.GO5619@atlas> References: <9bc3823f622c5f4f1bd5defe0ad96b96f2217f25.1562763283.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9bc3823f622c5f4f1bd5defe0ad96b96f2217f25.1562763283.git.vdavydov.dev@gmail.com> To: Vladimir Davydov Cc: tarantool-patches@freelists.org List-ID: * Vladimir Davydov [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