From: "n.pettik" <korablev@tarantool.org> To: tarantool-patches@freelists.org Cc: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Subject: [tarantool-patches] Re: [PATCH 2/5] schema: add new system space for FK constraints Date: Wed, 1 Aug 2018 23:54:27 +0300 [thread overview] Message-ID: <FAAA2017-8559-4DE3-9662-8572DF598979@tarantool.org> (raw) In-Reply-To: <24bb9776-b33d-a68f-8738-3967ffd013ea@tarantool.org> > Thanks for the patch! See my answers, 4 comments and a > patch on the branch. I’ve squashed your fixes. >> +++ b/src/box/alter.cc >> @@ -1920,7 +1920,7 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) >> * Can't drop index if foreign key constraints references >> * this index. >> */ >> - if (new_tuple == NULL) { >> + if (old_index != NULL && new_tuple == NULL) { >> Also, I didn’t get what you mean by mentioning index_def_change_requires_rebuild. >> Can index id change on its renaming? > > Index_id can not change. But a one can remove an index part, or add a new > one. Or change nullability that breaks uniqueness. I do not see how do you > deal with that. To detect such dramatic changes > index_def_change_requires_rebuild() is used in on_replace_dd_index(). Ok: +/** + * Check whether given index is referenced by some foreign key + * constraint or not. + * + * @fkey_head List of FK constraints belonging to parent space. + * @iid Index id which belongs to parent space and to be tested. + * @retval True if at least one FK constraint references this + * index; false otherwise. + */ +bool +index_is_fkey_referenced(struct rlist *fkey_head, uint32_t iid) +{ + struct fkey *fk; + rlist_foreach_entry(fk, fkey_head, parent_link) { + if (fk->index_id == iid) + return true; + } + return false; +} + @@ -1980,13 +2001,11 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * Can't drop index if foreign key constraints * references this index. */ - struct fkey *fk; - rlist_foreach_entry(fk, &old_space->parent_fkey, parent_link) { - if (fk->index_id == iid) { - tnt_raise(ClientError, ER_ALTER_SPACE, - space_name(old_space), - "can not drop referenced index"); - } + if (index_is_fkey_referenced(&old_space->parent_fkey, + iid)) { + tnt_raise(ClientError, ER_ALTER_SPACE, + space_name(old_space), + "can not drop referenced index”); @@ -2048,6 +2067,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) (void) new MoveIndex(alter, old_index->def->iid); } else if (index_def_change_requires_rebuild(old_index, index_def)) { + if (index_is_fkey_referenced(&old_space->parent_fkey, + iid)) { + tnt_raise(ClientError, ER_ALTER_SPACE, + space_name(old_space), + "can not alter referenced index"); + } ++ b/test/sql/foreign-keys.test.lua @@ -86,6 +86,11 @@ t = box.space._fk_constraint:insert(t) -- box.sql.execute("DROP INDEX i1 on t1;") +-- Referenced index can't be altered as well, if alter leads to +-- rebuild of index (e.g. index still can be renamed). +box.space._index:replace{512, 1, 'I1', 'tree', {unique = true}, {{field = 0, type = 'unsigned', is_nullable = true}}} +box.space._index:replace{512, 1, 'I2', 'tree', {unique = true}, {{field = 1, type = 'unsigned', is_nullable = true}}} +++ b/test/sql/foreign-keys.result @@ -181,6 +181,16 @@ box.sql.execute("DROP INDEX i1 on t1;") --- - error: 'Can''t modify space ''T1'': can not drop referenced index' ... +-- Referenced index can't be altered as well, if alter leads to +-- rebuild of index (e.g. index still can be renamed). +box.space._index:replace{512, 1, 'I1', 'tree', {unique = true}, {{field = 0, type = 'unsigned', is_nullable = true}}} +--- +- error: 'Can''t modify space ''T1'': can not alter referenced index' +... +box.space._index:replace{512, 1, 'I2', 'tree', {unique = true}, {{field = 1, type = 'unsigned', is_nullable = true}}} +--- +- [512, 1, 'I2', 'tree', {'unique': true}, [{'field': 1, 'type': 'unsigned', 'is_nullable': true}]] +... >> I use this format since it is easier to avoid confusing parent and child ids >> (i.e. what comes first and what comes second). Even if we add separate >> convenient Lua API for that purpose. >> If you insist on doing that, I will change format. > > Lua API is not about _fk_constraint space. Lua API will be like > space:create_fk() with fancy arguments etc. > > I think, _fk_constraint should be compact and easy to store and parse, > not to manually read or write. My opinion is that 3 the best options > exist: > > {{..., ..., ...}, {..., ..., ...}} - array of 2 arrays. First is child, > second is parent. Or: > > , {..., ..., ...}, {..., ..., ...}, - create separate fields in > _fk_constraint for child and parent arrays. Or: > > {child = {..., ..., ...}, parent = {..., ..., ...}} - map with two > keys and two array values. > > I do not insist on my format but think we should consult > Kostja and Kirill on the final one. Ok, finally we decided to implement second variant: +++ b/src/box/alter.cc @@ -3516,10 +3516,10 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) } /** - * Decode MsgPack array of links. It consists from maps: - * {parent_id (UINT) : child_id (UINT)}. + * Decode MsgPack arrays of links. They are stored as two + * separate arrays filled with unsigned fields numbers. * - * @param data MsgPack array of links. + * @param tuple Tuple to be inserted into _fk_constraints. * @param[out] out_count Count of links. * @param constraint_name Constraint name to use in error * messages. @@ -3528,65 +3528,48 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) * @retval Array of links. */ static struct field_link * -fkey_links_decode(const char *data, uint32_t *out_count, +fkey_links_decode(const struct tuple *tuple, uint32_t *out_count, const char *constraint_name, uint32_t constraint_len, uint32_t errcode) { - assert(mp_typeof(*data) == MP_ARRAY); - uint32_t count = mp_decode_array(&data); + const char *parent_cols = + tuple_field_with_type_xc(tuple, + BOX_FK_CONSTRAINT_FIELD_PARENT_COLS, + MP_ARRAY); + assert(mp_typeof(*parent_cols) == MP_ARRAY); + uint32_t count = mp_decode_array(&parent_cols); if (count == 0) { tnt_raise(ClientError, errcode, tt_cstr(constraint_name, constraint_len), "at least one link must be specified"); } + const char *child_cols = + tuple_field_with_type_xc(tuple, + BOX_FK_CONSTRAINT_FIELD_CHILD_COLS, + MP_ARRAY); + assert(mp_typeof(*child_cols) == MP_ARRAY); + if (mp_decode_array(&child_cols) != count) { + tnt_raise(ClientError, errcode, + tt_cstr(constraint_name, constraint_len), + "number of referenced and referencing fields " + "must be the same"); + } *out_count = count; size_t size = count * sizeof(struct field_link); struct field_link *region_links = (struct field_link *) region_alloc_xc(&fiber()->gc, size); memset(region_links, 0, size); - const char **map = &data; for (uint32_t i = 0; i < count; ++i) { - uint32_t map_sz = mp_decode_map(map); - if (map_sz != 2) { + if (mp_typeof(*parent_cols) != MP_UINT || + mp_typeof(*child_cols) != MP_UINT) { tnt_raise(ClientError, errcode, - tt_cstr(constraint_name, constraint_len), - tt_sprintf("link must be map with 2 fields")); - } - for (uint8_t j = 0; j < map_sz; ++j) { - if (mp_typeof(**map) != MP_STR) { - tnt_raise(ClientError, errcode, - tt_cstr(constraint_name, - constraint_len), - tt_sprintf("link %d is not map "\ - "with string keys", i)); - } - uint32_t key_len; - const char *key = mp_decode_str(map, &key_len); - if (mp_typeof(**map) != MP_UINT) { - tnt_raise(ClientError, errcode, - tt_cstr(constraint_name, - constraint_len), - tt_sprintf("value of %d link is not "\ - "unsigned", i)); - } - if (key_len == 6 && - memcmp(key, "parent", key_len) == 0) { - region_links[i].parent_field = - mp_decode_uint(map); - } else if (key_len == 5 && - memcmp(key, "child", key_len) == 0) { - region_links[i].child_field = - mp_decode_uint(map); - } else { - char *errmsg = tt_static_buf(); - snprintf(errmsg, TT_STATIC_BUF_LEN, - "unexpected key of link %d '%.*s'", i, - key_len, key); - tnt_raise(ClientError, errcode, - tt_cstr(constraint_name, - constraint_len), errmsg); - } + tt_cstr(constraint_name, + constraint_len), + tt_sprintf("value of %d link is not "\ + "unsigned", i)); } + region_links[i].parent_field = mp_decode_uint(&parent_cols); + region_links[i].child_field = mp_decode_uint(&child_cols); } return region_links; } @@ -3605,12 +3588,9 @@ fkey_def_new_from_tuple(const struct tuple *tuple, uint32_t errcode) "constraint name is too long"); } identifier_check_xc(name, name_len); - const char *links_raw = - tuple_field_with_type_xc(tuple, BOX_FK_CONSTRAINT_FIELD_LINKS, - MP_ARRAY); uint32_t link_count; - struct field_link *links = fkey_links_decode(links_raw, &link_count, - name, name_len, errcode); + struct field_link *links = fkey_links_decode(tuple, &link_count, name, + name_len, errcode); +++ b/src/box/schema_def.h @@ -235,7 +235,8 @@ enum { BOX_FK_CONSTRAINT_FIELD_MATCH = 4, BOX_FK_CONSTRAINT_FIELD_ON_DELETE = 5, BOX_FK_CONSTRAINT_FIELD_ON_UPDATE = 6, - BOX_FK_CONSTRAINT_FIELD_LINKS = 7, + BOX_FK_CONSTRAINT_FIELD_CHILD_COLS = 7, + BOX_FK_CONSTRAINT_FIELD_PARENT_COLS = 8, +++ b/src/box/lua/upgrade.lua @@ -516,7 +516,8 @@ local function upgrade_to_2_1_0() {name='match', type='string'}, {name='on_delete', type='string'}, {name='on_update', type='string'}, - {name='links', type='array'}} + {name='child_cols', type='array'}, + {name='parent_cols', type='array'}} +++ b/src/box/sql.c @@ -1256,7 +1256,7 @@ void tarantoolSqlite3LoadSchema(struct init_data *init) "CREATE TABLE \""TARANTOOL_SYS_FK_CONSTRAINT_NAME "\"(\"name\" TEXT, \"parent_id\" INT, \"child_id\" INT," "\"deferred\" INT, \"match\" TEXT, \"on_delete\" TEXT," - "\"on_update\" TEXT, \"links\"," + "\"on_update\" TEXT, \"child_cols\", \"parent_cols\”," Tests are fixed as well, but it is boring diff so if you want - see whole patch at the end of letter. Also, diff from the next patch: diff --git a/src/box/sql.c b/src/box/sql.c index bbd057a41..46a0c3472 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -1394,16 +1394,18 @@ tarantoolSqlite3MakeTableOpts(Table *pTable, const char *zSql, char *buf) } int -fkey_encode_links(const struct fkey_def *fkey, char *buf) +fkey_encode_links(uint32_t *links, uint32_t link_count, char *buf) { const struct Enc *enc = get_enc(buf); - char *p = enc->encode_array(buf, fkey->field_count); - for (uint32_t i = 0; i < fkey->field_count; ++i) { - p = enc->encode_map(p, 2); - p = enc->encode_str(p, "child", strlen("child")); - p = enc->encode_uint(p, fkey->links[i].child_field); - p = enc->encode_str(p, "parent", strlen("parent")); - p = enc->encode_uint(p, fkey->links[i].parent_field); + char *p = enc->encode_array(buf, link_count); + for (uint32_t i = 0; i < link_count; ++i) { + /* + * field_link consists of two uin32_t members, + * so if we calculate proper offset, we will + * get next parent/child member. + */ + size_t offset = sizeof(struct field_link) * i; + p = enc->encode_uint(p, *((char *) links + offset)); } return p - buf; } diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 80216524d..d2aa7f47c 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1567,7 +1567,7 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk) * Occupy registers for 8 fields: each member in * _constraint space plus one for final msgpack tuple. */ - int constr_tuple_reg = sqlite3GetTempRange(parse_context, 9); + int constr_tuple_reg = sqlite3GetTempRange(parse_context, 10); char *name_copy = sqlite3DbStrDup(parse_context->db, fk->name); if (name_copy == NULL) return; @@ -1600,22 +1600,54 @@ vdbe_emit_fkey_create(struct Parse *parse_context, const struct fkey_def *fk) fkey_action_strs[fk->on_delete], P4_STATIC); sqlite3VdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 6, 0, fkey_action_strs[fk->on_update], P4_STATIC); - size_t encoded_links_sz = fkey_encode_links(fk, NULL); + size_t encoded_parent_sz = fkey_encode_links(&fk->links[0].parent_field, + fk->field_count, NULL); + size_t encoded_child_sz = fkey_encode_links(&fk->links[0].child_field, + fk->field_count, NULL); + /* + * We are allocating memory for both parent and child + * arrays in the same chunk. Thus, first OP_Blob opcode + * interprets it as static memory, and the second one - + * as dynamic and releases memory. + */ char *encoded_links = sqlite3DbMallocRaw(parse_context->db, - encoded_links_sz); + encoded_child_sz + + encoded_parent_sz); if (encoded_links == NULL) { sqlite3DbFree(parse_context->db, name_copy); return; } - size_t real_links_sz = fkey_encode_links(fk, encoded_links); - sqlite3VdbeAddOp4(vdbe, OP_Blob, real_links_sz, constr_tuple_reg + 7, - SQL_SUBTYPE_MSGPACK, encoded_links, P4_DYNAMIC); - sqlite3VdbeAddOp3(vdbe, OP_MakeRecord, constr_tuple_reg, 8, - constr_tuple_reg + 8); + /* + * Here we use small memory trick: parent and child links + * are quite similar but assigned to different fields. + * So to avoid code duplication, we calculate offset + * and fetch proper parent or child link: + * + * +--------------------------------------+ + * | child | parent | child | parent| ... | + * |--------------------------------------| + * | link[0] | link[1] | ... | + * +--------------------------------------+ + */ + size_t real_parent_sz = fkey_encode_links(&fk->links[0].parent_field, + fk->field_count, + encoded_links); + size_t real_child_sz = fkey_encode_links(&fk->links[0].child_field, + fk->field_count, + encoded_links + + real_parent_sz); + sqlite3VdbeAddOp4(vdbe, OP_Blob, real_child_sz, constr_tuple_reg + 7, + SQL_SUBTYPE_MSGPACK, encoded_links + real_parent_sz, + P4_STATIC); + sqlite3VdbeAddOp4(vdbe, OP_Blob, real_parent_sz, constr_tuple_reg + 8, + SQL_SUBTYPE_MSGPACK, encoded_links, + P4_DYNAMIC); + sqlite3VdbeAddOp3(vdbe, OP_MakeRecord, constr_tuple_reg, 9, + constr_tuple_reg + 9); sqlite3VdbeAddOp2(vdbe, OP_SInsert, BOX_FK_CONSTRAINT_ID, - constr_tuple_reg + 8); + constr_tuple_reg + 9); sqlite3VdbeChangeP5(vdbe, OPFLAG_NCHANGE); - sqlite3ReleaseTempRange(parse_context, constr_tuple_reg, 9); + sqlite3ReleaseTempRange(parse_context, constr_tuple_reg, 10); } static int diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h index 69c2b9bc6..cbf8e5acb 100644 --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -151,15 +151,19 @@ int tarantoolSqlite3MakeTableOpts(Table * pTable, const char *zSql, char *buf); /** * Encode links of given foreign key constraint into MsgPack. + * Note: this function is adapted to encode only members of + * struct field_link since it uses offset of (sizeof(field_link)) + * to fetch next member. * - * @param fkey Encode links of this foreign key contraint. + * @param links Array of unsigned number representing parent or + * child field numbers. + * @param link_count Number of members in @links. * @param buf Buffer to hold encoded links. Can be NULL. * In this case function would simply calculate * memory required for such buffer. * @retval Length of encoded array. */ int -fkey_encode_links(const struct fkey_def *fkey, char *buf); +fkey_encode_links(uint32_t *links, uint32_t link_count, char *buf); > >>> >>> 10. Above you have said that order may be different, but here I see >>> that once you have found a unique index, you merely check that it >>> has sequentially the same field numbers as the fk. >> For some reason I forgot to fix this when was preparing patch. >> Fixed by now. Also, according to ANSI (11.8 4.a): >> • Each referenced column shall identify a column of the referenced table and the same column shall not be identi ed more than once. >> @@ -3710,6 +3710,41 @@ on_drop_or_replace_fkey_commit(struct trigger *trigger, void *event) >> fkey_delete(fk); >> } >> +static int >> +cmp_uint32(const void *_a, const void *_b) >> +{ >> + const uint32_t *a = (const uint32_t *) _a, *b = (const uint32_t *) _b; >> + if (*a == *b) >> + return 0; >> + return (*a > *b) ? 1 : -1; >> +} > > Looks like in Tarantool source we have plenty of various cmp functions > for qsort. I greped uint32_compare, cmp_uint32 (your), cmp_i64, size_compator, > nums_comparator, int64_cmp. How about to merge them into trivia/util.h as > cmp_uint32, cmp_int64, cmp_size? Since I don’t need ‘em anymore in current patch, lets simply open issue: https://github.com/tarantool/tarantool/issues/3598 > >> + >> +/** >> + * ANSI SQL doesn't allow list of referenced fields to contain >> + * duplicates. >> + */ >> +static int >> +fkey_links_check_duplicates(struct fkey_def *fk_def) >> +{ >> + uint32_t *parent_fields = >> + (uint32_t *) region_alloc(&fiber()->gc, fk_def->field_count); >> + if (parent_fields == NULL) { >> + tnt_raise(OutOfMemory, fk_def->field_count, "region", >> + "parent_fields"); >> + } >> + for (uint32_t i = 0; i < fk_def->field_count; ++i) >> + parent_fields[i] = fk_def->links[i].parent_field; >> + qsort(parent_fields, fk_def->field_count, sizeof(*parent_fields), >> + cmp_uint32); >> + uint32_t prev_val = parent_fields[0]; >> + for (uint32_t i = 1; i < fk_def->field_count; ++i) { >> + if (prev_val == parent_fields[i]) >> + return -1; >> + prev_val = parent_fields[i]; > > Why not > > for (uint32_t i = 1; i < fk_def->field_count; ++i) { > if (parent_fields[i - 1] == parent_fields[i]) > return -1; > } > > So you do not need prev_val actually. Also lets do tnt_raise here > instead of return -1. Nevermind. I reworked this function taking into consideration suggested optimization. Look below. > >> + } >> + return 0; >> +} >> + >> on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) >> @@ -3777,6 +3812,11 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) >> "field collation mismatch"); >> } >> } >> + if (fkey_links_check_duplicates(fk_def)) { >> + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, >> + fk_def->name, "referenced fields can not " >> + "contain duplicates"); >> + } >> Probably this is not best implementation. As an improvement I can add kind of optimisation: >> *pseudo code below* >> uint_64 mask; >> for parent_field in fk_def: >> if (pk_mask & (((uint64_t) 1) << parent_field ==1) >> return -1; >> column_mask_set_field(&mask, parent_field); >> end >> if (pk_mask & (((uint64_t) 1) << 63)) != 0 >> fkey_links_check_duplicates(…) // Full version of checking. >> else >> return 1; >> Is it worth the effort? I mean here we waste O(field_count) in case the largest >> filedno in parent table > 63. On the other hand, I don’t think that many users >> have tables with field count > 63, so it is likely to be reasonable. > > I like this optimization. What is more, I think, we can remove the sorting > things above then. For users who have field numbers > 63 the smallest problem > is slow alter. So for them we can just use O(field_count ^ 2) with no region > and sorting. Ok, done: /** * ANSI SQL doesn't allow list of referenced fields to contain - * duplicates. /** * ANSI SQL doesn't allow list of referenced fields to contain - * duplicates. + * duplicates. Firstly, we try to follow the easiest way: + * if all referenced fields numbers are less than 63, we can + * use bit mask. Otherwise, fall through slow check where we + * use O(field_cont^2) simple nested cycle iterations. */ -static int +static void fkey_links_check_duplicates(struct fkey_def *fk_def) { - uint32_t *parent_fields = - (uint32_t *) region_alloc(&fiber()->gc, fk_def->field_count); - if (parent_fields == NULL) { - tnt_raise(OutOfMemory, fk_def->field_count, "region", - "parent_fields"); - } - for (uint32_t i = 0; i < fk_def->field_count; ++i) - parent_fields[i] = fk_def->links[i].parent_field; - qsort(parent_fields, fk_def->field_count, sizeof(*parent_fields), - cmp_uint32); - uint32_t prev_val = parent_fields[0]; - for (uint32_t i = 1; i < fk_def->field_count; ++i) { - if (prev_val == parent_fields[i]) - return -1; - prev_val = parent_fields[i]; + uint64_t field_mask = 0; + for (uint32_t i = 0; i < fk_def->field_count; ++i) { + uint32_t parent_field = fk_def->links[i].parent_field; + if (parent_field > 63) + goto slow_check; + if (field_mask & (((uint64_t) 1) << parent_field)) + goto error; + column_mask_set_fieldno(&field_mask, parent_field); } - return 0; + return; +slow_check: + for (uint32_t i = 0; i < fk_def->field_count; ++i) { + uint32_t parent_field = fk_def->links[i].parent_field; + for (uint32_t j = i + 1; j < fk_def->field_count; ++j) { + if (parent_field == fk_def->links[j].parent_field) + goto error; + } + } + return; +error: + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, + "referenced fields can not contain duplicates"); > >> Moreover, it would be cool if column_mask_set_fieldno() worked the same >> as bit_set() from lib/bit, i.e. returned previous bit value. > > You are free to extend the API. Well, I see that in other places return value is not used, so lets keep things as they are. > >>> >>>> + fk_index = idx; >>>> + break; >>>> + } >>>> + if (fk_index == NULL) { >>>> + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, >>>> + fk_def->name, "referenced fields don't " >>>> + "compose unique index"); >>>> + } >>>> + struct fkey *fkey = (struct fkey *) malloc(sizeof(*fkey)); >>> >>> 11. I know how you like memory mapping. How about merge fkey_def into >>> fkey memory? I see, that it is relatively simple and linear thing. >> What is the point of doing this? Do you suggest to allocate enough memory for fkey >> right in fkey_def_new_from_tuple() ? Like this: >> @@ -3536,7 +3536,8 @@ fkey_def_new_from_tuple(const struct tuple *tuple, uint32_t errcode) >> struct field_link *links = fkey_links_decode(links_raw, &link_count, >> name, name_len, errcode); >> size_t fkey_sz = fkey_def_sizeof(link_count, name_len); >> - struct fkey_def *fk_def = (struct fkey_def *) malloc(fkey_sz); >> + struct fkey_def *fk_def = (struct fkey_def *) malloc(fkey_sz + >> + sizeof(struct fkey); >> If so, it would be quite confusing I think (since function returns only >> fkey_def, but memory would be allocated for fkey_def + fkey). >> If you mean smth else or insist on this change, I will fix it. > > I just like when structure is self-sufficient. When it can be freed with > one free, and can fit in one cache line for better performance on access fields, > maybe partially. If you do not want to merge them, I do not insist though. I don’t feel that it really matters: creation of FK constraints doesn’t seem to be frequently executed thing. On the other hand, such strange allocation may be misleading for those who investigate source code. > >>>> diff --git a/src/box/fkey.c b/src/box/fkey.c >>>> new file mode 100644 >>>> index 000000000..e45889a0d >>>> --- /dev/null >>>> +++ b/src/box/fkey.c >>>> +void >>>> +fkey_trigger_delete(struct sqlite3 *db, struct sql_trigger *p) >>>> +{ >>> >>> 12. Why do you need this function when sql_trigger_delete exists? >> Memory layout of FK trigger differs from ordinary one (from sql/fkey.c): >> size_t trigger_size = sizeof(struct sql_trigger) + >> sizeof(TriggerStep) + nFrom + 1; >> trigger = >> (struct sql_trigger *)sqlite3DbMallocZero(db, >> trigger_size); >> One can see, memory for TriggerStep, sql_trigger and name of >> target table is allocated in one chunk. Thus, fkey_trigger_delete() >> doesn’t release memory for TriggerStep. Overall, if compare >> these functions fkey_trigger_delete() looks much simpler. > > I very do not like that the same structure is allocated on different > layouts, it confuses. It is ok only during alter when we temporary > allocate some things on region. Lets allocate this trigger like > ordinary ones and remove fkey_trigger_delete. I like when a > structure is compact, as I say in the previous comment, but not when > it is sometimes compact and other times not. Ok, I slightly reworked allocation for FK triggers and it allows to use everywhere sql_trigger_delete(): +++ b/src/box/sql/fkey.c @@ -1251,20 +1251,14 @@ fkActionTrigger(struct Parse *pParse, struct Table *pTab, struct FKey *pFKey, pWhere, 0, 0, 0, 0, 0, 0); pWhere = 0; } - - /* Disable lookaside memory allocation */ - db->lookaside.bDisable++; - - size_t trigger_size = sizeof(struct sql_trigger) + - sizeof(TriggerStep) + nFrom + 1; - trigger = - (struct sql_trigger *)sqlite3DbMallocZero(db, - trigger_size); + trigger = (struct sql_trigger *)sqlite3DbMallocZero(db, + sizeof(*trigger)); if (trigger != NULL) { - pStep = trigger->step_list = (TriggerStep *)&trigger[1]; + size_t step_size = sizeof(TriggerStep) + nFrom + 1; + trigger->step_list = sqlite3DbMallocZero(db, step_size); + pStep = trigger->step_list; pStep->zTarget = (char *)&pStep[1]; - memcpy((char *)pStep->zTarget, zFrom, nFrom); - + memcpy(pStep->zTarget, zFrom, nFrom); pStep->pWhere = sqlite3ExprDup(db, pWhere, EXPRDUP_REDUCE); pStep->pExprList = @@ -1278,15 +1272,12 @@ fkActionTrigger(struct Parse *pParse, struct Table *pTab, struct FKey *pFKey, } } - /* Re-enable the lookaside buffer, if it was disabled earlier. */ - db->lookaside.bDisable—; >>>> diff --git a/test/engine/iterator.result b/test/engine/iterator.result >>>> index a36761df8..ba9b0545a 100644 >>>> --- a/test/engine/iterator.result >>>> +++ b/test/engine/iterator.result >>>> @@ -4211,7 +4211,7 @@ s:replace{35} >>>> ... >>>> state, value = gen(param,state) >>>> --- >>>> -- error: 'builtin/box/schema.lua:1049: usage: next(param, state)' >>>> +- error: 'builtin/box/schema.lua:1055: usage: next(param, state)' >>> >>> 18. This test fails on each schema change. Lets remove this file:line >>> from the error message alongside the patch. >> Ok: > > I meant remove 'file:line' from the error message. Not the test itself. > For example, you can catch the error and match the message using > string.match("usage: next(param, state)”). Ok, returned back (sorry, I just used Occam's razor): +++ b/test/engine/iterator.test.lua @@ -399,6 +399,11 @@ value gen,param,state = i:pairs({35}) state, value = gen(param,state) value +s:replace{35} +f = function() return gen(param, state) end +_, errmsg = pcall(f) +errmsg:match('usage: next%(param, state%)') +value +++ b/test/engine/iterator.result @@ -4207,6 +4207,24 @@ value --- - null ... +s:replace{35} +--- +- [35] +... +f = function() return gen(param, state) end +--- +... +_, errmsg = pcall(f) +--- +... +errmsg:match('usage: next%(param, state%)') +--- +- 'usage: next(param, state)' +... +value +--- +- null +... > >> diff --git a/src/box/alter.cc b/src/box/alter.cc >> index 7b6bd1a5a..c5d1f75df 100644 >> --- a/src/box/alter.cc >> +++ b/src/box/alter.cc >> +/** >> + * Decode MsgPack array of links. It consists from maps: >> + * {parent_id (UINT) : child_id (UINT)}. >> + * >> + * @param data MsgPack array of links. >> + * @param[out] out_count Count of links. >> + * @param constraint_name Constraint name to use in error >> + * messages. >> + * @param constraint_len Length of constraint name. >> + * @param errcode Errcode for client errors. >> + * @retval Array of links. >> + */ >> +static struct field_link * >> +fkey_links_decode(const char *data, uint32_t *out_count, >> + const char *constraint_name, uint32_t constraint_len, >> + uint32_t errcode) >> +{ >> + assert(mp_typeof(*data) == MP_ARRAY); >> + uint32_t count = mp_decode_array(&data); >> + if (count == 0) { >> + tnt_raise(ClientError, errcode, >> + tt_cstr(constraint_name, constraint_len), >> + "at least one link must be specified"); >> + } >> + *out_count = count; >> + size_t size = count * sizeof(struct field_link); >> + struct field_link *region_links = >> + (struct field_link *) region_alloc_xc(&fiber()->gc, size); >> + memset(region_links, 0, size); >> + const char **map = &data; >> + for (uint32_t i = 0; i < count; ++i) { >> + uint32_t map_sz = mp_decode_map(map); >> + if (map_sz != 2) { >> + tnt_raise(ClientError, errcode, >> + tt_cstr(constraint_name, constraint_len), >> + tt_sprintf("link must be map with 2 fields")); >> + } >> + for (uint8_t j = 0; j < map_sz; ++j) { >> + if (mp_typeof(**map) != MP_STR) { >> + tnt_raise(ClientError, errcode, >> + tt_cstr(constraint_name, >> + constraint_len), >> + tt_sprintf("link %d is not map "\ >> + "with string keys", i)); >> + } >> + uint32_t key_len; >> + const char *key = mp_decode_str(map, &key_len); > > 1. Until we have this https://github.com/tarantool/tarantool/issues/1253 > we should check tuple field internals. Here we need to check for MP_UINT > or the crash is possible: > > box.cfg{} > t = {'fk_1', 1, 1, false, 'simple', 'restrict', 'restrict', {{child = 'crash', parent = 'crash'}}} > box.space._fk_constraint:insert(t) > > Assertion failed: (0), function mp_decode_uint, file /Users/v.shpilevoy/Work/Repositories/tarantool/src/lib/msgpuck/msgpuck.h, line 1434. > Process 76355 stopped > * thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGABRT > frame #0: 0x00007fff56cf5b66 libsystem_kernel.dylib`__pthread_kill + 10 > libsystem_kernel.dylib`__pthread_kill: > -> 0x7fff56cf5b66 <+10>: jae 0x7fff56cf5b70 ; <+20> > 0x7fff56cf5b68 <+12>: movq %rax, %rdi > 0x7fff56cf5b6b <+15>: jmp 0x7fff56cecae9 ; cerror_nocancel > 0x7fff56cf5b70 <+20>: retq > Target 0: (tarantool) stopped. Added appropriate checks (see fixes to format of _fk_constraint space). And tests: @@ -58,6 +58,8 @@ t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {{wro box.space._fk_constraint:insert(t) t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {{child = 13, parent = 1}}} box.space._fk_constraint:insert(t) +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {{child = 'crash', parent = 'crash'}}} +box.space._fk_constraint:insert(t) +++ b/test/sql/foreign-keys.result @@ -135,6 +135,14 @@ box.space._fk_constraint:insert(t) - error: 'Failed to create foreign key constraint ''fk_1'': foreign key refers to nonexistent field' ... +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {{child = 'crash', parent = 'crash'}}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': value of 0 link is not + unsigned' +... > >> + if (key_len == 6 && >> + memcmp(key, "parent", key_len) == 0) { >> + region_links[i].parent_field = >> + mp_decode_uint(map); >> + } else if (key_len == 5 && >> + memcmp(key, "child", key_len) == 0) { >> + region_links[i].child_field = >> + mp_decode_uint(map); >> + } else { >> + char *errmsg = tt_static_buf(); >> + snprintf(errmsg, TT_STATIC_BUF_LEN, >> + "unexpected key of link %d '%.*s'", i, >> + key_len, key); >> + tnt_raise(ClientError, errcode, >> + tt_cstr(constraint_name, >> + constraint_len), errmsg); >> + } >> + } >> + } >> + return region_links; >> +} >> + >> +/** >> + * Remove FK constraint from child's list. >> + * Entries in child list are supposed to be unique >> + * by their name. >> + * >> + * @param list List of child FK constraints. >> + * @param fkey_name Name of constraint to be removed. >> + * @retval FK being removed. >> + */ >> +static struct fkey * >> +fkey_remove_child(struct rlist *list, const char *fkey_name) >> +{ >> + struct fkey *fk; >> + rlist_foreach_entry(fk, list, child_link) { >> + if (strcmp(fkey_name, fk->def->name) == 0) { >> + rlist_del_entry(fk, child_link); >> + return fk; >> + } >> + } > > 2. In all 3 places of usage you remove the fd from parent list too. > Lets move the deletion there and rename this function to something > like fkey_snatch_by_name or something. Ok: @@ -3625,21 +3626,22 @@ fkey_def_new_from_tuple(const struct tuple *tuple, uint32_t errcode) } /** - * Remove FK constraint from child's list. - * Entries in child list are supposed to be unique - * by their name. + * Remove FK constraint from child's and parent's lists and + * return it. Entries in child list are supposed to be + * unique by their name. * * @param list List of child FK constraints. * @param fkey_name Name of constraint to be removed. * @retval FK being removed. */ static struct fkey * -fkey_remove_child(struct rlist *list, const char *fkey_name) +fkey_grab_by_name(struct rlist *list, const char *fkey_name) { struct fkey *fk; rlist_foreach_entry(fk, list, child_link) { if (strcmp(fkey_name, fk->def->name) == 0) { rlist_del_entry(fk, child_link); + rlist_del_entry(fk, parent_link); @@ -3670,9 +3672,8 @@ on_replace_fkey_rollback(struct trigger *trigger, void *event) struct fkey *fk = (struct fkey *)trigger->data; struct space *parent = space_by_id(fk->def->parent_id); struct space *child = space_by_id(fk->def->child_id); - struct fkey *old_fkey = fkey_remove_child(&child->child_fkey, + struct fkey *old_fkey = fkey_grab_by_name(&child->child_fkey, fk->def->name); - rlist_del_entry(old_fkey, parent_link); @@ -3859,9 +3852,8 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) txn_on_rollback(txn, on_rollback); } else { struct fkey *old_fk = - fkey_remove_child(&child_space->child_fkey, + fkey_grab_by_name(&child_space->child_fkey, fk_def->name); - rlist_del_entry(old_fk, parent_link); rlist_add_entry(&child_space->child_fkey, fkey, child_link); rlist_add_entry(&parent_space->parent_fkey, fkey, @@ -3886,9 +3878,8 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) struct space *child_space = space_cache_find_xc(fk_def->child_id); struct fkey *old_fkey = - fkey_remove_child(&child_space->child_fkey, + fkey_grab_by_name(&child_space->child_fkey, fk_def->name); - rlist_del_entry(old_fkey, parent_link); >> diff --git a/src/box/fkey.h b/src/box/fkey.h >> new file mode 100644 >> index 000000000..0d537b1a7 >> --- /dev/null >> +++ b/src/box/fkey.h >> +/** Definition of foreign key constraint. */ >> +struct fkey_def { >> + /** Id of space containing the REFERENCES clause (child). */ >> + uint32_t child_id; >> + /** Id of space that the key points to (parent). */ >> + uint32_t parent_id; >> + /** Number of fields in this key. */ >> + uint32_t field_count; >> + /** True if constraint checking is deferred till COMMIT. */ >> + bool is_deferred; > > 3. What is the difference between this and pragma defer_foreign_keys? > Can we remove this flag or the pragma? Using pragma we can force defer FK checking (for all FK constraints despite their original is_deferred statuses) or vice versa — completely disable it. IDK whether smb need this feature or not, it is SQLite legacy, but still it seems to work. >> diff --git a/src/box/space.h b/src/box/space.h >> index 01a4af726..97650cffe 100644 >> --- a/src/box/space.h >> +++ b/src/box/space.h >> @@ -183,6 +183,15 @@ struct space { >> * of index id. >> */ >> struct index **index; >> + /** >> + * Lists of foreign key constraints. In SQL terms parent >> + * space is the "from" table i.e. the table that contains >> + * the REFERENCES clause. Child space is "to" table, in >> + * other words the table that is named in the REFERENCES >> + * clause. > > 4. Are you sure? > > https://en.wikipedia.org/wiki/Foreign_key > > "The table containing the foreign key is called the child table, > and the table containing the candidate key is called the > referenced or parent table.” It is definitely mistake. In other places I use correct terminology. Fixed. Updated patch: ======================================================================= diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt index a467d3517..6dd2c75b9 100644 --- a/src/box/CMakeLists.txt +++ b/src/box/CMakeLists.txt @@ -92,6 +92,7 @@ add_library(box STATIC space.c space_def.c sequence.c + fkey.c func.c func_def.c alter.cc diff --git a/src/box/alter.cc b/src/box/alter.cc index 7b6bd1a5a..5b55bfd7a 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -33,9 +33,11 @@ #include "user.h" #include "space.h" #include "index.h" +#include "fkey.h" #include "func.h" #include "coll_id_cache.h" #include "coll_id_def.h" +#include "column_mask.h" #include "txn.h" #include "tuple.h" #include "fiber.h" /* for gc_pool */ @@ -571,6 +573,14 @@ space_swap_triggers(struct space *new_space, struct space *old_space) old_space->sql_triggers = new_value; } +/** The same as for triggers - swap lists of FK constraints. */ +static void +space_swap_fkeys(struct space *new_space, struct space *old_space) +{ + rlist_swap(&new_space->child_fkey, &old_space->child_fkey); + rlist_swap(&new_space->parent_fkey, &old_space->parent_fkey); +} + /** * True if the space has records identified by key 'uid'. * Uses 'iid' index. @@ -781,9 +791,10 @@ alter_space_rollback(struct trigger *trigger, void * /* event */) space_fill_index_map(alter->old_space); space_fill_index_map(alter->new_space); /* - * Don't forget about space triggers. + * Don't forget about space triggers and foreign keys. */ space_swap_triggers(alter->new_space, alter->old_space); + space_swap_fkeys(alter->new_space, alter->old_space); struct space *new_space = space_cache_replace(alter->old_space); assert(new_space == alter->new_space); (void) new_space; @@ -879,9 +890,10 @@ alter_space_do(struct txn *txn, struct alter_space *alter) space_fill_index_map(alter->old_space); space_fill_index_map(alter->new_space); /* - * Don't forget about space triggers. + * Don't forget about space triggers and foreign keys. */ space_swap_triggers(alter->new_space, alter->old_space); + space_swap_fkeys(alter->new_space, alter->old_space); /* * The new space is ready. Time to update the space * cache with it. @@ -1742,6 +1754,18 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) space_name(old_space), "other views depend on this space"); } + /* + * No need to check existence of parent keys, + * since if we went so far, space would'n have + * any indexes. But referenced space has at least + * one referenced index which can't be dropped + * before constraint itself. + */ + if (! rlist_empty(&old_space->child_fkey)) { + tnt_raise(ClientError, ER_DROP_SPACE, + space_name(old_space), + "the space has foreign key constraints"); + } /** * The space must be deleted from the space * cache right away to achieve linearisable @@ -1842,6 +1866,26 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) } } +/** + * Check whether given index is referenced by some foreign key + * constraint or not. + * + * @fkey_head List of FK constraints belonging to parent space. + * @iid Index id which belongs to parent space and to be tested. + * @retval True if at least one FK constraint references this + * index; false otherwise. + */ +bool +index_is_fkey_referenced(struct rlist *fkey_head, uint32_t iid) +{ + struct fkey *fk; + rlist_foreach_entry(fk, fkey_head, parent_link) { + if (fk->index_id == iid) + return true; + } + return false; +} + /** * Just like with _space, 3 major cases: * @@ -1951,12 +1995,18 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * 3. Change of an index which does not require a rebuild. * 4. Change of an index which does require a rebuild. */ - /* - * First, move all unchanged indexes from the old space - * to the new one. - */ /* Case 1: drop the index, if it is dropped. */ if (old_index != NULL && new_tuple == NULL) { + /* + * Can't drop index if foreign key constraints + * references this index. + */ + if (index_is_fkey_referenced(&old_space->parent_fkey, + iid)) { + tnt_raise(ClientError, ER_ALTER_SPACE, + space_name(old_space), + "can not drop referenced index"); + } alter_space_move_indexes(alter, 0, iid); (void) new DropIndex(alter, old_index->def); } @@ -2017,6 +2067,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) (void) new MoveIndex(alter, old_index->def->iid); } else if (index_def_change_requires_rebuild(old_index, index_def)) { + if (index_is_fkey_referenced(&old_space->parent_fkey, + iid)) { + tnt_raise(ClientError, ER_ALTER_SPACE, + space_name(old_space), + "can not alter referenced index"); + } /* * Operation demands an index rebuild. */ @@ -3459,6 +3515,395 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) txn_on_commit(txn, on_commit); } +/** + * Decode MsgPack arrays of links. They are stored as two + * separate arrays filled with unsigned fields numbers. + * + * @param tuple Tuple to be inserted into _fk_constraints. + * @param[out] out_count Count of links. + * @param constraint_name Constraint name to use in error + * messages. + * @param constraint_len Length of constraint name. + * @param errcode Errcode for client errors. + * @retval Array of links. + */ +static struct field_link * +fkey_links_decode(const struct tuple *tuple, uint32_t *out_count, + const char *constraint_name, uint32_t constraint_len, + uint32_t errcode) +{ + const char *parent_cols = + tuple_field_with_type_xc(tuple, + BOX_FK_CONSTRAINT_FIELD_PARENT_COLS, + MP_ARRAY); + assert(mp_typeof(*parent_cols) == MP_ARRAY); + uint32_t count = mp_decode_array(&parent_cols); + if (count == 0) { + tnt_raise(ClientError, errcode, + tt_cstr(constraint_name, constraint_len), + "at least one link must be specified"); + } + const char *child_cols = + tuple_field_with_type_xc(tuple, + BOX_FK_CONSTRAINT_FIELD_CHILD_COLS, + MP_ARRAY); + assert(mp_typeof(*child_cols) == MP_ARRAY); + if (mp_decode_array(&child_cols) != count) { + tnt_raise(ClientError, errcode, + tt_cstr(constraint_name, constraint_len), + "number of referenced and referencing fields " + "must be the same"); + } + *out_count = count; + size_t size = count * sizeof(struct field_link); + struct field_link *region_links = + (struct field_link *) region_alloc_xc(&fiber()->gc, size); + memset(region_links, 0, size); + for (uint32_t i = 0; i < count; ++i) { + if (mp_typeof(*parent_cols) != MP_UINT || + mp_typeof(*child_cols) != MP_UINT) { + tnt_raise(ClientError, errcode, + tt_cstr(constraint_name, + constraint_len), + tt_sprintf("value of %d link is not "\ + "unsigned", i)); + } + region_links[i].parent_field = mp_decode_uint(&parent_cols); + region_links[i].child_field = mp_decode_uint(&child_cols); + } + return region_links; +} + +/** Create an instance of foreign key def constraint from tuple. */ +static struct fkey_def * +fkey_def_new_from_tuple(const struct tuple *tuple, uint32_t errcode) +{ + uint32_t name_len; + const char *name = + tuple_field_str_xc(tuple, BOX_FK_CONSTRAINT_FIELD_NAME, + &name_len); + if (name_len > BOX_NAME_MAX) { + tnt_raise(ClientError, errcode, + tt_cstr(name, BOX_INVALID_NAME_MAX), + "constraint name is too long"); + } + identifier_check_xc(name, name_len); + uint32_t link_count; + struct field_link *links = fkey_links_decode(tuple, &link_count, name, + name_len, errcode); + size_t fkey_sz = fkey_def_sizeof(link_count, name_len); + struct fkey_def *fk_def = (struct fkey_def *) malloc(fkey_sz); + if (fk_def == NULL) + tnt_raise(OutOfMemory, fkey_sz, "malloc", "fk_def"); + auto def_guard = make_scoped_guard([=] { free(fk_def); }); + memcpy(fk_def->name, name, name_len); + fk_def->name[name_len] = '\0'; + fk_def->links = (struct field_link *)((char *)&fk_def->name + + name_len + 1); + memcpy(fk_def->links, links, link_count * sizeof(struct field_link)); + fk_def->field_count = link_count; + fk_def->child_id = tuple_field_u32_xc(tuple, + BOX_FK_CONSTRAINT_FIELD_CHILD_ID); + fk_def->parent_id = + tuple_field_u32_xc(tuple, BOX_FK_CONSTRAINT_FIELD_PARENT_ID); + fk_def->is_deferred = + tuple_field_bool_xc(tuple, BOX_FK_CONSTRAINT_FIELD_DEFERRED); + const char *match = tuple_field_str_xc(tuple, + BOX_FK_CONSTRAINT_FIELD_MATCH, + &name_len); + fk_def->match = STRN2ENUM(fkey_match, match, name_len); + if (fk_def->match == fkey_match_MAX) { + tnt_raise(ClientError, errcode, fk_def->name, + "unknown MATCH clause"); + } + const char *on_delete_action = + tuple_field_str_xc(tuple, BOX_FK_CONSTRAINT_FIELD_ON_DELETE, + &name_len); + fk_def->on_delete = STRN2ENUM(fkey_action, on_delete_action, name_len); + if (fk_def->on_delete == fkey_action_MAX) { + tnt_raise(ClientError, errcode, fk_def->name, + "unknown ON DELETE action"); + } + const char *on_update_action = + tuple_field_str_xc(tuple, BOX_FK_CONSTRAINT_FIELD_ON_UPDATE, + &name_len); + fk_def->on_update = STRN2ENUM(fkey_action, on_update_action, name_len); + if (fk_def->on_update == fkey_action_MAX) { + tnt_raise(ClientError, errcode, fk_def->name, + "unknown ON UPDATE action"); + } + def_guard.is_active = false; + return fk_def; +} + +/** + * Remove FK constraint from child's and parent's lists and + * return it. Entries in child list are supposed to be + * unique by their name. + * + * @param list List of child FK constraints. + * @param fkey_name Name of constraint to be removed. + * @retval FK being removed. + */ +static struct fkey * +fkey_grab_by_name(struct rlist *list, const char *fkey_name) +{ + struct fkey *fk; + rlist_foreach_entry(fk, list, child_link) { + if (strcmp(fkey_name, fk->def->name) == 0) { + rlist_del_entry(fk, child_link); + rlist_del_entry(fk, parent_link); + return fk; + } + } + unreachable(); + return NULL; +} + +/** + * On rollback of creation we remove FK constraint from DD, i.e. + * from parent's and child's lists of constraints and + * release memory. + */ +static void +on_create_fkey_rollback(struct trigger *trigger, void *event) +{ + (void) event; + struct fkey *fk = (struct fkey *)trigger->data; + rlist_del_entry(fk, parent_link); + rlist_del_entry(fk, child_link); + fkey_delete(fk); +} + +/** Return old FK and release memory for the new one. */ +static void +on_replace_fkey_rollback(struct trigger *trigger, void *event) +{ + (void) event; + struct fkey *fk = (struct fkey *)trigger->data; + struct space *parent = space_by_id(fk->def->parent_id); + struct space *child = space_by_id(fk->def->child_id); + struct fkey *old_fkey = fkey_grab_by_name(&child->child_fkey, + fk->def->name); + fkey_delete(old_fkey); + rlist_add_entry(&child->child_fkey, fk, child_link); + rlist_add_entry(&parent->parent_fkey, fk, parent_link); +} + +/** On rollback of drop simply return back FK to DD. */ +static void +on_drop_fkey_rollback(struct trigger *trigger, void *event) +{ + (void) event; + struct fkey *fk_to_restore = (struct fkey *)trigger->data; + struct space *parent = space_by_id(fk_to_restore->def->parent_id); + struct space *child = space_by_id(fk_to_restore->def->child_id); + rlist_add_entry(&child->child_fkey, fk_to_restore, child_link); + rlist_add_entry(&parent->parent_fkey, fk_to_restore, parent_link); +} + +/** + * On commit of drop or replace we have already deleted old + * foreign key entry from both (parent's and child's) lists, + * so just release memory. + */ +static void +on_drop_or_replace_fkey_commit(struct trigger *trigger, void *event) +{ + (void) event; + struct fkey *fk = (struct fkey *)trigger->data; + fkey_delete(fk); +} + +/** + * ANSI SQL doesn't allow list of referenced fields to contain + * duplicates. Firstly, we try to follow the easiest way: + * if all referenced fields numbers are less than 63, we can + * use bit mask. Otherwise, fall through slow check where we + * use O(field_cont^2) simple nested cycle iterations. + */ +static void +fkey_links_check_duplicates(struct fkey_def *fk_def) +{ + uint64_t field_mask = 0; + for (uint32_t i = 0; i < fk_def->field_count; ++i) { + uint32_t parent_field = fk_def->links[i].parent_field; + if (parent_field > 63) + goto slow_check; + if (field_mask & (((uint64_t) 1) << parent_field)) + goto error; + column_mask_set_fieldno(&field_mask, parent_field); + } + return; +slow_check: + for (uint32_t i = 0; i < fk_def->field_count; ++i) { + uint32_t parent_field = fk_def->links[i].parent_field; + for (uint32_t j = i + 1; j < fk_def->field_count; ++j) { + if (parent_field == fk_def->links[j].parent_field) + goto error; + } + } + return; +error: + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, fk_def->name, + "referenced fields can not contain duplicates"); +} + +/** A trigger invoked on replace in the _fk_constraint space. */ +static void +on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) +{ + struct txn *txn = (struct txn *) event; + txn_check_singlestatement_xc(txn, "Space _fk_constraint"); + struct txn_stmt *stmt = txn_current_stmt(txn); + struct tuple *old_tuple = stmt->old_tuple; + struct tuple *new_tuple = stmt->new_tuple; + if (new_tuple != NULL) { + /* Create or replace foreign key. */ + struct fkey_def *fk_def = + fkey_def_new_from_tuple(new_tuple, + ER_CREATE_FK_CONSTRAINT); + auto fkey_def_guard = make_scoped_guard([=] { free(fk_def); }); + struct space *child_space = + space_cache_find_xc(fk_def->child_id); + if (child_space->def->opts.is_view) { + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + fk_def->name, + "referencing space can't be VIEW"); + } + struct space *parent_space = + space_cache_find_xc(fk_def->parent_id); + if (parent_space->def->opts.is_view) { + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + fk_def->name, + "referenced space can't be VIEW"); + } + /* + * FIXME: until SQL triggers are completely + * integrated into server (i.e. we are able to + * invoke triggers even if DML occurred via Lua + * interface), it makes no sense to provide any + * checks on existing data in space. + */ + struct index *pk = space_index(child_space, 0); + if (index_size(pk) > 0) { + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + fk_def->name, + "referencing space must be empty"); + } + /* Check types of referenced fields. */ + for (uint32_t i = 0; i < fk_def->field_count; ++i) { + uint32_t child_fieldno = fk_def->links[i].child_field; + uint32_t parent_fieldno = fk_def->links[i].parent_field; + if (child_fieldno >= child_space->def->field_count || + parent_fieldno >= parent_space->def->field_count) { + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + fk_def->name, "foreign key refers to " + "nonexistent field"); + } + struct field_def *child_field = + &child_space->def->fields[child_fieldno]; + struct field_def *parent_field = + &parent_space->def->fields[parent_fieldno]; + if (! field_type1_contains_type2(parent_field->type, + child_field->type)) { + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + fk_def->name, "field type mismatch"); + } + if (child_field->coll_id != parent_field->coll_id) { + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + fk_def->name, + "field collation mismatch"); + } + } + fkey_links_check_duplicates(fk_def); + /* + * Search for suitable index in parent space: + * it must be unique and consist exactly from + * referenced columns (but order may be + * different). + */ + struct index *fk_index = NULL; + for (uint32_t i = 0; i < parent_space->index_count; ++i) { + struct index *idx = space_index(parent_space, i); + if (!idx->def->opts.is_unique) + continue; + if (idx->def->key_def->part_count != + fk_def->field_count) + continue; + uint32_t j; + for (j = 0; j < fk_def->field_count; ++j) { + if (key_def_find(idx->def->key_def, + fk_def->links[j].parent_field) + == NULL) + break; + } + if (j != fk_def->field_count) + continue; + fk_index = idx; + break; + } + if (fk_index == NULL) { + tnt_raise(ClientError, ER_CREATE_FK_CONSTRAINT, + fk_def->name, "referenced fields don't " + "compose unique index"); + } + struct fkey *fkey = (struct fkey *) malloc(sizeof(*fkey)); + if (fkey == NULL) + tnt_raise(OutOfMemory, sizeof(*fkey), "malloc", "fkey"); + auto fkey_guard = make_scoped_guard([=] { free(fkey); }); + memset(fkey, 0, sizeof(*fkey)); + fkey->def = fk_def; + fkey->index_id = fk_index->def->iid; + if (old_tuple == NULL) { + rlist_add_entry(&child_space->child_fkey, fkey, + child_link); + rlist_add_entry(&parent_space->parent_fkey, fkey, + parent_link); + struct trigger *on_rollback = + txn_alter_trigger_new(on_create_fkey_rollback, + fkey); + txn_on_rollback(txn, on_rollback); + } else { + struct fkey *old_fk = + fkey_grab_by_name(&child_space->child_fkey, + fk_def->name); + rlist_add_entry(&child_space->child_fkey, fkey, + child_link); + rlist_add_entry(&parent_space->parent_fkey, fkey, + parent_link); + struct trigger *on_rollback = + txn_alter_trigger_new(on_replace_fkey_rollback, + fkey); + txn_on_rollback(txn, on_rollback); + struct trigger *on_commit = + txn_alter_trigger_new(on_drop_or_replace_fkey_commit, + old_fk); + txn_on_commit(txn, on_commit); + } + fkey_def_guard.is_active = false; + fkey_guard.is_active = false; + } else if (new_tuple == NULL && old_tuple != NULL) { + /* Drop foreign key. */ + struct fkey_def *fk_def = + fkey_def_new_from_tuple(old_tuple, + ER_DROP_FK_CONSTRAINT); + auto fkey_guard = make_scoped_guard([=] { free(fk_def); }); + struct space *child_space = + space_cache_find_xc(fk_def->child_id); + struct fkey *old_fkey = + fkey_grab_by_name(&child_space->child_fkey, + fk_def->name); + struct trigger *on_commit = + txn_alter_trigger_new(on_drop_or_replace_fkey_commit, + old_fkey); + txn_on_commit(txn, on_commit); + struct trigger *on_rollback = + txn_alter_trigger_new(on_drop_fkey_rollback, old_fkey); + txn_on_rollback(txn, on_rollback); + } +} + struct trigger alter_space_on_replace_space = { RLIST_LINK_INITIALIZER, on_replace_dd_space, NULL, NULL }; @@ -3523,4 +3968,8 @@ struct trigger on_replace_trigger = { RLIST_LINK_INITIALIZER, on_replace_dd_trigger, NULL, NULL }; +struct trigger on_replace_fk_constraint = { + RLIST_LINK_INITIALIZER, on_replace_dd_fk_constraint, NULL, NULL +}; + /* vim: set foldmethod=marker */ diff --git a/src/box/alter.h b/src/box/alter.h index 8ea29c77b..4108fa47c 100644 --- a/src/box/alter.h +++ b/src/box/alter.h @@ -45,6 +45,7 @@ extern struct trigger on_replace_sequence; extern struct trigger on_replace_sequence_data; extern struct trigger on_replace_space_sequence; extern struct trigger on_replace_trigger; +extern struct trigger on_replace_fk_constraint; extern struct trigger on_stmt_begin_space; extern struct trigger on_stmt_begin_index; extern struct trigger on_stmt_begin_truncate; diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap index a8a00ec29e7106afbd06294cf6ffe291f90a2e10..10f77f641b6308209ba01e6449bb22cc35963a9f 100644 GIT binary patch delta 1814 zcmV+x2kH2z4Vn&+8Gko6FfC^}WiT^1V=`hg3Q2BrbYX5|WjY{XGdMLeGBIK;GG%39 zEi_?cGc95_Ff=VNHDzUCVPj%qVPP~1RzqxWV{1AfdwmKD)w&D1%?6(U&U2<Mm8Adx z0000ewJ-euP_;Aw`bUTlNzfRn003Wngcn>16PR8tG{9G2A%B*nvW-`ySL`6KHf6VV z%amlLB-W*Se@Rl?3cz(5$a~6rR6)dCAE{j~8m?KWsS!9-rj!EW0N4QR0HXn2ouqhO zG~UIq23{P3HZj)s>z}avUb8aSBks#hfP{3I`vt)e+adZQpsSOl@}TO1;HWLnL_dP} zRmSwCk+CmAdw<u}HI~l1bK8rtUIUUDnXvA9S9x9E!rb1d3oTOOjRJIalCgbVd5gDJ zXX)Ra<Bnu(hbn-sPSOaI94TDT=zk&3dc08~@1^VNBqyyp%zXdrmZYnb+(L`KY+=`B zqg|aO4uNx8;goPzs?;T^N;fEy3H6;wClL6ORH~d*DSwq?!jNTu=Oc)3GS7w1X~hL- z1UI4uhULmm6##Vt1^r?@?rw9m2SQgTi9^C-<=rig@;vD3BuAOWQJt4p-eSr@S10+b z2lFd$F=sn|BC3$+KkWK$*6o$IXiJJ}Fx$^Z6N9c!vem~s+s{W^(vnLOtjE>M+Gx#! zu1<37&wrC(J+5p?X%q-~3HBAsOcR3rr?a)u83apSc8JOx==ZZvC<Oyuous}GhM@|? zv2Wpv%f4^tCG6_#qU~pW*ZC%X=d~sJN1x|5M`H^7e17p5zrE~@!VtK7-}1V4xU#V> zxGK9ILu^0r^NL7kmVTR($5-UmMgu%4s16F9oqv>0>O{(f%vMn)CW=H?C)vr6phlQP zm{KNGDMSfGDW~RCgA-J9Lt~;Tb#;<KG#@-0nMa!OAe-qR!$GD-hNi`Di^7H&4KW#F zFu+`Zu>j-MI+~%fpeb}zVI)K83x%#u@+D@<f~frHll5pj^1pj+y3isaZ)QSQCrJc` zcz@`A;%If4>ks~Cb(m}6eqH6>zxvPeOr(EbG53MFg_m@l@9tcO*cMsYC-jrh)k*FP zUM#vUw|$_d`MjnHor1u0UW0v^2C6#E)Nkh&^x<}h0vUbZ?|%I(%d-<AC=^G8u1-=G zr+ZZ@eD8lwiF7K3P?2!(J(2GJ4M}CXxqrz);am(^sLa#36^2l&(+R0PK~g|WKWcn< zJ!E)Lm>pf6WPPDUWz|7-m^;?v3RmgOL*>}AWs4PxqpOn~&FA}HS&z6IU7aL=I_b<q z+1M|RyrdOs;0%htiD*aT!Eo1b234bIqjlz$X6WqtZb_+(u1>N9Yt<5C9Nc-q0)Ka? z;p2Z|GI~jT@$;|NON!~XI7z%MoIA3gkG0N!WR-Nzyi$y=PI4y)3>pnrI@|2$qm2zL zR7Ih5=Fz~Q-;1v&fDB0x7cw9`INAaSi=GvfM@4~PsPRx_5E_^P0003{0PzI}Dd&m? z5`eHkilZ=&ffxuw5R3v7XMg|{gntAYpjt4r-^d`k=SQL>dmg;ae~0m7<UBfHoSn_K zW}>9xd#qxM5Nd0_7-fOLf-Hg|p+A-mn55wKvv;8EU;I(IcxrT)6(wzmc{T;w?qGdP zTaN0>nGc)5E7^iUAvGRX<z<nP8QJ3izfE%3nmK&QVXZ0lU%+mxCy?xH=zm0L;DbzK zBdUr6k|?W07-Xp$x~_fNssjNe_k>Nnjt#DUWdUCxdJ1$^m;2T_PG&hZj}9ZMcJ0s+ z$6(bU-x!;Xoq7mcLzq=5m17&AOY6yY7zyl{10)9wC&0O|Bsrn2+@PwWly^gmxMmNM zs3B#;9|#+_><z<yiEs}n&40miuWb44r&<cqrb!DT`c-HIcpuL`H;nohiGoY;;nU9W zV;%@VnAP%2cs#=GmV4zDDJz{(vo>RCT+@$wC=lv6OKH}l%!TV(@N}#u6<JC@$HH^M zp;+DKXuIt12KGYIthD(ob9r2vkYYpJ*}-RH$GW4N=W;MX!aws;MSmw_eEgXk`N1SY z&qJ4AlRq^uNg{TN@P#i@A5&R(9n>*Gp$x8Oj4Lg+G29w6yJ1tg<_I|l6c&ky|77<U zY$1I9Lds&0&|UhJwv}mR4IL#~J}put_$baD;OJFrvqO!6zFFFnZZlw<>EbPAaK))4 z7C?-~nY69poEiC?9)EiVG<UY#L2u5Gv%PRU$zpI8LxBPFia(fw&lmMK%EE&bwsSjS zrD$U0)2b+krqr2iqm5q4X)gn}$*Wh_|6fkXM-Hz@{-l?15*4WSCy9Tp-j(_zU32|m zA031OL(7uk=2!kyUv+%KuxwUl2W4GN9eS#ye{j~d<WR!`t`8&9@KvOqBn43p)ex=i E3amY33;+NC delta 1699 zcmV;U23+}?4yX-~8Gki5FfC^@G-fw8H#rJPZgX^DZewLSAYx=OVKg#1V=Xo^Ff=VR zH#adYIA$<6Eiz^~VK^`|F)=qdI0{xnY;R+0Iv{&}3JTS_3%bn)FaXY~@A2EE00000 z04TLD{QyusF#sw<untMk7&id`Up&AM2ap_8K2ct%)hPuc5r2u%1Dtdthk@FZnGtQL zq{+<`lO*YB%N4E_fREI4wd=GkVz`4fdcG}`x2rRP>+&h36w?690OJ7SjC}R|`e!^< zufxoBSySaEK}34Y9gE@!en$00z_rw*@?r{$VyQo#kG>SK>wIY(Gha_6_ARVsq0YPW z*^{p>3xXA)K!3gUtrNGN1@ezpT`H224JN>~)O>B+%A2IIdZ>SQUNe%x&&UF}mYPMF z<VfKnR$n!OS(mg*w7uMGsY$EVW9Iu^v?SM3GpQn}@&|-lvwAHxzsossplA9N4p%a9 zo>%g6a!e(V!@(sGEAUSy(+NYC{hgO0NP}%ImC`FNL4Q!RY*}%n3YHx!04xm(FRr{v zl!I%j`QeM%l{Z=Rb?}Fz#-snSYrI36D{t~UtJh#?KQB85*HV+!OFXonm)}`Amn5Jr zSFUeXZ5CWh&9XmF0_t-0owY^*(UxFi^2{+JpjYYq%_<FoC9cn?&K&4i^*yN+3|vc1 zjb9wclz#}mo&{p?>>GD}0%4t7vHjt9ooVEEE?c6%^m+ajRha@mpI!XLX|9h}V+i2A z@rhe|T+Ko(uFkH@5&Tr*=hczUq5heY2QkUNS%n#JEj3Bpit$d4G7*DgL3m(5TGT2F z94mTMRGt((D5}nmjt=Ss$^@zeY-T+Y)EjXvHGeM*i6^2%bzXPkLU15BlTD5dj*v}P z!_|DamYPB}H(Lp|d7$Y4qv0@PGgC9OfyFV4f(DrkG8kko#8`;gVrZ;|sZe1;Q!301 z424=>WUi&=%LED&qVlCr*5%B||L!vCQjv%@8VT1@b7K*MWDE0|O4Ko**D)efkeJS8 zK!2(n6H`6bb=>(U`tZ-F0)bI|$NTk%AJ0yXpi~?YuB9fdV^Ej7e?_$i!nM?_Aqm60 z`&X+x53Z$V3HxeQC!Tlex*X2Ks1;7>#I3wicRHC$CJrY=2}I$TaHmRP$WmdOj*tpL zDiEFf!}j<<_2_um?AUb7<ft(?uBE1ZseeePy;1d;JE+T5&DNpKvyqgPbfwm~mYOj1 z`F>Z{WsSzQ)C6E@9ooDZ`^l1@wSo*Vi{$U4dSkpeZW)-xR7uXN9oje;I=58#to6mU z)GQIMSmcX~J1<y(9cuXapO=izkuQGU#d1mMFpHN+%mRyy?B}Jd^Do1*(xHv(xqp_L zJ2_y)>bTPRW<M`)Zh*#Alv;<j8W#azNCJUKsNq;-5DClx0Du4}0PzI}Dd&m?5`eHc zio-CDff$BE0E|KtM?fGb2njSmwO}-3>|otvVxbn$nEy|s!{{(Nj1CZ|XWOmW^1O_Q z#C9R1B6~4P0)Yit2t#sze0O3}a)0*OYw7YYtf)91`JUH`Pa9%0YaYejkXxhcI3*z3 zdE&uuxjDLwcwchr;ms`(kQeo&CcC+u2o3ol@WqTwal~b%ln5QO6oL_ZE4HeMEh(My z_v_fe^><ak7er6BFLY%562{3Rr^e{WMg>zAXfXbi)iZOl46U*mTj66G#((TQBk0n5 z@)<oY4g-ONQth~(3rms{8<`ta`P_f=wD_@mkOU1mHT1wuY%u<m)iZOl3{AjWf3|$~ zt8m|QXkeD0hZ@Msd-hKiZ0KL;Q@khOrOoi`UOgQ>poLS)q+!uN;jpegcC;P#`N7`U znw9D$>kp61-&FJnXm;?~*ng4k=;HZ8FhRgSHd00X7<B%WiF{!a-SZ&o*D9aNO_EGI zB{7vRtd9oyxDE<2a=|>f+87shpvG{^Ol^j3&MJS%IeJ)8Cti{*Ub%&E>I-oWh=lIa zrQD!Ql+Ehs<+5HPA%914?C?i#t&Krytl`a4o=ls7UrZNkEfZHnq<_5tDm2be+j_v6 zk)6|HU_kR{(H-|;3^`j1=aVc3C;Vs%FuC|kE%^FTe<LpZkFbK<maIKZW_(1&;Gu~+ zlWeroE4jI4;5K>1vid)s6X20|EK)(~T|9{xS1W;}zgF#f{gJ7;9%W`;4+U&31%|V| tLZAw@V?4uhT=^Z8O&m3K=&6$aL0H$8dkzbvjtIq9$$FA7L^;(Et?i$*Eo}e* diff --git a/src/box/errcode.h b/src/box/errcode.h index b61b387f2..213a1864b 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -219,6 +219,8 @@ struct errcode_record { /*164 */_(ER_NO_SUCH_GROUP, "Replication group '%s' does not exist") \ /*165 */_(ER_NO_SUCH_MODULE, "Module '%s' does not exist") \ /*166 */_(ER_NO_SUCH_COLLATION, "Collation '%s' does not exist") \ + /*167 */_(ER_CREATE_FK_CONSTRAINT, "Failed to create foreign key constraint '%s': %s") \ + /*168 */_(ER_DROP_FK_CONSTRAINT, "Failed to drop foreign key constraint '%s': %s") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/fkey.c b/src/box/fkey.c new file mode 100644 index 000000000..0bdccb521 --- /dev/null +++ b/src/box/fkey.c @@ -0,0 +1,56 @@ +/* + * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ +#include "fkey.h" +#include "sql.h" +#include "sql/sqliteInt.h" + +const char *fkey_action_strs[] = { + /* [FKEY_ACTION_RESTRICT] = */ "no_action", + /* [FKEY_ACTION_SET_NULL] = */ "set_null", + /* [FKEY_ACTION_SET_DEFAULT] = */ "set_default", + /* [FKEY_ACTION_CASCADE] = */ "cascade", + /* [FKEY_ACTION_NO_ACTION] = */ "restrict" +}; + +const char *fkey_match_strs[] = { + /* [FKEY_MATCH_SIMPLE] = */ "simple", + /* [FKEY_MATCH_PARTIAL] = */ "partial", + /* [FKEY_MATCH_FULL] = */ "full" +}; + +void +fkey_delete(struct fkey *fkey) +{ + sql_trigger_delete(sql_get(), fkey->on_delete_trigger); + sql_trigger_delete(sql_get(), fkey->on_update_trigger); + free(fkey->def); + free(fkey); +} diff --git a/src/box/fkey.h b/src/box/fkey.h new file mode 100644 index 000000000..ed99617ca --- /dev/null +++ b/src/box/fkey.h @@ -0,0 +1,138 @@ +#ifndef TARANTOOL_BOX_FKEY_H_INCLUDED +#define TARANTOOL_BOX_FKEY_H_INCLUDED +/* + * Copyright 2010-2018, Tarantool AUTHORS, please see AUTHORS file. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * 1. Redistributions of source code must retain the above + * copyright notice, this list of conditions and the + * following disclaimer. + * + * 2. Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials + * provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL + * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR + * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF + * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF + * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ +#include <stdbool.h> +#include <stdint.h> + +#include "space.h" + +#if defined(__cplusplus) +extern "C" { +#endif /* defined(__cplusplus) */ + +struct sqlite3; + +enum fkey_action { + FKEY_NO_ACTION = 0, + FKEY_ACTION_SET_NULL, + FKEY_ACTION_SET_DEFAULT, + FKEY_ACTION_CASCADE, + FKEY_ACTION_RESTRICT, + fkey_action_MAX +}; + +enum fkey_match { + FKEY_MATCH_SIMPLE = 0, + FKEY_MATCH_PARTIAL, + FKEY_MATCH_FULL, + fkey_match_MAX +}; + +extern const char *fkey_action_strs[]; + +extern const char *fkey_match_strs[]; + +/** Structure describing field dependencies for foreign keys. */ +struct field_link { + uint32_t parent_field; + uint32_t child_field; +}; + +/** Definition of foreign key constraint. */ +struct fkey_def { + /** Id of space containing the REFERENCES clause (child). */ + uint32_t child_id; + /** Id of space that the key points to (parent). */ + uint32_t parent_id; + /** Number of fields in this key. */ + uint32_t field_count; + /** True if constraint checking is deferred till COMMIT. */ + bool is_deferred; + /** Match condition for foreign key. SIMPLE by default. */ + enum fkey_match match; + /** ON DELETE action. NO ACTION by default. */ + enum fkey_action on_delete; + /** ON UPDATE action. NO ACTION by default. */ + enum fkey_action on_update; + /** Mapping of fields in child to fields in parent. */ + struct field_link *links; + /** Name of the constraint. */ + char name[0]; +}; + +/** Structure representing foreign key relationship. */ +struct fkey { + struct fkey_def *def; + /** Index id of referenced index in parent space. */ + uint32_t index_id; + /** Triggers for actions. */ + struct sql_trigger *on_delete_trigger; + struct sql_trigger *on_update_trigger; + /** Links for parent and child lists. */ + struct rlist parent_link; + struct rlist child_link; +}; + +/** + * Alongside with struct fkey_def itself, we reserve memory for + * string containing its name and for array of links. + * Memory layout: + * +-------------------------+ <- Allocated memory starts here + * | struct fkey_def | + * |-------------------------| + * | name + \0 | + * |-------------------------| + * | links | + * +-------------------------+ + */ +static inline size_t +fkey_def_sizeof(uint32_t links_count, uint32_t name_len) +{ + return sizeof(struct fkey) + links_count * sizeof(struct field_link) + + name_len + 1; +} + +static inline bool +fkey_is_self_referenced(const struct fkey_def *fkey) +{ + return fkey->child_id == fkey->parent_id; +} + +/** Release memory for foreign key and its triggers, if any. */ +void +fkey_delete(struct fkey *fkey); + +#if defined(__cplusplus) +} /* extern "C" */ +#endif /* __cplusplus */ + +#endif /* TARANTOOL_BOX_FKEY_H_INCLUDED */ diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index d14dd748b..b73d9ab78 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -508,6 +508,7 @@ box.schema.space.drop = function(space_id, space_name, opts) local _vindex = box.space[box.schema.VINDEX_ID] local _truncate = box.space[box.schema.TRUNCATE_ID] local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID] + local _fk_constraint = box.space[box.schema.FK_CONSTRAINT_ID] local sequence_tuple = _space_sequence:delete{space_id} if sequence_tuple ~= nil and sequence_tuple[3] == true then -- Delete automatically generated sequence. @@ -521,6 +522,9 @@ box.schema.space.drop = function(space_id, space_name, opts) local v = keys[i] _index:delete{v[1], v[2]} end + for _, t in _fk_constraint.index.child_id:pairs({space_id}) do + _fk_constraint:delete({t.name, space_id}) + end revoke_object_privs('space', space_id) _truncate:delete{space_id} if _space:delete{space_id} == nil then diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index ca3fefc0d..d07560d6c 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -551,6 +551,8 @@ box_lua_space_init(struct lua_State *L) lua_setfield(L, -2, "SQL_STAT1_ID"); lua_pushnumber(L, BOX_SQL_STAT4_ID); lua_setfield(L, -2, "SQL_STAT4_ID"); + lua_pushnumber(L, BOX_FK_CONSTRAINT_ID); + lua_setfield(L, -2, "FK_CONSTRAINT_ID"); lua_pushnumber(L, BOX_TRUNCATE_ID); lua_setfield(L, -2, "TRUNCATE_ID"); lua_pushnumber(L, BOX_SEQUENCE_ID); diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua index f112a93ae..8a30e9f7d 100644 --- a/src/box/lua/upgrade.lua +++ b/src/box/lua/upgrade.lua @@ -509,6 +509,27 @@ local function upgrade_to_2_1_0() {unique = true}, {{0, 'string'}, {1, 'string'}, {5, 'scalar'}}} + local fk_constr_ft = {{name='name', type='string'}, + {name='child_id', type='unsigned'}, + {name='parent_id', type='unsigned'}, + {name='is_deferred', type='boolean'}, + {name='match', type='string'}, + {name='on_delete', type='string'}, + {name='on_update', type='string'}, + {name='child_cols', type='array'}, + {name='parent_cols', type='array'}} + log.info("create space _fk_constraint") + _space:insert{box.schema.FK_CONSTRAINT_ID, ADMIN, '_fk_constraint', 'memtx', + 0, setmap({}), fk_constr_ft} + + log.info("create index primary on _fk_constraint") + _index:insert{box.schema.FK_CONSTRAINT_ID, 0, 'primary', 'tree', + {unique = true}, {{0, 'string'}, {1, 'unsigned'}}} + + log.info("create secondary index child_id on _fk_constraint") + _index:insert{box.schema.FK_CONSTRAINT_ID, 1, 'child_id', 'tree', + {unique = false}, {{1, 'unsigned'}}} + -- Nullability wasn't skipable. This was fixed in 1-7. -- Now, abscent field means NULL, so we can safely set second -- field in format, marking it nullable. diff --git a/src/box/schema.cc b/src/box/schema.cc index 86c56ee2e..faad53700 100644 --- a/src/box/schema.cc +++ b/src/box/schema.cc @@ -412,6 +412,22 @@ schema_init() COLL_NONE, SORT_ORDER_ASC); /* _sql_stat4 - extensive statistics on space, seen in SQL. */ sc_space_new(BOX_SQL_STAT4_ID, "_sql_stat4", key_def, NULL, NULL); + + key_def_delete(key_def); + key_def = key_def_new(2); + if (key_def == NULL) + diag_raise(); + /* Constraint name. */ + key_def_set_part(key_def, 0, BOX_FK_CONSTRAINT_FIELD_NAME, + FIELD_TYPE_STRING, ON_CONFLICT_ACTION_ABORT, NULL, + COLL_NONE, SORT_ORDER_ASC); + /* Child space. */ + key_def_set_part(key_def, 1, BOX_FK_CONSTRAINT_FIELD_CHILD_ID, + FIELD_TYPE_UNSIGNED, ON_CONFLICT_ACTION_ABORT, NULL, + COLL_NONE, SORT_ORDER_ASC); + /* _fk_сonstraint - foreign keys constraints. */ + sc_space_new(BOX_FK_CONSTRAINT_ID, "_fk_constraint", key_def, + &on_replace_fk_constraint, NULL); } void diff --git a/src/box/schema_def.h b/src/box/schema_def.h index 5ab4bb002..fd57d22b9 100644 --- a/src/box/schema_def.h +++ b/src/box/schema_def.h @@ -107,6 +107,8 @@ enum { /** Space ids for SQL statictics. */ BOX_SQL_STAT1_ID = 348, BOX_SQL_STAT4_ID = 349, + /** Space id of _fk_constraint. */ + BOX_FK_CONSTRAINT_ID = 356, /** End of the reserved range of system spaces. */ BOX_SYSTEM_ID_MAX = 511, BOX_ID_NIL = 2147483647 @@ -224,6 +226,19 @@ enum { BOX_TRIGGER_FIELD_OPTS = 2, }; +/** _fk_constraint fields. */ +enum { + BOX_FK_CONSTRAINT_FIELD_NAME = 0, + BOX_FK_CONSTRAINT_FIELD_CHILD_ID = 1, + BOX_FK_CONSTRAINT_FIELD_PARENT_ID = 2, + BOX_FK_CONSTRAINT_FIELD_DEFERRED = 3, + BOX_FK_CONSTRAINT_FIELD_MATCH = 4, + BOX_FK_CONSTRAINT_FIELD_ON_DELETE = 5, + BOX_FK_CONSTRAINT_FIELD_ON_UPDATE = 6, + BOX_FK_CONSTRAINT_FIELD_CHILD_COLS = 7, + BOX_FK_CONSTRAINT_FIELD_PARENT_COLS = 8, +}; + /* * Different objects which can be subject to access * control. diff --git a/src/box/space.c b/src/box/space.c index e53f1598c..90a80ed7b 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -163,6 +163,8 @@ space_create(struct space *space, struct engine *engine, space->index_map[index_def->iid] = index; } space_fill_index_map(space); + rlist_create(&space->parent_fkey); + rlist_create(&space->child_fkey); return 0; fail_free_indexes: @@ -220,6 +222,8 @@ space_delete(struct space *space) * on_replace_dd_trigger on deletion from _trigger. */ assert(space->sql_triggers == NULL); + assert(rlist_empty(&space->parent_fkey)); + assert(rlist_empty(&space->child_fkey)); space->vtab->destroy(space); } diff --git a/src/box/space.h b/src/box/space.h index 01a4af726..d60ba6c56 100644 --- a/src/box/space.h +++ b/src/box/space.h @@ -183,6 +183,15 @@ struct space { * of index id. */ struct index **index; + /** + * Lists of foreign key constraints. In SQL terms child + * space is the "from" table i.e. the table that contains + * the REFERENCES clause. Parent space is "to" table, in + * other words the table that is named in the REFERENCES + * clause. + */ + struct rlist parent_fkey; + struct rlist child_fkey; }; /** Initialize a base space instance. */ diff --git a/src/box/sql.c b/src/box/sql.c index d48c3cfe5..9795ad2ac 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -1251,6 +1251,14 @@ void tarantoolSqlite3LoadSchema(struct init_data *init) "\"sample\"," "PRIMARY KEY(\"tbl\", \"idx\", \"sample\"))"); + sql_init_callback(init, TARANTOOL_SYS_FK_CONSTRAINT_NAME, + BOX_FK_CONSTRAINT_ID, 0, + "CREATE TABLE \""TARANTOOL_SYS_FK_CONSTRAINT_NAME + "\"(\"name\" TEXT, \"parent_id\" INT, \"child_id\" INT," + "\"deferred\" INT, \"match\" TEXT, \"on_delete\" TEXT," + "\"on_update\" TEXT, \"child_cols\", \"parent_cols\"," + "PRIMARY KEY(\"name\", \"child_id\"))"); + /* Read _space */ if (space_foreach(space_foreach_put_cb, init) != 0) { init->rc = SQL_TARANTOOL_ERROR; diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c index 222405a1f..c941b6e58 100644 --- a/src/box/sql/fkey.c +++ b/src/box/sql/fkey.c @@ -35,6 +35,7 @@ */ #include "coll.h" #include "sqliteInt.h" +#include "box/fkey.h" #include "box/schema.h" #include "box/session.h" #include "tarantoolInt.h" @@ -707,31 +708,6 @@ sqlite3FkReferences(Table * pTab) pTab->def->name); } -/** - * The second argument is a Trigger structure allocated by the - * fkActionTrigger() routine. This function deletes the sql_trigger - * structure and all of its sub-components. - * - * The Trigger structure or any of its sub-components may be - * allocated from the lookaside buffer belonging to database - * handle dbMem. - * - * @param db Database connection. - * @param trigger AST object. - */ -static void -sql_fk_trigger_delete(struct sqlite3 *db, struct sql_trigger *trigger) -{ - if (trigger == NULL) - return; - struct TriggerStep *trigger_step = trigger->step_list; - sql_expr_delete(db, trigger_step->pWhere, false); - sql_expr_list_delete(db, trigger_step->pExprList); - sql_select_delete(db, trigger_step->pSelect); - sql_expr_delete(db, trigger->pWhen, false); - sqlite3DbFree(db, trigger); -} - /* * The second argument points to an FKey object representing a foreign key * for which pTab is the child table. An UPDATE statement against pTab @@ -1275,20 +1251,14 @@ fkActionTrigger(struct Parse *pParse, struct Table *pTab, struct FKey *pFKey, pWhere, 0, 0, 0, 0, 0, 0); pWhere = 0; } - - /* Disable lookaside memory allocation */ - db->lookaside.bDisable++; - - size_t trigger_size = sizeof(struct sql_trigger) + - sizeof(TriggerStep) + nFrom + 1; - trigger = - (struct sql_trigger *)sqlite3DbMallocZero(db, - trigger_size); + trigger = (struct sql_trigger *)sqlite3DbMallocZero(db, + sizeof(*trigger)); if (trigger != NULL) { - pStep = trigger->step_list = (TriggerStep *)&trigger[1]; + size_t step_size = sizeof(TriggerStep) + nFrom + 1; + trigger->step_list = sqlite3DbMallocZero(db, step_size); + pStep = trigger->step_list; pStep->zTarget = (char *)&pStep[1]; - memcpy((char *)pStep->zTarget, zFrom, nFrom); - + memcpy(pStep->zTarget, zFrom, nFrom); pStep->pWhere = sqlite3ExprDup(db, pWhere, EXPRDUP_REDUCE); pStep->pExprList = @@ -1302,15 +1272,12 @@ fkActionTrigger(struct Parse *pParse, struct Table *pTab, struct FKey *pFKey, } } - /* Re-enable the lookaside buffer, if it was disabled earlier. */ - db->lookaside.bDisable--; - sql_expr_delete(db, pWhere, false); sql_expr_delete(db, pWhen, false); sql_expr_list_delete(db, pList); sql_select_delete(db, pSelect); if (db->mallocFailed == 1) { - sql_fk_trigger_delete(db, trigger); + sql_trigger_delete(db, trigger); return 0; } assert(pStep != 0); @@ -1408,8 +1375,8 @@ sqlite3FkDelete(sqlite3 * db, Table * pTab) assert(pFKey->isDeferred == 0 || pFKey->isDeferred == 1); /* Delete any triggers created to implement actions for this FK. */ - sql_fk_trigger_delete(db, pFKey->apTrigger[0]); - sql_fk_trigger_delete(db, pFKey->apTrigger[1]); + sql_trigger_delete(db, pFKey->apTrigger[0]); + sql_trigger_delete(db, pFKey->apTrigger[1]); pNext = pFKey->pNextFrom; sqlite3DbFree(db, pFKey); diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h index e1430a398..bc61e8426 100644 --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -20,6 +20,7 @@ #define TARANTOOL_SYS_TRUNCATE_NAME "_truncate" #define TARANTOOL_SYS_SQL_STAT1_NAME "_sql_stat1" #define TARANTOOL_SYS_SQL_STAT4_NAME "_sql_stat4" +#define TARANTOOL_SYS_FK_CONSTRAINT_NAME "_fk_constraint" /* Max space id seen so far. */ #define TARANTOOL_SYS_SCHEMA_MAXID_KEY "max_id" diff --git a/test/box/access_misc.result b/test/box/access_misc.result index 53af3e8bd..3ba33ee97 100644 --- a/test/box/access_misc.result +++ b/test/box/access_misc.result @@ -815,6 +815,11 @@ box.space._space:select() - [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx', 'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'}, {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]] + - [356, 1, '_fk_constraint', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'}, + {'name': 'child_id', 'type': 'unsigned'}, {'name': 'parent_id', 'type': 'unsigned'}, + {'name': 'is_deferred', 'type': 'boolean'}, {'name': 'match', 'type': 'string'}, + {'name': 'on_delete', 'type': 'string'}, {'name': 'on_update', 'type': 'string'}, + {'name': 'child_cols', 'type': 'array'}, {'name': 'parent_cols', 'type': 'array'}]] ... box.space._func:select() --- diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result index ae042664a..77a24b425 100644 --- a/test/box/access_sysview.result +++ b/test/box/access_sysview.result @@ -230,11 +230,11 @@ box.session.su('guest') ... #box.space._vspace:select{} --- -- 22 +- 23 ... #box.space._vindex:select{} --- -- 48 +- 50 ... #box.space._vuser:select{} --- @@ -262,7 +262,7 @@ box.session.su('guest') ... #box.space._vindex:select{} --- -- 48 +- 50 ... #box.space._vuser:select{} --- diff --git a/test/box/alter.result b/test/box/alter.result index c41b52f48..0d50855d2 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -107,7 +107,7 @@ space = box.space[t[1]] ... space.id --- -- 350 +- 357 ... space.field_count --- @@ -152,7 +152,7 @@ space_deleted ... space:replace{0} --- -- error: Space '350' does not exist +- error: Space '357' does not exist ... _index:insert{_space.id, 0, 'primary', 'tree', {unique=true}, {{0, 'unsigned'}}} --- @@ -231,6 +231,8 @@ _index:select{} - [348, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string']]] - [349, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string'], [ 5, 'scalar']]] + - [356, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]] + - [356, 1, 'child_id', 'tree', {'unique': false}, [[1, 'unsigned']]] ... -- modify indexes of a system space _index:delete{_index.id, 0} diff --git a/test/box/misc.result b/test/box/misc.result index 892851823..a680f752e 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -491,6 +491,8 @@ t; 164: box.error.NO_SUCH_GROUP 165: box.error.NO_SUCH_MODULE 166: box.error.NO_SUCH_COLLATION + 167: box.error.CREATE_FK_CONSTRAINT + 168: box.error.DROP_FK_CONSTRAINT ... test_run:cmd("setopt delimiter ''"); --- diff --git a/test/engine/iterator.result b/test/engine/iterator.result index 98b0b3e7d..6e03fbbbd 100644 --- a/test/engine/iterator.result +++ b/test/engine/iterator.result @@ -4211,9 +4211,15 @@ s:replace{35} --- - [35] ... -state, value = gen(param,state) +f = function() return gen(param, state) end +--- +... +_, errmsg = pcall(f) +--- +... +errmsg:match('usage: next%(param, state%)') --- -- error: 'builtin/box/schema.lua:1051: usage: next(param, state)' +- 'usage: next(param, state)' ... value --- diff --git a/test/engine/iterator.test.lua b/test/engine/iterator.test.lua index fcf753f46..9ff51ebe1 100644 --- a/test/engine/iterator.test.lua +++ b/test/engine/iterator.test.lua @@ -400,7 +400,9 @@ gen,param,state = i:pairs({35}) state, value = gen(param,state) value s:replace{35} -state, value = gen(param,state) +f = function() return gen(param, state) end +_, errmsg = pcall(f) +errmsg:match('usage: next%(param, state%)') value s:drop() diff --git a/test/sql/foreign-keys.result b/test/sql/foreign-keys.result new file mode 100644 index 000000000..c2ec429c3 --- /dev/null +++ b/test/sql/foreign-keys.result @@ -0,0 +1,336 @@ +env = require('test_run') +--- +... +test_run = env.new() +--- +... +test_run:cmd('restart server default with cleanup=1') +-- Check that tuple inserted into _fk_constraint is FK constrains +-- valid data. +-- +box.sql.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a INT, b INT);") +--- +... +box.sql.execute("CREATE UNIQUE INDEX i1 ON t1(a);") +--- +... +box.sql.execute("CREATE TABLE t2 (a INT, b INT, id INT PRIMARY KEY);") +--- +... +box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;") +--- +... +-- Parent and child spaces must exist. +-- +t = {'fk_1', 666, 777, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: Space '666' does not exist +... +parent_id = box.space._space.index.name:select('T1')[1]['id'] +--- +... +child_id = box.space._space.index.name:select('T2')[1]['id'] +--- +... +view_id = box.space._space.index.name:select('V1')[1]['id'] +--- +... +-- View can't reference another space or be referenced by another space. +-- +t = {'fk_1', child_id, view_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': referenced space can''t + be VIEW' +... +t = {'fk_1', view_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': referencing space can''t + be VIEW' +... +box.sql.execute("DROP VIEW v1;") +--- +... +-- Match clause can be only one of: simple, partial, full. +-- +t = {'fk_1', child_id, parent_id, false, 'wrong_match', 'restrict', 'restrict', {0}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': unknown MATCH clause' +... +-- On conflict actions can be only one of: set_null, set_default, +-- restrict, cascade, no_action. +t = {'fk_1', child_id, parent_id, false, 'simple', 'wrong_action', 'restrict', {0}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': unknown ON DELETE action' +... +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'wrong_action', {0}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': unknown ON UPDATE action' +... +-- Temporary restriction (until SQL triggers work from Lua): +-- referencing space must be empty. +-- +box.sql.execute("INSERT INTO t2 VALUES (1, 2, 3);") +--- +... +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {2}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': referencing space must + be empty' +... +box.sql.execute("DELETE FROM t2;") +--- +... +-- Links must be specififed correctly. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {}, {}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': at least one link must + be specified' +... +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {2}, {1,2}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': number of referenced and + referencing fields must be the same' +... +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {13}, {1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': foreign key refers to + nonexistent field' +... +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {'crash'}, {'crash'}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': value of 0 link is not + unsigned' +... +-- Referenced fields must compose unique index. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0, 1}, {1, 2}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': referenced fields don''t + compose unique index' +... +-- Referencing and referenced fields must feature compatible types. +-- Temporary, in SQL all fields except for INTEGER PRIMARY KEY +-- are scalar. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {1}, {0}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': field type mismatch' +... +-- Each referenced column must appear once. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0, 1}, {1, 1}} +--- +... +box.space._fk_constraint:insert(t) +--- +- error: 'Failed to create foreign key constraint ''fk_1'': referenced fields can + not contain duplicates' +... +-- Successful creation. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +-- Implicitly referenced index can't be dropped, +-- ergo - space can't be dropped until it is referenced. +-- +box.sql.execute("DROP INDEX i1 on t1;") +--- +- error: 'Can''t modify space ''T1'': can not drop referenced index' +... +-- Referenced index can't be altered as well, if alter leads to +-- rebuild of index (e.g. index still can be renamed). +box.space._index:replace{512, 1, 'I1', 'tree', {unique = true}, {{field = 0, type = 'unsigned', is_nullable = true}}} +--- +- error: 'Can''t modify space ''T1'': can not alter referenced index' +... +box.space._index:replace{512, 1, 'I2', 'tree', {unique = true}, {{field = 1, type = 'unsigned', is_nullable = true}}} +--- +- [512, 1, 'I2', 'tree', {'unique': true}, [{'field': 1, 'type': 'unsigned', 'is_nullable': true}]] +... +-- Finally, can't drop space until it has FK constraints, +-- i.e. by manual removing tuple from _space. +-- But drop() will delete constraints. +-- +box.space.T2.index[0]:drop() +--- +... +box.space._space:delete(child_id) +--- +- error: 'Can''t drop space ''T2'': the space has foreign key constraints' +... +box.space.T2:drop() +--- +... +-- Make sure that constraint has been successfully dropped, +-- so we can drop now and parent space. +-- +box.space._fk_constraint:select() +--- +- [] +... +box.space.T1:drop() +--- +... +-- Create several constraints to make sure that they are held +-- as linked lists correctly including self-referencing constraints. +-- +box.sql.execute("CREATE TABLE child (id INT PRIMARY KEY, a INT);") +--- +... +box.sql.execute("CREATE TABLE parent (a INT, id INT PRIMARY KEY);") +--- +... +parent_id = box.space._space.index.name:select('PARENT')[1]['id'] +--- +... +child_id = box.space._space.index.name:select('CHILD')[1]['id'] +--- +... +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +t = {'fk_2', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +t = {'fk_3', parent_id, child_id, false, 'simple', 'restrict', 'restrict', {1}, {0}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +t = {'self_1', child_id, child_id, false, 'simple', 'restrict', 'restrict', {0}, {0}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +t = {'self_2', parent_id, parent_id, false, 'simple', 'restrict', 'restrict', {1}, {1}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +box.space._fk_constraint:count() +--- +- 5 +... +box.space._fk_constraint:delete{'fk_2', child_id} +--- +- ['fk_2', 515, 516, false, 'simple', 'restrict', 'restrict', [0], [1]] +... +box.space._fk_constraint:delete{'fk_1', child_id} +--- +- ['fk_1', 515, 516, false, 'simple', 'restrict', 'restrict', [0], [1]] +... +t = {'fk_2', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +box.space._fk_constraint:delete{'fk_2', child_id} +--- +- ['fk_2', 515, 516, false, 'simple', 'restrict', 'restrict', [0], [1]] +... +box.space._fk_constraint:delete{'self_2', parent_id} +--- +- ['self_2', 516, 516, false, 'simple', 'restrict', 'restrict', [1], [1]] +... +box.space._fk_constraint:delete{'self_1', child_id} +--- +- ['self_1', 515, 515, false, 'simple', 'restrict', 'restrict', [0], [0]] +... +box.space._fk_constraint:delete{'fk_3', parent_id} +--- +- ['fk_3', 516, 515, false, 'simple', 'restrict', 'restrict', [1], [0]] +... +box.space._fk_constraint:count() +--- +- 0 +... +-- Replace is also OK. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +--- +... +t = box.space._fk_constraint:insert(t) +--- +... +t = {'fk_1', child_id, parent_id, false, 'simple', 'cascade', 'restrict', {0}, {1}} +--- +... +t = box.space._fk_constraint:replace(t) +--- +... +box.space._fk_constraint:select({'fk_1', child_id})[1]['on_delete'] +--- +- cascade +... +t = {'fk_1', child_id, parent_id, true, 'simple', 'cascade', 'restrict', {0}, {1}} +--- +... +t = box.space._fk_constraint:replace(t) +--- +... +box.space._fk_constraint:select({'fk_1', child_id})[1]['is_deferred'] +--- +- true +... +box.space.CHILD:drop() +--- +... +box.space.PARENT:drop() +--- +... +-- Clean-up SQL DD hash. +test_run:cmd('restart server default with cleanup=1') diff --git a/test/sql/foreign-keys.test.lua b/test/sql/foreign-keys.test.lua new file mode 100644 index 000000000..a7a242bc2 --- /dev/null +++ b/test/sql/foreign-keys.test.lua @@ -0,0 +1,154 @@ +env = require('test_run') +test_run = env.new() +test_run:cmd('restart server default with cleanup=1') + + +-- Check that tuple inserted into _fk_constraint is FK constrains +-- valid data. +-- +box.sql.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a INT, b INT);") +box.sql.execute("CREATE UNIQUE INDEX i1 ON t1(a);") +box.sql.execute("CREATE TABLE t2 (a INT, b INT, id INT PRIMARY KEY);") +box.sql.execute("CREATE VIEW v1 AS SELECT * FROM t1;") + +-- Parent and child spaces must exist. +-- +t = {'fk_1', 666, 777, false, 'simple', 'restrict', 'restrict', {0}, {1}} +box.space._fk_constraint:insert(t) + +parent_id = box.space._space.index.name:select('T1')[1]['id'] +child_id = box.space._space.index.name:select('T2')[1]['id'] +view_id = box.space._space.index.name:select('V1')[1]['id'] + +-- View can't reference another space or be referenced by another space. +-- +t = {'fk_1', child_id, view_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +box.space._fk_constraint:insert(t) +t = {'fk_1', view_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +box.space._fk_constraint:insert(t) +box.sql.execute("DROP VIEW v1;") + +-- Match clause can be only one of: simple, partial, full. +-- +t = {'fk_1', child_id, parent_id, false, 'wrong_match', 'restrict', 'restrict', {0}, {1}} +box.space._fk_constraint:insert(t) + +-- On conflict actions can be only one of: set_null, set_default, +-- restrict, cascade, no_action. +t = {'fk_1', child_id, parent_id, false, 'simple', 'wrong_action', 'restrict', {0}, {1}} +box.space._fk_constraint:insert(t) +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'wrong_action', {0}, {1}} +box.space._fk_constraint:insert(t) + +-- Temporary restriction (until SQL triggers work from Lua): +-- referencing space must be empty. +-- +box.sql.execute("INSERT INTO t2 VALUES (1, 2, 3);") +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {2}, {1}} +box.space._fk_constraint:insert(t) +box.sql.execute("DELETE FROM t2;") + +-- Links must be specififed correctly. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {}, {}} +box.space._fk_constraint:insert(t) +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {2}, {1,2}} +box.space._fk_constraint:insert(t) +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {13}, {1}} +box.space._fk_constraint:insert(t) +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {'crash'}, {'crash'}} +box.space._fk_constraint:insert(t) + +-- Referenced fields must compose unique index. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0, 1}, {1, 2}} +box.space._fk_constraint:insert(t) + +-- Referencing and referenced fields must feature compatible types. +-- Temporary, in SQL all fields except for INTEGER PRIMARY KEY +-- are scalar. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {1}, {0}} +box.space._fk_constraint:insert(t) + +-- Each referenced column must appear once. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0, 1}, {1, 1}} +box.space._fk_constraint:insert(t) + +-- Successful creation. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +t = box.space._fk_constraint:insert(t) + +-- Implicitly referenced index can't be dropped, +-- ergo - space can't be dropped until it is referenced. +-- +box.sql.execute("DROP INDEX i1 on t1;") + +-- Referenced index can't be altered as well, if alter leads to +-- rebuild of index (e.g. index still can be renamed). +box.space._index:replace{512, 1, 'I1', 'tree', {unique = true}, {{field = 0, type = 'unsigned', is_nullable = true}}} +box.space._index:replace{512, 1, 'I2', 'tree', {unique = true}, {{field = 1, type = 'unsigned', is_nullable = true}}} + +-- Finally, can't drop space until it has FK constraints, +-- i.e. by manual removing tuple from _space. +-- But drop() will delete constraints. +-- +box.space.T2.index[0]:drop() +box.space._space:delete(child_id) +box.space.T2:drop() + +-- Make sure that constraint has been successfully dropped, +-- so we can drop now and parent space. +-- +box.space._fk_constraint:select() +box.space.T1:drop() + +-- Create several constraints to make sure that they are held +-- as linked lists correctly including self-referencing constraints. +-- +box.sql.execute("CREATE TABLE child (id INT PRIMARY KEY, a INT);") +box.sql.execute("CREATE TABLE parent (a INT, id INT PRIMARY KEY);") + +parent_id = box.space._space.index.name:select('PARENT')[1]['id'] +child_id = box.space._space.index.name:select('CHILD')[1]['id'] + +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +t = box.space._fk_constraint:insert(t) +t = {'fk_2', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +t = box.space._fk_constraint:insert(t) +t = {'fk_3', parent_id, child_id, false, 'simple', 'restrict', 'restrict', {1}, {0}} +t = box.space._fk_constraint:insert(t) +t = {'self_1', child_id, child_id, false, 'simple', 'restrict', 'restrict', {0}, {0}} +t = box.space._fk_constraint:insert(t) +t = {'self_2', parent_id, parent_id, false, 'simple', 'restrict', 'restrict', {1}, {1}} +t = box.space._fk_constraint:insert(t) + +box.space._fk_constraint:count() +box.space._fk_constraint:delete{'fk_2', child_id} +box.space._fk_constraint:delete{'fk_1', child_id} +t = {'fk_2', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +t = box.space._fk_constraint:insert(t) +box.space._fk_constraint:delete{'fk_2', child_id} +box.space._fk_constraint:delete{'self_2', parent_id} +box.space._fk_constraint:delete{'self_1', child_id} +box.space._fk_constraint:delete{'fk_3', parent_id} +box.space._fk_constraint:count() + +-- Replace is also OK. +-- +t = {'fk_1', child_id, parent_id, false, 'simple', 'restrict', 'restrict', {0}, {1}} +t = box.space._fk_constraint:insert(t) +t = {'fk_1', child_id, parent_id, false, 'simple', 'cascade', 'restrict', {0}, {1}} +t = box.space._fk_constraint:replace(t) +box.space._fk_constraint:select({'fk_1', child_id})[1]['on_delete'] +t = {'fk_1', child_id, parent_id, true, 'simple', 'cascade', 'restrict', {0}, {1}} +t = box.space._fk_constraint:replace(t) +box.space._fk_constraint:select({'fk_1', child_id})[1]['is_deferred'] + +box.space.CHILD:drop() +box.space.PARENT:drop() + +-- Clean-up SQL DD hash. +test_run:cmd('restart server default with cleanup=1') -- 2.15.1
next prev parent reply other threads:[~2018-08-01 20:54 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-07-13 2:04 [tarantool-patches] [PATCH 0/5] Move FK constraints to server Nikita Pettik 2018-07-13 2:04 ` [tarantool-patches] [PATCH 1/5] sql: prohibit creation of FK on unexisting tables Nikita Pettik 2018-07-17 21:05 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:12 ` Vladislav Shpilevoy 2018-08-01 20:54 ` n.pettik 2018-08-02 22:15 ` Vladislav Shpilevoy 2018-08-06 0:27 ` n.pettik 2018-07-13 2:04 ` [tarantool-patches] [PATCH 2/5] schema: add new system space for FK constraints Nikita Pettik 2018-07-17 21:05 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:12 ` Vladislav Shpilevoy 2018-08-01 20:54 ` n.pettik [this message] 2018-08-02 22:15 ` Vladislav Shpilevoy 2018-08-06 0:28 ` n.pettik 2018-08-06 18:24 ` Vladislav Shpilevoy 2018-07-13 2:04 ` [tarantool-patches] [PATCH 3/5] sql: introduce ADD CONSTRAINT statement Nikita Pettik 2018-07-17 21:05 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:12 ` Vladislav Shpilevoy 2018-08-01 20:54 ` n.pettik 2018-08-02 22:15 ` Vladislav Shpilevoy 2018-08-06 0:28 ` n.pettik 2018-08-06 18:24 ` Vladislav Shpilevoy 2018-08-06 23:43 ` n.pettik 2018-07-13 2:04 ` [tarantool-patches] [PATCH 4/5] sql: display error on FK creation and drop failure Nikita Pettik 2018-07-17 21:04 ` [tarantool-patches] " Vladislav Shpilevoy 2018-07-25 10:03 ` n.pettik 2018-07-26 20:11 ` Vladislav Shpilevoy 2018-07-13 2:04 ` [tarantool-patches] [PATCH 5/5] sql: remove SQLITE_OMIT_FOREIGN_KEY define guard Nikita Pettik 2018-07-17 21:04 ` [tarantool-patches] Re: [PATCH 0/5] Move FK constraints to server Vladislav Shpilevoy 2018-08-07 14:57 ` 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=FAAA2017-8559-4DE3-9662-8572DF598979@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='[tarantool-patches] Re: [PATCH 2/5] schema: add new system space for FK constraints' \ /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