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 53B822427F for ; Mon, 18 Jun 2018 11:42:57 -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 iZMvN8FubtfW for ; Mon, 18 Jun 2018 11:42:57 -0400 (EDT) Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 A70AC24270 for ; Mon, 18 Jun 2018 11:42:56 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3 09/10] sql: move Triggers to server References: <45ff716a-8295-50d8-f317-cc28f9d0dba4@tarantool.org> From: Vladislav Shpilevoy Message-ID: <0f277954-4d97-2896-7e4f-a3d78b87c4d1@tarantool.org> Date: Mon, 18 Jun 2018 18:42:53 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Kirill Shcherbatov , tarantool-patches@freelists.org Hello. Thanks for the fixes! See 3 comments below. On 15/06/2018 19:21, Kirill Shcherbatov wrote: >> 1. When you rollback insertion, that means you have no old_trigger. >> So sql_trigger_name(old_trigger) crashes. Please, add a test >> and fix the problem. > No, there is no problem. Maybe "old_trigger" is a little confusing name, but on_rollback->data = new_trigger; for insert and it is not NULL. > New ErrInjection test validates this case. > > --- a/test/sql/errinj.test.lua > +++ b/test/sql/errinj.test.lua > @@ -44,3 +44,18 @@ errinj.set("ERRINJ_IPROTO_TX_DELAY", false) > > box.sql.execute('DROP TABLE test') > box.schema.user.revoke('guest', 'read,write,execute', 'universe') > + > + 1. Please, start a sentence with capital letter and finish with dot. 2. Put here link to the issue of the form: -- -- gh-####: description. -- 3. I have pushed some fixes in a separate commit. Please, look and squash. After the patch will be LGTM. > +-- test crash on error.injection > +box.sql.execute("CREATE TABLE t1(id INTEGER PRIMARY KEY, a INTEGER);"); > +box.sql.execute("CREATE TABLE t2(id INTEGER PRIMARY KEY, a INTEGER);"); > +box.error.injection.set("ERRINJ_WAL_IO", true) > +box.sql.execute("CREATE TRIGGER t1t INSERT ON t1 BEGIN INSERT INTO t2 VALUES (1, 1); END;") > +box.sql.execute("CREATE INDEX t1a ON t1(a);") > +box.error.injection.set("ERRINJ_WAL_IO", false) > +box.sql.execute("CREATE TRIGGER t1t INSERT ON t1 BEGIN INSERT INTO t2 VALUES (1, 1); END;") > +box.sql.execute("INSERT INTO t1 VALUES (3, 3);") > +box.sql.execute("SELECT * from t1"); > +box.sql.execute("SELECT * from t2"); > +box.sql.execute("DROP TABLE t1;") > +box.sql.execute("DROP TABLE t2;") > >