<HTML><BODY><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Hi!<br><br>Thanks for your review.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Yes, i definitely missed diag_set with sequence_by_id. Will be fixed in v5. <br><br>Also i will reconsider combining child & parent space search.</p><p style="font-family: Arial, Tahoma, Verdana, sans-serif;" data-mce-style="font-family: Arial, Tahoma, Verdana, sans-serif;">Sincerely,<br>Ilya Kosarev</p><br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Суббота, 9 ноября 2019, 23:01 +03:00 от Sergey Ostanevich <sergos@tarantool.org>:<br>
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<div id="style_15733296721644614836_BODY"><div class="class_1573361704">
Hi!<br><br>LGTM, just couple of nits below you can add to a follow-up.<br><br>Sergos<br><br><blockquote style="border-left:1px solid #0857A6;margin:10px;padding:0 0 0 10px;"><div id=""><div class="js-helper_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix"><div><div id="style_15692542750458027292_BODY_mailru_css_attribute_postfix">diff --git a/src/box/alter.cc b/src/box/alter.cc<br>
index e56d7c6cc..69a749326 100644<br>
--- a/src/box/alter.cc<br>
+++ b/src/box/alter.cc<br>@@ -4117,14 +4167,18 @@ on_replace_dd_sequence_data(struct trigger * /* trigger */, void *event)<br>
struct tuple *old_tuple = stmt->old_tuple;<br>
struct tuple *new_tuple = stmt->new_tuple;<br><br>
- uint32_t id = tuple_field_u32_xc(old_tuple ?: new_tuple,<br>
- BOX_SEQUENCE_DATA_FIELD_ID);<br>
- struct sequence *seq = sequence_cache_find(id);<br>
+ uint32_t id;<br>
+ if (tuple_field_u32(old_tuple ?: new_tuple, BOX_SEQUENCE_DATA_FIELD_ID,<br>
+ &id) != 0)<br>
+ return -1;<br>
+ struct sequence *seq = sequence_by_id(id);<br>
if (seq == NULL)<br>
return -1;<br></div></div></div></div></blockquote>Note, that here we will miss the diag_set for non-zero value. <br>Perhaps a question of further patches for sequence_by_id() - but I didn't find it in log<br><br><blockquote style="border-left:1px solid #0857A6;margin:10px;padding:0 0 0 10px;"><div id=""><div class="js-helper_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix"><div><div id="style_15692542750458027292_BODY_mailru_css_attribute_postfix">@@ -4958,10 +5034,10 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)<br>
fk_constraint_def_new_from_tuple(old_tuple,<br>
ER_DROP_FK_CONSTRAINT);<br>
auto fk_def_guard = make_scoped_guard([=] { free(fk_def); });<br>
- struct space *child_space =<br>
- space_cache_find_xc(fk_def->child_id);<br>
- struct space *parent_space =<br>
- space_cache_find_xc(fk_def->parent_id);<br>
+ struct space *child_space = space_cache_find(fk_def->child_id);<br>
+ struct space *parent_space = space_cache_find(fk_def->parent_id);<br>
+ if (child_space == NULL or parent_space == NULL)<br>
+ return -1;</div></div></div></div></blockquote>Shall we return after the first error? Will they affect each other?<br><br><blockquote style="border-left:1px solid #0857A6;margin:10px;padding:0 0 0 10px;"><div id=""><div class="js-helper_mailru_css_attribute_postfix js-readmsg-msg_mailru_css_attribute_postfix"><div><div id="style_15692542750458027292_BODY_mailru_css_attribute_postfix"><br></div></div></div></div></blockquote> <br><style></style>
</div></div>
</div>
</div>
</div>
</blockquote>
<br>
<br>-- <br>Ilya Kosarev<br></BODY></HTML>