From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: "n.pettik" <korablev@tarantool.org>, tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH 2/2] sql: transactional DDL Date: Wed, 24 Jul 2019 22:58:07 +0200 [thread overview] Message-ID: <981da4b5-8c1e-9acb-5983-3ccfe9cee25f@tarantool.org> (raw) In-Reply-To: <F05B7622-DD90-4826-BB5C-2B6D9487DAC4@tarantool.org> Hi! Thanks for the comments! On 24/07/2019 15:23, n.pettik wrote: > > Thanks, the only dubious nit I can spot: should we > set initiateTTrans for DDL operations which consist > of one box operations? Yeah, I thought about it, and even made it and reverted back several times having some doubts 'code consistency vs transaction necessity'. After all I decided, that each DDL op should be transactional, even if consists of one statement. Because 1) it is consistent with other SQL DDL, 2) it protects us from the case when SQL DDL will complicate. I think, the latter matters the most, taking into account incoming information schema, when even a simple index creation will insert into _index, into something like _constraint_name to ensure name uniqueness, etc. > Like: create index, drop index, > create trigger, drop trigger, drop constraint etc > For instance, we don’t set it for TRUNCATE stmt. Good catch, now I will. ---------------------------------------------------------------- diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 06eab7f2d..c5e41cbf3 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -779,6 +779,7 @@ cmd ::= with(C) DELETE FROM fullname(X) indexed_opt(I) where_opt(W). { /////////////////////////// The TRUNCATE statement ///////////////////////////// // cmd ::= TRUNCATE TABLE fullname(X). { + pParse->initiateTTrans = true; sql_table_truncate(pParse, X); } ---------------------------------------------------------------- > The only two operations which require starting transactions > are CREATE TABLE and RENAME. Do not forget about DROP TABLE. > Btw, when checking > results of RENAME I’d also check that operation (update of > ‘CREATE TRIGGER …' string) performed on _trigger has > been committed/rollbacked. > Agree. ---------------------------------------------------------------- diff --git a/test/sql/ddl.result b/test/sql/ddl.result index 834d9757f..a47e15bb4 100644 --- a/test/sql/ddl.result +++ b/test/sql/ddl.result @@ -187,7 +187,58 @@ box.space.T1:count() == 0 and box.space.T2:count() == 0 | - true | ... -box.execute('DROP TABLE t1') +-- +-- Rename transactionally changes name of the table and its +-- mentioning in trigger bodies. +-- +_trigger_index = box.space._trigger.index.space_id + | --- + | ... +test_run:cmd("setopt delimiter '$'") + | --- + | - true + | ... +box.execute([[CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW + BEGIN + INSERT INTO t2 VALUES(1); + END; ]])$ + | --- + | - row_count: 1 + | ... + +box.begin() +box.execute('ALTER TABLE t1 RENAME TO t1_new;') +sql = _trigger_index:select(box.space.T1_NEW.id)[1].opts.sql +assert(sql:find('T1_NEW')) +box.rollback()$ + | --- + | ... +test_run:cmd("setopt delimiter ''")$ + | --- + | - true + | ... + +sql = _trigger_index:select(box.space.T1.id)[1].opts.sql + | --- + | ... +not sql:find('T1_NEW') and sql:find('t1') ~= nil + | --- + | - true + | ... + +box.execute('ALTER TABLE t1 RENAME TO t1_new;') + | --- + | - row_count: 0 + | ... +sql = _trigger_index:select(box.space.T1_NEW.id)[1].opts.sql + | --- + | ... +sql:find('T1_NEW') ~= nil + | --- + | - true + | ... + +box.execute('DROP TABLE t1_new') | --- | - row_count: 1 | ... diff --git a/test/sql/ddl.test.lua b/test/sql/ddl.test.lua index 300f8b20c..371f0339e 100644 --- a/test/sql/ddl.test.lua +++ b/test/sql/ddl.test.lua @@ -85,7 +85,32 @@ box.begin() truncate_both() box.commit() box.space.T1:count() == 0 and box.space.T2:count() == 0 -box.execute('DROP TABLE t1') +-- +-- Rename transactionally changes name of the table and its +-- mentioning in trigger bodies. +-- +_trigger_index = box.space._trigger.index.space_id +test_run:cmd("setopt delimiter '$'") +box.execute([[CREATE TRIGGER t1t AFTER INSERT ON t1 FOR EACH ROW + BEGIN + INSERT INTO t2 VALUES(1); + END; ]])$ + +box.begin() +box.execute('ALTER TABLE t1 RENAME TO t1_new;') +sql = _trigger_index:select(box.space.T1_NEW.id)[1].opts.sql +assert(sql:find('T1_NEW')) +box.rollback()$ +test_run:cmd("setopt delimiter ''")$ + +sql = _trigger_index:select(box.space.T1.id)[1].opts.sql +not sql:find('T1_NEW') and sql:find('t1') ~= nil + +box.execute('ALTER TABLE t1 RENAME TO t1_new;') +sql = _trigger_index:select(box.space.T1_NEW.id)[1].opts.sql +sql:find('T1_NEW') ~= nil + +box.execute('DROP TABLE t1_new') box.execute('DROP TABLE t2') -- ----------------------------------------------------------------
next prev parent reply other threads:[~2019-07-24 20:56 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-19 22:49 [tarantool-patches] [PATCH 0/2] SQL TXN DDL Vladislav Shpilevoy 2019-07-19 22:49 ` [tarantool-patches] [PATCH 1/2] alter: do not access space in drop ck commit trigger Vladislav Shpilevoy 2019-07-19 22:49 ` [tarantool-patches] [PATCH 2/2] sql: transactional DDL Vladislav Shpilevoy 2019-07-20 7:42 ` [tarantool-patches] " Konstantin Osipov 2019-07-20 14:02 ` Vladislav Shpilevoy 2019-07-20 15:16 ` Konstantin Osipov 2019-07-22 13:59 ` n.pettik 2019-07-22 21:35 ` Vladislav Shpilevoy 2019-07-24 13:23 ` n.pettik 2019-07-24 20:58 ` Vladislav Shpilevoy [this message] 2019-07-25 12:04 ` n.pettik 2019-07-31 13:19 ` [tarantool-patches] Re: [PATCH 0/2] SQL TXN DDL Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=981da4b5-8c1e-9acb-5983-3ccfe9cee25f@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [PATCH 2/2] sql: transactional DDL' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox