[patches] [PATCH 1/4] memtx: check for new optional fields on format update

[v.shpilevoy at tarantool.org]

> 
> Please don't mention GitHub tickets in the source code (tests are OK)
> unless absolutely necessary. The code should be self-explanatory as is
> and shouldn't require any references to external resources.

Ok, fixed on a branch:
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 8a62d8ed8..22eecd8a5 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1584,7 +1584,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
                 * format covers some nullable indexed fields in
                 * the format tail. And when the format is reset,
                 * these fields become optional - index
-                * comparators must be updated. See gh-3229.
+                * comparators must be updated.
                 */
                struct key_def **keys;
                size_t bsize = old_space->index_count * sizeof(keys[0]);

> 
> We have a similar logic in on_replace_dd_index(). Can't we move it,
> along with all those numerous calls to index_def_update_optionality()
> spread over alter.cc, to a single place to avoid code duplication?
> alter_space_do() or alter_def(), may be? AFAIR I suggested to do that
> when I reviewed the original patch set, but you just said that you
> disagreed without any further elaboration...

No, this logic can not be moved into alter_space_do or another common alter function, because
1) optionality update must be done before alter_space_move_indexes, in which new min field count is used to decide, do we need move an index, or modify it. Alter_def is later;
2) alter_space_do can not do optionality update, because it would be encapsulation violation. Alter space do just executes a sequence of virtual functions. It must not be updated, when _sequence, _collation and other system spaces are changed.