* [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers [not found] <cover.1574874484.git.i.kosarev@tarantool.org> @ 2019-11-27 17:11 ` Ilya Kosarev 2019-11-28 15:26 ` Sergey Ostanevich 0 siblings, 1 reply; 4+ messages in thread From: Ilya Kosarev @ 2019-11-27 17:11 UTC (permalink / raw) To: tarantool-patches std operator new might throw so we need to wrap it in triggers to provide non-throwing triggers. It also means alter_space_move_indexes returns an error code now. It's usages are updated. Part of #4247 --- src/box/alter.cc | 135 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 95 insertions(+), 40 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 4a3241a796..bb78af6a5c 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -1814,7 +1814,7 @@ on_create_space_rollback(struct trigger *trigger, void *event) * Create MoveIndex operation for a range of indexes in a space * for range [begin, end) */ -void +int alter_space_move_indexes(struct alter_space *alter, uint32_t begin, uint32_t end) { @@ -1839,9 +1839,17 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, new_def = index_def_dup(old_def); index_def_update_optionality(new_def, min_field_count); - (void) new ModifyIndex(alter, old_index, new_def); + try { + (void) new ModifyIndex(alter, old_index, new_def); + } catch (Exception *e) { + return -1; + } } else { - (void) new MoveIndex(alter, old_def->iid); + try { + (void) new MoveIndex(alter, old_def->iid); + } catch (Exception *e) { + return -1; + } } continue; } @@ -1856,11 +1864,20 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, index_def_update_optionality(new_def, min_field_count); auto guard = make_scoped_guard([=] { index_def_delete(new_def); }); if (!index_def_change_requires_rebuild(old_index, new_def)) - (void) new ModifyIndex(alter, old_index, new_def); + try { + (void) new ModifyIndex(alter, old_index, new_def); + } catch (Exception *e) { + return -1; + } else - (void) new RebuildIndex(alter, new_def, old_def); + try { + (void) new RebuildIndex(alter, new_def, old_def); + } catch (Exception *e) { + return -1; + } guard.is_active = false; } + return 0; } /** @@ -2301,15 +2318,21 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) old_space->index_count, def->fields, def->field_count); - (void) new CheckSpaceFormat(alter); - (void) new ModifySpace(alter, def); - (void) new RebuildCkConstraints(alter); + try { + (void) new CheckSpaceFormat(alter); + (void) new ModifySpace(alter, def); + (void) new RebuildCkConstraints(alter); + } catch (Exception *e) { + return -1; + } def_guard.is_active = false; /* Create MoveIndex ops for all space indexes. */ - alter_space_move_indexes(alter, 0, old_space->index_id_max + 1); - /* Remember to update schema_version. */ - (void) new UpdateSchemaVersion(alter); + if (alter_space_move_indexes(alter, 0, + old_space->index_id_max + 1) != 0) + return -1; try { + /* Remember to update schema_version. */ + (void) new UpdateSchemaVersion(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; @@ -2476,13 +2499,24 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) "can not drop a referenced index"); return -1; } - alter_space_move_indexes(alter, 0, iid); - (void) new DropIndex(alter, old_index); + if (alter_space_move_indexes(alter, 0, iid) != 0) + return -1; + try { + (void) new DropIndex(alter, old_index); + } catch (Exception *e) { + return -1; + } } /* Case 2: create an index, if it is simply created. */ if (old_index == NULL && new_tuple != NULL) { - alter_space_move_indexes(alter, 0, iid); - CreateIndex *create_index = new CreateIndex(alter); + if (alter_space_move_indexes(alter, 0, iid)) + return -1; + CreateIndex *create_index; + try { + create_index = new CreateIndex(alter); + } catch (Exception *e) { + return -1; + } create_index->new_index_def = index_def_new_from_tuple(new_tuple, old_space); if (create_index->new_index_def == NULL) @@ -2536,10 +2570,16 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) def->field_count); index_def_update_optionality(index_def, alter->new_min_field_count); - alter_space_move_indexes(alter, 0, iid); + if (alter_space_move_indexes(alter, 0, iid)) + return -1; if (index_def_cmp(index_def, old_index->def) == 0) { /* Index is not changed so just move it. */ - (void) new MoveIndex(alter, old_index->def->iid); + try { + (void) new MoveIndex(alter, old_index->def->iid); + } catch (Exception *e) { + return -1; + } + } else if (index_def_change_requires_rebuild(old_index, index_def)) { if (index_is_used_by_fk_constraint(&old_space->parent_fk_constraint, @@ -2552,8 +2592,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) /* * Operation demands an index rebuild. */ - (void) new RebuildIndex(alter, index_def, - old_index->def); + try { + (void) new RebuildIndex(alter, index_def, + old_index->def); + } catch (Exception *e) { + return -1; + } index_def_guard.is_active = false; } else { /* @@ -2561,8 +2605,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * but we still need to check that tuples stored * in the space conform to the new format. */ - (void) new CheckSpaceFormat(alter); - (void) new ModifyIndex(alter, old_index, index_def); + try { + (void) new CheckSpaceFormat(alter); + (void) new ModifyIndex(alter, old_index, index_def); + } catch (Exception *e) { + return -1; + } index_def_guard.is_active = false; } } @@ -2570,11 +2618,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * Create MoveIndex ops for the remaining indexes in the * old space. */ - alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1); - (void) new MoveCkConstraints(alter); - /* Add an op to update schema_version on commit. */ - (void) new UpdateSchemaVersion(alter); + if (alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1) != 0) + return -1; try { + (void) new MoveCkConstraints(alter); + /* Add an op to update schema_version on commit. */ + (void) new UpdateSchemaVersion(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; @@ -2654,16 +2703,16 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) stmt->row->group_id = GROUP_LOCAL; } - /* - * Recreate all indexes of the truncated space. - */ - for (uint32_t i = 0; i < old_space->index_count; i++) { - struct index *old_index = old_space->index[i]; - (void) new TruncateIndex(alter, old_index->def->iid); - } - - (void) new MoveCkConstraints(alter); try { + /* + * Recreate all indexes of the truncated space. + */ + for (uint32_t i = 0; i < old_space->index_count; i++) { + struct index *old_index = old_space->index[i]; + (void) new TruncateIndex(alter, old_index->def->iid); + } + + (void) new MoveCkConstraints(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; @@ -5562,13 +5611,19 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) if (alter == NULL) return -1; auto scoped_guard = make_scoped_guard([=] {alter_space_delete(alter);}); - alter_space_move_indexes(alter, 0, index->def->iid); - (void) new RebuildFuncIndex(alter, index->def, func); - alter_space_move_indexes(alter, index->def->iid + 1, - space->index_id_max + 1); - (void) new MoveCkConstraints(alter); - (void) new UpdateSchemaVersion(alter); + if (alter_space_move_indexes(alter, 0, index->def->iid) != 0) + return -1; try { + (void) new RebuildFuncIndex(alter, index->def, func); + } catch (Exception *e) { + return -1; + } + if (alter_space_move_indexes(alter, index->def->iid + 1, + space->index_id_max + 1) != 0) + return -1; + try { + (void) new MoveCkConstraints(alter); + (void) new UpdateSchemaVersion(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; -- 2.17.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers 2019-11-27 17:11 ` [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers Ilya Kosarev @ 2019-11-28 15:26 ` Sergey Ostanevich 0 siblings, 0 replies; 4+ messages in thread From: Sergey Ostanevich @ 2019-11-28 15:26 UTC (permalink / raw) To: Ilya Kosarev; +Cc: tarantool-patches [-- Attachment #1: Type: text/plain, Size: 8462 bytes --] LGTM. Sergos >Wednesday, November 27, 2019 8:11 PM +03:00 from Ilya Kosarev <i.kosarev@tarantool.org>: > >std operator new might throw so we need to wrap it in triggers to >provide non-throwing triggers. It also means alter_space_move_indexes >returns an error code now. It's usages are updated. > >Part of #4247 >--- > src/box/alter.cc | 135 +++++++++++++++++++++++++++++++++-------------- > 1 file changed, 95 insertions(+), 40 deletions(-) > >diff --git a/src/box/alter.cc b/src/box/alter.cc >index 4a3241a796..bb78af6a5c 100644 >--- a/src/box/alter.cc >+++ b/src/box/alter.cc >@@ -1814,7 +1814,7 @@ on_create_space_rollback(struct trigger *trigger, void *event) > * Create MoveIndex operation for a range of indexes in a space > * for range [begin, end) > */ >-void >+int > alter_space_move_indexes(struct alter_space *alter, uint32_t begin, > uint32_t end) > { >@@ -1839,9 +1839,17 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, > new_def = index_def_dup(old_def); > index_def_update_optionality(new_def, > min_field_count); >- (void) new ModifyIndex(alter, old_index, new_def); >+ try { >+ (void) new ModifyIndex(alter, old_index, new_def); >+ } catch (Exception *e) { >+ return -1; >+ } > } else { >- (void) new MoveIndex(alter, old_def->iid); >+ try { >+ (void) new MoveIndex(alter, old_def->iid); >+ } catch (Exception *e) { >+ return -1; >+ } > } > continue; > } >@@ -1856,11 +1864,20 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, > index_def_update_optionality(new_def, min_field_count); > auto guard = make_scoped_guard([=] { index_def_delete(new_def); }); > if (!index_def_change_requires_rebuild(old_index, new_def)) >- (void) new ModifyIndex(alter, old_index, new_def); >+ try { >+ (void) new ModifyIndex(alter, old_index, new_def); >+ } catch (Exception *e) { >+ return -1; >+ } > else >- (void) new RebuildIndex(alter, new_def, old_def); >+ try { >+ (void) new RebuildIndex(alter, new_def, old_def); >+ } catch (Exception *e) { >+ return -1; >+ } > guard.is_active = false; > } >+ return 0; > } > > /** >@@ -2301,15 +2318,21 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) > old_space->index_count, > def->fields, > def->field_count); >- (void) new CheckSpaceFormat(alter); >- (void) new ModifySpace(alter, def); >- (void) new RebuildCkConstraints(alter); >+ try { >+ (void) new CheckSpaceFormat(alter); >+ (void) new ModifySpace(alter, def); >+ (void) new RebuildCkConstraints(alter); >+ } catch (Exception *e) { >+ return -1; >+ } > def_guard.is_active = false; > /* Create MoveIndex ops for all space indexes. */ >- alter_space_move_indexes(alter, 0, old_space->index_id_max + 1); >- /* Remember to update schema_version. */ >- (void) new UpdateSchemaVersion(alter); >+ if (alter_space_move_indexes(alter, 0, >+ old_space->index_id_max + 1) != 0) >+ return -1; > try { >+ /* Remember to update schema_version. */ >+ (void) new UpdateSchemaVersion(alter); > alter_space_do(stmt, alter); > } catch (Exception *e) { > return -1; >@@ -2476,13 +2499,24 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) > "can not drop a referenced index"); > return -1; > } >- alter_space_move_indexes(alter, 0, iid); >- (void) new DropIndex(alter, old_index); >+ if (alter_space_move_indexes(alter, 0, iid) != 0) >+ return -1; >+ try { >+ (void) new DropIndex(alter, old_index); >+ } catch (Exception *e) { >+ return -1; >+ } > } > /* Case 2: create an index, if it is simply created. */ > if (old_index == NULL && new_tuple != NULL) { >- alter_space_move_indexes(alter, 0, iid); >- CreateIndex *create_index = new CreateIndex(alter); >+ if (alter_space_move_indexes(alter, 0, iid)) >+ return -1; >+ CreateIndex *create_index; >+ try { >+ create_index = new CreateIndex(alter); >+ } catch (Exception *e) { >+ return -1; >+ } > create_index->new_index_def = > index_def_new_from_tuple(new_tuple, old_space); > if (create_index->new_index_def == NULL) >@@ -2536,10 +2570,16 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) > def->field_count); > index_def_update_optionality(index_def, > alter->new_min_field_count); >- alter_space_move_indexes(alter, 0, iid); >+ if (alter_space_move_indexes(alter, 0, iid)) >+ return -1; > if (index_def_cmp(index_def, old_index->def) == 0) { > /* Index is not changed so just move it. */ >- (void) new MoveIndex(alter, old_index->def->iid); >+ try { >+ (void) new MoveIndex(alter, old_index->def->iid); >+ } catch (Exception *e) { >+ return -1; >+ } >+ > } else if (index_def_change_requires_rebuild(old_index, > index_def)) { > if (index_is_used_by_fk_constraint(&old_space->parent_fk_constraint, >@@ -2552,8 +2592,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) > /* > * Operation demands an index rebuild. > */ >- (void) new RebuildIndex(alter, index_def, >- old_index->def); >+ try { >+ (void) new RebuildIndex(alter, index_def, >+ old_index->def); >+ } catch (Exception *e) { >+ return -1; >+ } > index_def_guard.is_active = false; > } else { > /* >@@ -2561,8 +2605,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) > * but we still need to check that tuples stored > * in the space conform to the new format. > */ >- (void) new CheckSpaceFormat(alter); >- (void) new ModifyIndex(alter, old_index, index_def); >+ try { >+ (void) new CheckSpaceFormat(alter); >+ (void) new ModifyIndex(alter, old_index, index_def); >+ } catch (Exception *e) { >+ return -1; >+ } > index_def_guard.is_active = false; > } > } >@@ -2570,11 +2618,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) > * Create MoveIndex ops for the remaining indexes in the > * old space. > */ >- alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1); >- (void) new MoveCkConstraints(alter); >- /* Add an op to update schema_version on commit. */ >- (void) new UpdateSchemaVersion(alter); >+ if (alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1) != 0) >+ return -1; > try { >+ (void) new MoveCkConstraints(alter); >+ /* Add an op to update schema_version on commit. */ >+ (void) new UpdateSchemaVersion(alter); > alter_space_do(stmt, alter); > } catch (Exception *e) { > return -1; >@@ -2654,16 +2703,16 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) > stmt->row->group_id = GROUP_LOCAL; > } > >- /* >- * Recreate all indexes of the truncated space. >- */ >- for (uint32_t i = 0; i < old_space->index_count; i++) { >- struct index *old_index = old_space->index[i]; >- (void) new TruncateIndex(alter, old_index->def->iid); >- } >- >- (void) new MoveCkConstraints(alter); > try { >+ /* >+ * Recreate all indexes of the truncated space. >+ */ >+ for (uint32_t i = 0; i < old_space->index_count; i++) { >+ struct index *old_index = old_space->index[i]; >+ (void) new TruncateIndex(alter, old_index->def->iid); >+ } >+ >+ (void) new MoveCkConstraints(alter); > alter_space_do(stmt, alter); > } catch (Exception *e) { > return -1; >@@ -5562,13 +5611,19 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) > if (alter == NULL) > return -1; > auto scoped_guard = make_scoped_guard([=] {alter_space_delete(alter);}); >- alter_space_move_indexes(alter, 0, index->def->iid); >- (void) new RebuildFuncIndex(alter, index->def, func); >- alter_space_move_indexes(alter, index->def->iid + 1, >- space->index_id_max + 1); >- (void) new MoveCkConstraints(alter); >- (void) new UpdateSchemaVersion(alter); >+ if (alter_space_move_indexes(alter, 0, index->def->iid) != 0) >+ return -1; > try { >+ (void) new RebuildFuncIndex(alter, index->def, func); >+ } catch (Exception *e) { >+ return -1; >+ } >+ if (alter_space_move_indexes(alter, index->def->iid + 1, >+ space->index_id_max + 1) != 0) >+ return -1; >+ try { >+ (void) new MoveCkConstraints(alter); >+ (void) new UpdateSchemaVersion(alter); > alter_space_do(stmt, alter); > } catch (Exception *e) { > return -1; >-- >2.17.1 > [-- Attachment #2: Type: text/html, Size: 10079 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Tarantool-patches] [PATCH v5 0/8] refactoring: remove exceptions from triggers follow-ups @ 2019-11-22 2:46 Ilya Kosarev 2019-11-22 2:46 ` [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers Ilya Kosarev 0 siblings, 1 reply; 4+ messages in thread From: Ilya Kosarev @ 2019-11-22 2:46 UTC (permalink / raw) To: tarantool-patches This patchset is a pack of follow-ups to finish clearing triggers from exceptions. It is based on review comments for the patches of the original "refactoring: remove exceptions from triggers" patchset. Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4247-remove-exceptions-from-triggers Issue: https://github.com/tarantool/tarantool/issues/4247 Ilya Kosarev (8): refactoring: wrap new operator calls in triggers refactoring: specify struct name in allocation diagnostics refactoring: recombine error conditions in triggers refactoring: set diagnostics if sequence_by_id fails refactoring: clear triggers from fresh exceptions refactoring: update comment for index_def_check_tuple refactoring: remove redundant line in txn_alter_trigger_new refactoring: remove try..catch wrapper around trigger->run src/box/alter.cc | 192 +++++++++++++++++++++++++--------------- src/box/user.cc | 2 +- src/lib/core/trigger.cc | 25 ++---- src/lua/trigger.c | 2 +- 4 files changed, 133 insertions(+), 88 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers 2019-11-22 2:46 [Tarantool-patches] [PATCH v5 0/8] refactoring: remove exceptions from triggers follow-ups Ilya Kosarev @ 2019-11-22 2:46 ` Ilya Kosarev 2019-11-26 15:07 ` Sergey Ostanevich 0 siblings, 1 reply; 4+ messages in thread From: Ilya Kosarev @ 2019-11-22 2:46 UTC (permalink / raw) To: tarantool-patches std operator new might throw so we need to wrap it in triggers to provide non-throwing triggers. It also means alter_space_move_indexes returns an error code now. It's usages are updated. Part of #4247 --- src/box/alter.cc | 135 +++++++++++++++++++++++++++++++++-------------- 1 file changed, 95 insertions(+), 40 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 4a3241a79..022c7dfa2 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -1814,7 +1814,7 @@ on_create_space_rollback(struct trigger *trigger, void *event) * Create MoveIndex operation for a range of indexes in a space * for range [begin, end) */ -void +int alter_space_move_indexes(struct alter_space *alter, uint32_t begin, uint32_t end) { @@ -1839,9 +1839,17 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, new_def = index_def_dup(old_def); index_def_update_optionality(new_def, min_field_count); - (void) new ModifyIndex(alter, old_index, new_def); + try { + (void) new ModifyIndex(alter, old_index, new_def); + } catch (Exception *e) { + return -1; + } } else { - (void) new MoveIndex(alter, old_def->iid); + try { + (void) new MoveIndex(alter, old_def->iid); + } catch (Exception *e) { + return -1; + } } continue; } @@ -1856,11 +1864,20 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, index_def_update_optionality(new_def, min_field_count); auto guard = make_scoped_guard([=] { index_def_delete(new_def); }); if (!index_def_change_requires_rebuild(old_index, new_def)) - (void) new ModifyIndex(alter, old_index, new_def); + try { + (void) new ModifyIndex(alter, old_index, new_def); + } catch (Exception *e) { + return -1; + } else - (void) new RebuildIndex(alter, new_def, old_def); + try { + (void) new RebuildIndex(alter, new_def, old_def); + } catch (Exception *e) { + return -1; + } guard.is_active = false; } + return 0; } /** @@ -2301,15 +2318,21 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) old_space->index_count, def->fields, def->field_count); - (void) new CheckSpaceFormat(alter); - (void) new ModifySpace(alter, def); - (void) new RebuildCkConstraints(alter); + try { + (void) new CheckSpaceFormat(alter); + (void) new ModifySpace(alter, def); + (void) new RebuildCkConstraints(alter); + } catch (Exception *e) { + return -1; + } def_guard.is_active = false; /* Create MoveIndex ops for all space indexes. */ - alter_space_move_indexes(alter, 0, old_space->index_id_max + 1); - /* Remember to update schema_version. */ - (void) new UpdateSchemaVersion(alter); + if (alter_space_move_indexes(alter, 0, + old_space->index_id_max + 1) != 0) + return -1; try { + /* Remember to update schema_version. */ + (void) new UpdateSchemaVersion(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; @@ -2476,13 +2499,24 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) "can not drop a referenced index"); return -1; } - alter_space_move_indexes(alter, 0, iid); - (void) new DropIndex(alter, old_index); + if (alter_space_move_indexes(alter, 0, iid) != 0) + return -1; + try { + (void) new DropIndex(alter, old_index); + } catch (Exception *e) { + return -1; + } } /* Case 2: create an index, if it is simply created. */ if (old_index == NULL && new_tuple != NULL) { - alter_space_move_indexes(alter, 0, iid); - CreateIndex *create_index = new CreateIndex(alter); + if (alter_space_move_indexes(alter, 0, iid)) + return -1; + CreateIndex *create_index; + try { + create_index = new CreateIndex(alter); + } catch (Exception *e) { + return -1; + } create_index->new_index_def = index_def_new_from_tuple(new_tuple, old_space); if (create_index->new_index_def == NULL) @@ -2536,10 +2570,16 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) def->field_count); index_def_update_optionality(index_def, alter->new_min_field_count); - alter_space_move_indexes(alter, 0, iid); + if (alter_space_move_indexes(alter, 0, iid)) + return -1; if (index_def_cmp(index_def, old_index->def) == 0) { /* Index is not changed so just move it. */ - (void) new MoveIndex(alter, old_index->def->iid); + try { + (void) new MoveIndex(alter, old_index->def->iid); + } catch (Exception *e) { + return -1; + } + } else if (index_def_change_requires_rebuild(old_index, index_def)) { if (index_is_used_by_fk_constraint(&old_space->parent_fk_constraint, @@ -2552,8 +2592,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) /* * Operation demands an index rebuild. */ - (void) new RebuildIndex(alter, index_def, - old_index->def); + try { + (void) new RebuildIndex(alter, index_def, + old_index->def); + } catch (Exception *e) { + return -1; + } index_def_guard.is_active = false; } else { /* @@ -2561,8 +2605,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * but we still need to check that tuples stored * in the space conform to the new format. */ - (void) new CheckSpaceFormat(alter); - (void) new ModifyIndex(alter, old_index, index_def); + try { + (void) new CheckSpaceFormat(alter); + (void) new ModifyIndex(alter, old_index, index_def); + } catch (Exception *e) { + return -1; + } index_def_guard.is_active = false; } } @@ -2570,11 +2618,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) * Create MoveIndex ops for the remaining indexes in the * old space. */ - alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1); - (void) new MoveCkConstraints(alter); - /* Add an op to update schema_version on commit. */ - (void) new UpdateSchemaVersion(alter); + if (alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1) != 0) + return -1; try { + (void) new MoveCkConstraints(alter); + /* Add an op to update schema_version on commit. */ + (void) new UpdateSchemaVersion(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; @@ -2654,16 +2703,16 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event) stmt->row->group_id = GROUP_LOCAL; } - /* - * Recreate all indexes of the truncated space. - */ - for (uint32_t i = 0; i < old_space->index_count; i++) { - struct index *old_index = old_space->index[i]; - (void) new TruncateIndex(alter, old_index->def->iid); - } - - (void) new MoveCkConstraints(alter); try { + /* + * Recreate all indexes of the truncated space. + */ + for (uint32_t i = 0; i < old_space->index_count; i++) { + struct index *old_index = old_space->index[i]; + (void) new TruncateIndex(alter, old_index->def->iid); + } + + (void) new MoveCkConstraints(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; @@ -5562,13 +5611,19 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) if (alter == NULL) return -1; auto scoped_guard = make_scoped_guard([=] {alter_space_delete(alter);}); - alter_space_move_indexes(alter, 0, index->def->iid); - (void) new RebuildFuncIndex(alter, index->def, func); - alter_space_move_indexes(alter, index->def->iid + 1, - space->index_id_max + 1); - (void) new MoveCkConstraints(alter); - (void) new UpdateSchemaVersion(alter); + if (alter_space_move_indexes(alter, 0, index->def->iid) != 0) + return -1; try { + (void) new RebuildFuncIndex(alter, index->def, func); + } catch (Exception *e) { + return -1; + } + if (alter_space_move_indexes(alter, index->def->iid + 1, + space->index_id_max + 1) != 0) + return -1; + try { + (void) new MoveCkConstraints(alter); + (void) new UpdateSchemaVersion(alter); alter_space_do(stmt, alter); } catch (Exception *e) { return -1; -- 2.17.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers 2019-11-22 2:46 ` [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers Ilya Kosarev @ 2019-11-26 15:07 ` Sergey Ostanevich 0 siblings, 0 replies; 4+ messages in thread From: Sergey Ostanevich @ 2019-11-26 15:07 UTC (permalink / raw) To: Ilya Kosarev; +Cc: tarantool-patches Hi! One nit below, otherwise LGTM. Sergos On 22 Nov 05:46, Ilya Kosarev wrote: > @@ -5562,13 +5611,19 @@ on_replace_dd_func_index(struct trigger *trigger, void *event) > if (alter == NULL) > return -1; > auto scoped_guard = make_scoped_guard([=] {alter_space_delete(alter);}); > - alter_space_move_indexes(alter, 0, index->def->iid); > - (void) new RebuildFuncIndex(alter, index->def, func); > - alter_space_move_indexes(alter, index->def->iid + 1, > - space->index_id_max + 1); > - (void) new MoveCkConstraints(alter); > - (void) new UpdateSchemaVersion(alter); > + if (alter_space_move_indexes(alter, 0, index->def->iid) != 0) > + return -1; > try { > + (void) new RebuildFuncIndex(alter, index->def, func); > + } catch (Exception *e) { > + return -1; > + } > + if (alter_space_move_indexes(alter, index->def->iid + 1, > + space->index_id_max + 1) != 0) indentation is broken for the line above - need 4 extra spaces. > + return -1; > + try { > + (void) new MoveCkConstraints(alter); > + (void) new UpdateSchemaVersion(alter); > alter_space_do(stmt, alter); > } catch (Exception *e) { > return -1; > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-28 15:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <cover.1574874484.git.i.kosarev@tarantool.org>
2019-11-27 17:11 ` [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers Ilya Kosarev
2019-11-28 15:26 ` Sergey Ostanevich
2019-11-22 2:46 [Tarantool-patches] [PATCH v5 0/8] refactoring: remove exceptions from triggers follow-ups Ilya Kosarev
2019-11-22 2:46 ` [Tarantool-patches] [PATCH v5 1/8] refactoring: wrap new operator calls in triggers Ilya Kosarev
2019-11-26 15:07 ` Sergey Ostanevich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox