[patches] [PATCH 4/4] Allow to do not specify tail nullable index columns

[Vladimir Davydov]

On Sat, Feb 17, 2018 at 11:38:57PM +0300, v.shpilevoy at tarantool.org wrote:
> 
> 
> С уважением, Владислав Шпилевой.
> 
> > 17 февр. 2018 г., в 23:19, Vladimir Davydov <vdavydov.dev at gmail.com> написал(а):
> > 
> >> On Tue, Feb 13, 2018 at 06:28:38PM +0300, Vladislav Shpilevoy wrote:
> >> diff --git a/src/box/alter.cc b/src/box/alter.cc
> >> index c48e89f9e..cb3b8353d 100644
> >> --- a/src/box/alter.cc
> >> +++ b/src/box/alter.cc
> >> @@ -323,7 +323,39 @@ index_def_new_from_tuple(struct tuple *tuple, struct space *space)
> >>                         space->def->field_count) != 0)
> >>            diag_raise();
> >>    }
> >> -    key_def = key_def_new_with_parts(part_def, part_count);
> >> +    struct space_def *sd = space->def;
> >> +    int unchanged_index_count = 0;
> >> +    struct key_def **keys = NULL;
> >> +    /*
> >> +     * To detect which key parts are optional, min_field_count
> >> +     * is required. But min_field_count from the old space
> >> +     * format can not be used. For example, consider the case,
> >> +     * when a space has no format, has a primary index on the
> >> +     * first field and has a single secondary index on a
> >> +     * non-nullable second field. Min field count here is 2.
> >> +     * Now alter the secondary index to make its part be
> >> +     * nullable. The 'space' variable here is the old space,
> >> +     * where min_field_count is still 2, but actually it is
> >> +     * already 1. Actual min_field_count must be calculated
> >> +     * using old unchanged indexes, NEW definition of an
> >> +     * updated index and a space format, defined by a user.
> >> +     */
> >> +    for (uint32_t i = 0; i < space->index_count; ++i) {
> >> +        if (space->index[i]->def->iid != index_id)
> >> +            ++unchanged_index_count;
> >> +    }
> >> +    if (unchanged_index_count > 0) {
> >> +        size_t bsize = unchanged_index_count * sizeof(keys[0]);
> >> +        keys = (struct key_def **) region_alloc_xc(&fiber()->gc, bsize);
> >> +        for (uint32_t i = 0, j = 0; i < space->index_count; ++i) {
> >> +            if (space->index[i]->def->iid != index_id)
> >> +                keys[j++] = space->index[i]->def->key_def;
> >> +        }
> >> +    }
> >> +    uint32_t min_field_count =
> >> +        tuple_format_min_field_count(keys, unchanged_index_count,
> >> +                         sd->fields, sd->field_count);
> >> +    key_def = key_def_new_with_parts(part_def, part_count, min_field_count);
> > 
> > What will happen if there are two indexes over the same field, nullable
> > and not nullable, and the one which is not nullable is set to be
> > nullable? I don't see where you update is_optional then.
> 
> Is_optional in such a case is set due to decreased min_field_count.

Well, I tried to check this, but got a crash:

  vlad at esperanza test$ cat test.lua
  box.cfg{log_level = 2}
  
  s = box.schema.space.create('test')
  s:create_index('pk')
  i1 = s:create_index('i1', {parts = {2, 'unsigned', 3, 'unsigned'}})
  i2 = s:create_index('i2', {parts = {3, 'unsigned', 2, 'unsigned'}})
  
  i1:alter{parts = {{2, 'unsigned'}, {3, 'unsigned', is_nullable = true}}}
  i2:alter{parts = {{3, 'unsigned', is_nullable = true}, {2, 'unsigned'}}}
  
  s:insert{1, 1}
  vlad at esperanza test$ tarantool test.lua
  tarantool: /home/vlad/src/tarantool/src/box/tuple_format.c:94: tuple_format_create: Assertion `part->fieldno + 1 <= format->min_field_count || part->is_optional' failed.
  Aborted

> 
> > 
> > AFAIU is_optional should be updated for all indexes in alter_space_do()
> > or in AlterSpaceOps, not here.
> 
> I disagree.