From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 965442A595 for ; Wed, 20 Mar 2019 16:31:49 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ap0C8b8p5wVz for ; Wed, 20 Mar 2019 16:31:49 -0400 (EDT) Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 1BEC52A586 for ; Wed, 20 Mar 2019 16:31:49 -0400 (EDT) From: Georgy Kirichenko Subject: [tarantool-patches] [PATCH 3/3] Require for single statement not autocommit in case of ddl Date: Wed, 20 Mar 2019 23:31:35 +0300 Message-Id: <53290003330b698f98a6b96e6c320740f1f661b9.1553112720.git.georgy@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: Georgy Kirichenko Allow single statement transactions within begin/commit in case of an ddl operation instead of auto commit requirements. This is essential for a transactional applier. Needed for: #2798 --- src/box/memtx_engine.c | 20 ++------ src/box/txn.c | 9 +++- src/box/txn.h | 5 ++ test/box/ddl.result | 8 +-- test/box/ddl.test.lua | 4 +- test/box/transaction.result | 53 +++++--------------- test/box/transaction.test.lua | 27 ++++------ test/engine/ddl.result | 92 ++++++++++++++++++++++++++++++++++ test/engine/ddl.test.lua | 55 ++++++++++++++++++++ test/engine/truncate.result | 3 +- test/sql-tap/trigger1.test.lua | 12 ++--- test/sql/delete.result | 8 ++- test/sql/delete.test.lua | 3 +- 13 files changed, 204 insertions(+), 95 deletions(-) diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c index d468d1cd8..924f8bbc4 100644 --- a/src/box/memtx_engine.c +++ b/src/box/memtx_engine.c @@ -384,15 +384,7 @@ static int memtx_engine_begin(struct engine *engine, struct txn *txn) { (void)engine; - /* - * Register a trigger to rollback transaction on yield. - * This must be done in begin(), since it's - * the first thing txn invokes after txn->n_stmts++, - * to match with trigger_clear() in rollbackStatement(). - */ - if (txn->is_autocommit == false) { - memtx_init_txn(txn); - } + (void)txn; return 0; } @@ -404,15 +396,9 @@ memtx_engine_begin_statement(struct engine *engine, struct txn *txn) if (txn->engine_tx == NULL) { struct space *space = txn_last_stmt(txn)->space; - if (space->def->id > BOX_SYSTEM_ID_MAX && - ! rlist_empty(&space->on_replace)) { - /** - * A space on_replace trigger may initiate - * a yield. - */ - assert(txn->is_autocommit); + if (space->def->id > BOX_SYSTEM_ID_MAX) + /* Setup triggers for non-ddl transactions. */ memtx_init_txn(txn); - } } return 0; } diff --git a/src/box/txn.c b/src/box/txn.c index deb4fac47..ad77dd76c 100644 --- a/src/box/txn.c +++ b/src/box/txn.c @@ -147,6 +147,7 @@ txn_begin(bool is_autocommit) txn->n_local_rows = 0; txn->n_remote_rows = 0; txn->is_autocommit = is_autocommit; + txn->is_single_statement = false; txn->has_triggers = false; txn->is_aborted = false; txn->in_sub_stmt = 0; @@ -191,6 +192,11 @@ txn_begin_stmt(struct space *space) diag_set(ClientError, ER_SUB_STMT_MAX); return NULL; } + if (!stailq_empty(&txn->stmts) && txn->is_single_statement) { + diag_set(ClientError, ER_UNSUPPORTED, + "DDL", "multi-statement transactions"); + return NULL; + } struct txn_stmt *stmt = txn_stmt_new(txn); if (stmt == NULL) { @@ -430,11 +436,12 @@ txn_abort(struct txn *txn) int txn_check_singlestatement(struct txn *txn, const char *where) { - if (!txn->is_autocommit || !txn_is_first_statement(txn)) { + if (!txn_is_first_statement(txn)) { diag_set(ClientError, ER_UNSUPPORTED, where, "multi-statement transactions"); return -1; } + txn->is_single_statement = true; return 0; } diff --git a/src/box/txn.h b/src/box/txn.h index c9829da9e..3572b005d 100644 --- a/src/box/txn.h +++ b/src/box/txn.h @@ -149,6 +149,11 @@ struct txn { * (statement end causes an automatic transaction commit). */ bool is_autocommit; + /** + * True if this transaction is allowed to have only one statement. + * Used for ddl operations. + */ + bool is_single_statement; /** * True if the transaction was aborted so should be * rolled back at commit. diff --git a/test/box/ddl.result b/test/box/ddl.result index 3d6d07f43..e48284ffd 100644 --- a/test/box/ddl.result +++ b/test/box/ddl.result @@ -299,11 +299,7 @@ box.space._collation:replace(c) --- - error: collation does not support alter ... -box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU') ---- -- error: Space _collation does not support multi-statement transactions -... -box.rollback() +box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU') box.rollback() --- ... box.internal.collation.create('test', 'ICU', 'ru_RU') @@ -645,11 +641,11 @@ _ = fiber.create(function() end); --- ... +-- Should be Ok for now box.begin() test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) box.commit(); --- -- error: DDL does not support multi-statement transactions ... test_run:cmd("setopt delimiter ''"); --- diff --git a/test/box/ddl.test.lua b/test/box/ddl.test.lua index 5c147cdfb..101bc6f9b 100644 --- a/test/box/ddl.test.lua +++ b/test/box/ddl.test.lua @@ -131,8 +131,7 @@ c = box.space._collation:get{1}:totable() c[2] = 'unicode_test' box.space._collation:replace(c) -box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU') -box.rollback() +box.begin() box.internal.collation.create('test2', 'ICU', 'ru_RU') box.rollback() box.internal.collation.create('test', 'ICU', 'ru_RU') box.internal.collation.exists('test') @@ -260,6 +259,7 @@ _ = fiber.create(function() c:put(true) end); +-- Should be Ok for now box.begin() test_latch:create_index("sec2", {unique = true, parts = {2, 'unsigned'}}) box.commit(); diff --git a/test/box/transaction.result b/test/box/transaction.result index 8a4d11d3b..7def44d5d 100644 --- a/test/box/transaction.result +++ b/test/box/transaction.result @@ -84,77 +84,50 @@ 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'); --- -- error: Space _schema does not support multi-statement transactions -... -box.rollback(); ---- -... -box.begin() box.schema.func.create('test'); ---- -- error: Space _func does not support multi-statement transactions +- error: DDL does not support multi-statement transactions ... box.rollback(); --- ... box.begin() box.schema.user.create('test'); --- -- error: Space _user does not support multi-statement transactions -... -box.rollback(); ---- -... -box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv'); ---- -- error: Space _priv does not support multi-statement transactions -... -box.rollback(); ---- -... -box.begin() box.space._user:delete{box.schema.GUEST_ID}; ---- -- error: Space _user does not support multi-statement transactions +- error: DDL does not support multi-statement transactions ... box.rollback(); --- ... -box.begin() box.space._space:delete{box.schema.CLUSTER_ID}; +-- but this is Ok now +box.begin() box.schema.func.create('test') box.rollback(); --- -- error: DDL does not support multi-statement transactions ... -box.rollback(); +box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv') box.rollback(); --- ... -box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false}; +space = box.schema.space.create('test'); --- -- error: Space _sequence does not support multi-statement transactions ... -box.rollback(); +box.begin() box.space._space:delete{space.id} box.rollback(); --- ... -box.begin() box.space._schema:insert{'test'}; +box.begin() box.space._space:delete{space.id} box.commit(); --- -- error: Space _schema does not support multi-statement transactions ... -box.rollback(); +box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false} box.rollback(); --- ... -box.begin() box.space._cluster:insert{123456789, 'abc'}; +box.begin() box.space._schema:insert{'test'} box.rollback(); --- -- error: Space _cluster does not support multi-statement transactions ... -box.rollback(); +box.begin() box.space._cluster:insert{30, '00000000-0000-0000-0000-000000000001'} box.rollback(); --- ... s = box.schema.space.create('test'); --- ... -box.begin() index = s:create_index('primary'); ---- -- error: DDL does not support multi-statement transactions -... -box.rollback(); +box.begin() index = s:create_index('primary') box.rollback(); --- ... index = s:create_index('primary'); diff --git a/test/box/transaction.test.lua b/test/box/transaction.test.lua index 8f7dfedec..0d212ca29 100644 --- a/test/box/transaction.test.lua +++ b/test/box/transaction.test.lua @@ -41,27 +41,22 @@ 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.func.create('test'); -box.rollback(); box.begin() box.schema.user.create('test'); box.rollback(); -box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv'); -box.rollback(); -box.begin() box.space._user:delete{box.schema.GUEST_ID}; -box.rollback(); -box.begin() box.space._space:delete{box.schema.CLUSTER_ID}; -box.rollback(); -box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false}; -box.rollback(); -box.begin() box.space._schema:insert{'test'}; -box.rollback(); -box.begin() box.space._cluster:insert{123456789, 'abc'}; -box.rollback(); +-- but this is Ok now +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'); +box.begin() box.space._space:delete{space.id} box.rollback(); +box.begin() box.space._space:delete{space.id} box.commit(); +box.begin() box.space._sequence:insert{1, 1, 'test', 1, 1, 2, 1, 0, false} box.rollback(); +box.begin() box.space._schema:insert{'test'} box.rollback(); +box.begin() box.space._cluster:insert{30, '00000000-0000-0000-0000-000000000001'} box.rollback(); s = box.schema.space.create('test'); -box.begin() index = s:create_index('primary'); -box.rollback(); +box.begin() index = s:create_index('primary') box.rollback(); index = s:create_index('primary'); t = nil function multi() diff --git a/test/engine/ddl.result b/test/engine/ddl.result index 8d34d5ef4..c493bd4ac 100644 --- a/test/engine/ddl.result +++ b/test/engine/ddl.result @@ -2075,6 +2075,15 @@ i3:select() ... -- Check that recovery works. inspector:cmd("restart server default") +test_run = require('test_run') +--- +... +inspector = test_run.new() +--- +... +engine = inspector:get_cfg('engine') +--- +... s = box.space.test --- ... @@ -2130,3 +2139,86 @@ box.snapshot() s:drop() --- ... +-- test ddl operation within begin/commit/rollback +-- acquire free space id +space = box.schema.space.create('ddl_test', {engine = engine}) +--- +... +id = space.id +--- +... +space:drop() +--- +... +inspector:cmd("setopt delimiter ';'") +--- +- true +... +box.begin() +s = box.schema.space.create('ddl_test', {engine = engine, id = id}) +box.rollback(); +--- +... +box.begin() +s = box.schema.space.create('ddl_test', {engine = engine, id = id}) +box.commit(); +--- +... +box.begin() +s:create_index('pk') +box.rollback(); +--- +... +box.begin() +s:create_index('pk') +box.commit(); +--- +... +s:replace({1}); +--- +- [1] +... +s:replace({2}); +--- +- [2] +... +s:replace({3}); +--- +- [3] +... +box.begin() +s:truncate() +box.commit(); +--- +... +s:select(); +--- +- [] +... +box.begin() +box.schema.user.grant('guest', 'write', 'space', 'ddl_test') +box.rollback(); +--- +... +box.begin() +box.schema.user.grant('guest', 'write', 'space', 'ddl_test') +box.commit(); +--- +... +box.begin() +box.schema.user.revoke('guest', 'write', 'space', 'ddl_test') +box.rollback(); +--- +... +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(); +--- +... +s:drop(); +--- +... diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua index cdaf7a5bf..636f6c3b9 100644 --- a/test/engine/ddl.test.lua +++ b/test/engine/ddl.test.lua @@ -759,6 +759,9 @@ i3:select() -- Check that recovery works. inspector:cmd("restart server default") +test_run = require('test_run') +inspector = test_run.new() +engine = inspector:get_cfg('engine') s = box.space.test s.index.i1:select() @@ -775,3 +778,55 @@ s.index.i1:select() box.snapshot() s:drop() + +-- test ddl operation within begin/commit/rollback +-- acquire free space id +space = box.schema.space.create('ddl_test', {engine = engine}) +id = space.id +space:drop() + +inspector:cmd("setopt delimiter ';'") +box.begin() +s = box.schema.space.create('ddl_test', {engine = engine, id = id}) +box.rollback(); + +box.begin() +s = box.schema.space.create('ddl_test', {engine = engine, id = id}) +box.commit(); + +box.begin() +s:create_index('pk') +box.rollback(); + +box.begin() +s:create_index('pk') +box.commit(); + +s:replace({1}); +s:replace({2}); +s:replace({3}); + +box.begin() +s:truncate() +box.commit(); +s:select(); + +box.begin() +box.schema.user.grant('guest', 'write', 'space', 'ddl_test') +box.rollback(); + +box.begin() +box.schema.user.grant('guest', 'write', 'space', 'ddl_test') +box.commit(); + +box.begin() +box.schema.user.revoke('guest', 'write', 'space', 'ddl_test') +box.rollback(); + +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(); +s:drop(); diff --git a/test/engine/truncate.result b/test/engine/truncate.result index b4de787fb..277e7bda6 100644 --- a/test/engine/truncate.result +++ b/test/engine/truncate.result @@ -24,14 +24,13 @@ box.begin() ... s:truncate() --- -- error: DDL does not support multi-statement transactions ... box.commit() --- ... s:select() --- -- - [123] +- [] ... s:drop() --- diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua index 2984d4c21..924e57b58 100755 --- a/test/sql-tap/trigger1.test.lua +++ b/test/sql-tap/trigger1.test.lua @@ -849,28 +849,24 @@ test:do_catchsql_test( -- }) -test:do_catchsql_test( +test:do_execsql_test( "trigger1-16.8", [[ START TRANSACTION; CREATE TRIGGER tr168 INSERT ON tA BEGIN INSERT INTO t16 values(1); END; + ROLLBACK; ]], { - 1, [[Space _trigger does not support multi-statement transactions]] }) -test:execsql [[ - ROLLBACK; -]] - -test:do_catchsql_test( +test:do_execsql_test( "trigger1-16.9", [[ START TRANSACTION; DROP TRIGGER t16err3; + ROLLBACK; ]], { - 1, [[Space _trigger does not support multi-statement transactions]] }) -- MUST_WORK_TEST -- #------------------------------------------------------------------------- diff --git a/test/sql/delete.result b/test/sql/delete.result index e024dd697..dcefa8d5f 100644 --- a/test/sql/delete.result +++ b/test/sql/delete.result @@ -76,17 +76,21 @@ box.sql.execute("INSERT INTO t1 VALUES(1, 1, 'one');") box.sql.execute("INSERT INTO t1 VALUES(2, 2, 'two');") --- ... --- Can't truncate in transaction. +-- Truncate rollback box.sql.execute("START TRANSACTION") --- ... box.sql.execute("TRUNCATE TABLE t1;") --- -- error: DDL does not support multi-statement transactions ... box.sql.execute("ROLLBACK") --- ... +box.sql.execute("SELECT * FROM t1") +--- +- - [1, 1, 'one'] + - [2, 2, 'two'] +... -- Can't truncate view. box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;") --- diff --git a/test/sql/delete.test.lua b/test/sql/delete.test.lua index 5a0813071..b61a993a8 100644 --- a/test/sql/delete.test.lua +++ b/test/sql/delete.test.lua @@ -50,10 +50,11 @@ box.sql.execute("CREATE TABLE t1(id INT PRIMARY KEY, a INT, b TEXT);") box.sql.execute("INSERT INTO t1 VALUES(1, 1, 'one');") box.sql.execute("INSERT INTO t1 VALUES(2, 2, 'two');") --- Can't truncate in transaction. +-- Truncate rollback box.sql.execute("START TRANSACTION") box.sql.execute("TRUNCATE TABLE t1;") box.sql.execute("ROLLBACK") +box.sql.execute("SELECT * FROM t1") -- Can't truncate view. box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;") -- 2.21.0