<HTML><BODY><br>LGTM. <br>Sergos<br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Wednesday, November 27, 2019 8:11 PM +03:00 from Ilya Kosarev <i.kosarev@tarantool.org>:<br>
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<div id="style_15748746911424622181_BODY">std operator new might throw so we need to wrap it in triggers to<br>
provide non-throwing triggers. It also means alter_space_move_indexes<br>
returns an error code now. It's usages are updated.<br>
<br>
Part of #4247<br>
---<br>
src/box/alter.cc | 135 +++++++++++++++++++++++++++++++++--------------<br>
1 file changed, 95 insertions(+), 40 deletions(-)<br>
<br>
diff --git a/src/box/alter.cc b/src/box/alter.cc<br>
index 4a3241a796..bb78af6a5c 100644<br>
--- a/src/box/alter.cc<br>
+++ b/src/box/alter.cc<br>
@@ -1814,7 +1814,7 @@ on_create_space_rollback(struct trigger *trigger, void *event)<br>
* Create MoveIndex operation for a range of indexes in a space<br>
* for range [begin, end)<br>
*/<br>
-void<br>
+int<br>
alter_space_move_indexes(struct alter_space *alter, uint32_t begin,<br>
uint32_t end)<br>
{<br>
@@ -1839,9 +1839,17 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin,<br>
new_def = index_def_dup(old_def);<br>
index_def_update_optionality(new_def,<br>
min_field_count);<br>
- (void) new ModifyIndex(alter, old_index, new_def);<br>
+ try {<br>
+ (void) new ModifyIndex(alter, old_index, new_def);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
} else {<br>
- (void) new MoveIndex(alter, old_def->iid);<br>
+ try {<br>
+ (void) new MoveIndex(alter, old_def->iid);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
}<br>
continue;<br>
}<br>
@@ -1856,11 +1864,20 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin,<br>
index_def_update_optionality(new_def, min_field_count);<br>
auto guard = make_scoped_guard([=] { index_def_delete(new_def); });<br>
if (!index_def_change_requires_rebuild(old_index, new_def))<br>
- (void) new ModifyIndex(alter, old_index, new_def);<br>
+ try {<br>
+ (void) new ModifyIndex(alter, old_index, new_def);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
else<br>
- (void) new RebuildIndex(alter, new_def, old_def);<br>
+ try {<br>
+ (void) new RebuildIndex(alter, new_def, old_def);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
guard.is_active = false;<br>
}<br>
+ return 0;<br>
}<br>
<br>
/**<br>
@@ -2301,15 +2318,21 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)<br>
old_space->index_count,<br>
def->fields,<br>
def->field_count);<br>
- (void) new CheckSpaceFormat(alter);<br>
- (void) new ModifySpace(alter, def);<br>
- (void) new RebuildCkConstraints(alter);<br>
+ try {<br>
+ (void) new CheckSpaceFormat(alter);<br>
+ (void) new ModifySpace(alter, def);<br>
+ (void) new RebuildCkConstraints(alter);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
def_guard.is_active = false;<br>
/* Create MoveIndex ops for all space indexes. */<br>
- alter_space_move_indexes(alter, 0, old_space->index_id_max + 1);<br>
- /* Remember to update schema_version. */<br>
- (void) new UpdateSchemaVersion(alter);<br>
+ if (alter_space_move_indexes(alter, 0,<br>
+ old_space->index_id_max + 1) != 0)<br>
+ return -1;<br>
try {<br>
+ /* Remember to update schema_version. */<br>
+ (void) new UpdateSchemaVersion(alter);<br>
alter_space_do(stmt, alter);<br>
} catch (Exception *e) {<br>
return -1;<br>
@@ -2476,13 +2499,24 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)<br>
"can not drop a referenced index");<br>
return -1;<br>
}<br>
- alter_space_move_indexes(alter, 0, iid);<br>
- (void) new DropIndex(alter, old_index);<br>
+ if (alter_space_move_indexes(alter, 0, iid) != 0)<br>
+ return -1;<br>
+ try {<br>
+ (void) new DropIndex(alter, old_index);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
}<br>
/* Case 2: create an index, if it is simply created. */<br>
if (old_index == NULL && new_tuple != NULL) {<br>
- alter_space_move_indexes(alter, 0, iid);<br>
- CreateIndex *create_index = new CreateIndex(alter);<br>
+ if (alter_space_move_indexes(alter, 0, iid))<br>
+ return -1;<br>
+ CreateIndex *create_index;<br>
+ try {<br>
+ create_index = new CreateIndex(alter);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
create_index->new_index_def =<br>
index_def_new_from_tuple(new_tuple, old_space);<br>
if (create_index->new_index_def == NULL)<br>
@@ -2536,10 +2570,16 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)<br>
def->field_count);<br>
index_def_update_optionality(index_def,<br>
alter->new_min_field_count);<br>
- alter_space_move_indexes(alter, 0, iid);<br>
+ if (alter_space_move_indexes(alter, 0, iid))<br>
+ return -1;<br>
if (index_def_cmp(index_def, old_index->def) == 0) {<br>
/* Index is not changed so just move it. */<br>
- (void) new MoveIndex(alter, old_index->def->iid);<br>
+ try {<br>
+ (void) new MoveIndex(alter, old_index->def->iid);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
+<br>
} else if (index_def_change_requires_rebuild(old_index,<br>
index_def)) {<br>
if (index_is_used_by_fk_constraint(&old_space->parent_fk_constraint,<br>
@@ -2552,8 +2592,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)<br>
/*<br>
* Operation demands an index rebuild.<br>
*/<br>
- (void) new RebuildIndex(alter, index_def,<br>
- old_index->def);<br>
+ try {<br>
+ (void) new RebuildIndex(alter, index_def,<br>
+ old_index->def);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
index_def_guard.is_active = false;<br>
} else {<br>
/*<br>
@@ -2561,8 +2605,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)<br>
* but we still need to check that tuples stored<br>
* in the space conform to the new format.<br>
*/<br>
- (void) new CheckSpaceFormat(alter);<br>
- (void) new ModifyIndex(alter, old_index, index_def);<br>
+ try {<br>
+ (void) new CheckSpaceFormat(alter);<br>
+ (void) new ModifyIndex(alter, old_index, index_def);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
index_def_guard.is_active = false;<br>
}<br>
}<br>
@@ -2570,11 +2618,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)<br>
* Create MoveIndex ops for the remaining indexes in the<br>
* old space.<br>
*/<br>
- alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1);<br>
- (void) new MoveCkConstraints(alter);<br>
- /* Add an op to update schema_version on commit. */<br>
- (void) new UpdateSchemaVersion(alter);<br>
+ if (alter_space_move_indexes(alter, iid + 1, old_space->index_id_max + 1) != 0)<br>
+ return -1;<br>
try {<br>
+ (void) new MoveCkConstraints(alter);<br>
+ /* Add an op to update schema_version on commit. */<br>
+ (void) new UpdateSchemaVersion(alter);<br>
alter_space_do(stmt, alter);<br>
} catch (Exception *e) {<br>
return -1;<br>
@@ -2654,16 +2703,16 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)<br>
stmt->row->group_id = GROUP_LOCAL;<br>
}<br>
<br>
- /*<br>
- * Recreate all indexes of the truncated space.<br>
- */<br>
- for (uint32_t i = 0; i < old_space->index_count; i++) {<br>
- struct index *old_index = old_space->index[i];<br>
- (void) new TruncateIndex(alter, old_index->def->iid);<br>
- }<br>
-<br>
- (void) new MoveCkConstraints(alter);<br>
try {<br>
+ /*<br>
+ * Recreate all indexes of the truncated space.<br>
+ */<br>
+ for (uint32_t i = 0; i < old_space->index_count; i++) {<br>
+ struct index *old_index = old_space->index[i];<br>
+ (void) new TruncateIndex(alter, old_index->def->iid);<br>
+ }<br>
+<br>
+ (void) new MoveCkConstraints(alter);<br>
alter_space_do(stmt, alter);<br>
} catch (Exception *e) {<br>
return -1;<br>
@@ -5562,13 +5611,19 @@ on_replace_dd_func_index(struct trigger *trigger, void *event)<br>
if (alter == NULL)<br>
return -1;<br>
auto scoped_guard = make_scoped_guard([=] {alter_space_delete(alter);});<br>
- alter_space_move_indexes(alter, 0, index->def->iid);<br>
- (void) new RebuildFuncIndex(alter, index->def, func);<br>
- alter_space_move_indexes(alter, index->def->iid + 1,<br>
- space->index_id_max + 1);<br>
- (void) new MoveCkConstraints(alter);<br>
- (void) new UpdateSchemaVersion(alter);<br>
+ if (alter_space_move_indexes(alter, 0, index->def->iid) != 0)<br>
+ return -1;<br>
try {<br>
+ (void) new RebuildFuncIndex(alter, index->def, func);<br>
+ } catch (Exception *e) {<br>
+ return -1;<br>
+ }<br>
+ if (alter_space_move_indexes(alter, index->def->iid + 1,<br>
+ space->index_id_max + 1) != 0)<br>
+ return -1;<br>
+ try {<br>
+ (void) new MoveCkConstraints(alter);<br>
+ (void) new UpdateSchemaVersion(alter);<br>
alter_space_do(stmt, alter);<br>
} catch (Exception *e) {<br>
return -1;<br>
-- <br>
2.17.1<br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<br>
<br> <br></BODY></HTML>