[tarantool-patches] Re: [PATCH v2 09/11] sql: new _trigger space format with space_id
Kirill Shcherbatov
kshcherbatov at tarantool.org
Thu Jun 14 19:12:40 MSK 2018
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);
More information about the Tarantool-patches
mailing list