On 18 Jun 20:57, Ilya Kosarev wrote:
> Since e1d3fe8ab8eed65394ad17409401a93b6fcdc435 (tuple format: don't
> allow null where array/map is expected) tuple fields are nullable by
> default. It seems strange at least in case we have implicit fields in
> front of explicit nullable field. Now fields are nullable by default
> except arrays & maps due to reasons specified in mentioned commit.
>
> Closes #5027
> ---
I believe your patch indeed fixes the problem. But I have
no idea how it is connected with map/array fields, since
in the test case field type is unsigned. Could you please
provide more info concerning fix&problem topic?
> Branch:
https://github.com/tarantool/tarantool/tree/i.kosarev/gh-5027-fix-tuple-fields-nullability> Issue:
https://github.com/tarantool/tarantool/issues/5027>
> @ChangeLog:
> * Fix confusing implicit requirements for tuple fields.
>
> src/box/tuple_format.c | 6 +++++-
> test/engine/insert.result | 33 +++++++++++++++++++++++++++++++++
> test/engine/insert.test.lua | 12 ++++++++++++
> 3 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 68ec2a7499..4b8f32149e 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -152,7 +152,7 @@ tuple_field_new(void)
> field->type = FIELD_TYPE_ANY;
> field->offset_slot = TUPLE_OFFSET_SLOT_NIL;
> field->coll_id = COLL_NONE;
> - field->nullable_action = ON_CONFLICT_ACTION_DEFAULT;
> + field->nullable_action = ON_CONFLICT_ACTION_NONE;
> field->multikey_required_fields = NULL;
> return field;
> }
> @@ -528,6 +528,10 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
> multikey_required_fields;
> required_fields = multikey_required_fields;
> }
This snippet definitely lacks good comment...
> + if ((field->type == FIELD_TYPE_ARRAY ||
> + field->type == FIELD_TYPE_MAP) &&
> + tuple_field_is_nullable(field))
> + field->nullable_action = ON_CONFLICT_ACTION_DEFAULT;
> /*
> * Mark all leaf non-nullable fields as required
> * by setting the corresponding bit in the bitmap
> diff --git a/test/engine/insert.result b/test/engine/insert.result
> index 13ffe8ceb1..37a4e8cc68 100644
> --- a/test/engine/insert.result
> +++ b/test/engine/insert.result
> @@ -988,3 +988,36 @@ s:select()
> s:drop()
> ---
> ...
> +-- gh-5027: Do not require extra fields
AFAIR we have agreement that regression tests are extracted
in a separate file.
> +s = box.schema.space.create('test')
> +---
> +...
> +i1 = s:create_index('i1', {parts={{1, 'unsigned'}}})
> +---
> +...
> +i2 = s:create_index('i2', {parts={{5, 'unsigned', is_nullable=true}}})
> +---
> +...
> +s:insert{1, 2, 3}
Why not use test case right from the issue?
> +s:insert{1, 2, 3}
> +---
> +- error: Tuple field 5 required by space format is missing
> +...
> +s:drop()
> +---
> +...
> diff --git a/test/engine/insert.test.lua b/test/engine/insert.test.lua
> index 2ffc8cd0a0..112ca62c53 100644
> --- a/test/engine/insert.test.lua
> +++ b/test/engine/insert.test.lua
> @@ -210,3 +210,15 @@ ddd:delete(ffi.cast('double', 123))
> s:select()
>
> s:drop()
> +
> +-- gh-5027: Do not require extra fields
> +s = box.schema.space.create('test')
> +i1 = s:create_index('i1', {parts={{1, 'unsigned'}}})
> +i2 = s:create_index('i2', {parts={{5, 'unsigned', is_nullable=true}}})
> +s:insert{1, 2, 3}
> +s:drop()
> +s = box.schema.space.create('test')
> +i1 = s:create_index('i1', {parts={{1, 'unsigned'}}})
> +i2 = s:create_index('i2', {parts={{5, 'unsigned', is_nullable=false}}})
> +s:insert{1, 2, 3}
> +s:drop()
> \ No newline at end of file
Let's put newline, this message looks annoying.