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 55CCD25E23 for ; Thu, 14 Jun 2018 12:12:43 -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 7iTxRRmzVHSO for ; Thu, 14 Jun 2018 12:12:43 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 149E520341 for ; Thu, 14 Jun 2018 12:12:43 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 09/11] sql: new _trigger space format with space_id References: <36b21e2d-d459-d373-7c56-02537de5fdd9@tarantool.org> From: Kirill Shcherbatov Message-ID: <579ec1a5-31f5-4aa5-8eb9-98b3fab4b2b3@tarantool.org> Date: Thu, 14 Jun 2018 19:12:40 +0300 MIME-Version: 1.0 In-Reply-To: <36b21e2d-d459-d373-7c56-02537de5fdd9@tarantool.org> Content-Type: text/plain; charset=utf-8 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: tarantool-patches@freelists.org Cc: "v.shpilevoy@tarantool.org" On 13.06.2018 21:53, Vladislav Shpilevoy wrote: > Thanks for the patch! Almost ok. See 5 minor comments below. > > On 09/06/2018 12:32, Kirill Shcherbatov wrote: >> As we would like to lookup triggers by space_id in future >> on space deletion to delete assocoated _trigger tuples we > > 1. "assocoated" - typo. fixed > 2. Unnecessary diff. Either do this: {data, data}, or this { data, data }. > Not this: {data, data }. Same below. _index:insert{_trigger.id, 0, 'primary', 'tree', { unique = true }, - {{0, 'string'}} } + {{0, 'string'}}} log.info("create index secondary on _trigger") _index:insert{_trigger.id, 1, 'space_id', 'tree', { unique = false }, - {{1, 'unsigned'}} } + {{1, 'unsigned'}}} > 3. Out of 80. - mp_sizeof_str(trigger_stmt_new_len) + mp_sizeof_uint(space_id); + mp_sizeof_str(trigger_stmt_new_len) + + mp_sizeof_uint(space_id); > 4. 'is refer to' is incorrect construction. Maybe did you mean 'refers to'? - /** The ID of space the trigger is refer to. */ + /** The ID of space the trigger refers to. */ > 5. How about to test space_id is stored in _trigger? I mean test > box.space._trigger:select{} full result. Not two columns only. --- a/test/sql/upgrade.test.lua +++ b/test/sql/upgrade.test.lua @@ -25,9 +25,16 @@ box.space._index:get({box.space._space.index['name']:get('T1').id, 0}) box.sql.execute("CREATE TABLE t(x INTEGER PRIMARY KEY);") box.sql.execute("CREATE TABLE t_out(x INTEGER PRIMARY KEY);") box.sql.execute("CREATE TRIGGER t1t AFTER INSERT ON t BEGIN INSERT INTO t_out VALUES(1); END;") +box.sql.execute("CREATE TRIGGER t2t AFTER INSERT ON t BEGIN INSERT INTO t_out VALUES(2); END;") box.space._space.index['name']:get('T') box.space._space.index['name']:get('T_OUT') -box.space._trigger:get('T1T') +t1t = box.space._trigger:get('T1T') +t2t = box.space._trigger:get('T2T') +t1t.name +t1t.opts +t2t.name +t2t.opts +assert(t1t.id == t2t.id) And some extra changed required by next message review: +++ b/src/box/schema_def.h @@ -217,6 +217,12 @@ enum { BOX_SPACE_SEQUENCE_FIELD_IS_GENERATED = 2, }; +enum { + BOX_TRIGGER_FIELD_NAME = 0, + BOX_TRIGGER_FIELD_SPACE_ID = 1, + BOX_TRIGGER_FIELD_OPTS = 2, +}; + @@ -1320,7 +1320,7 @@ void tarantoolSqlite3LoadSchema(InitData *init) ptr = mp_decode_str(&field, &len); name = strndup(ptr, len); - field = tuple_field(tuple, 2); + field = tuple_field(tuple, BOX_TRIGGER_FIELD_OPTS); @@ -670,10 +670,10 @@ int tarantoolSqlite3RenameTrigger(const char *trig_name, return SQL_TARANTOOL_ERROR; assert(tuple != NULL); assert(tuple_field_count(tuple) == 3); - const char *field = box_tuple_field(tuple, 1); + const char *field = box_tuple_field(tuple, BOX_TRIGGER_FIELD_SPACE_ID); assert(mp_typeof(*field) == MP_UINT); uint32_t space_id = mp_decode_uint(&field); - field = box_tuple_field(tuple, 2); + field = box_tuple_field(tuple, BOX_TRIGGER_FIELD_OPTS);