

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