[tarantool-patches] [PATCH] Allow nullability mismatch in space indices and format.

[tarantool-patches] [PATCH] Allow nullability mismatch in space indices and format.

[Serge Petrenko]

Field is_nullable option must be the same in index parts and space
format. This causes problems with altering this option. The only way to
do it is to drop space format, alter nullability in index, and then
reset space format. Not too convenient.
Fix this by allowing different nullability in space format and indices.
This allows to change nullability in space format and index separately.
If at least one of the options is set to false, the resulting
nullability is also set to false.
Modify tests and add a test case.

Closes #3430
---
https://github.com/tarantool/tarantool/issues/3430
https://github.com/tarantool/tarantool/tree/sergepetrenko/gh-3430-allow-nullability-mismatch

 src/box/tuple_format.c    |  11 ++-
 test/engine/ddl.result    |  10 ++-
 test/engine/ddl.test.lua  |   6 +-
 test/engine/null.result   | 205 +++++++++++++++++++++++++++++++++++++---------
 test/engine/null.test.lua |  74 +++++++++++++----
 5 files changed, 239 insertions(+), 67 deletions(-)

diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 2e19d2e3c..a640c23d3 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -84,12 +84,11 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
 			if (part->fieldno >= field_count) {
 				field->is_nullable = part->is_nullable;
 			} else if (field->is_nullable != part->is_nullable) {
-				diag_set(ClientError, ER_NULLABLE_MISMATCH,
-					 part->fieldno + TUPLE_INDEX_BASE,
-					 field->is_nullable ? "nullable" :
-					 "not nullable", part->is_nullable ?
-					 "nullable" : "not nullable");
-				return -1;
+				/* 
+				 * In case of mismatch set the most strict
+				 * option for is_nullable.
+				 */
+				field->is_nullable = false;
 			}
 
 			/*
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 0cb9ed6c2..c8fd8e6f2 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -1431,9 +1431,12 @@ s:create_index('primary', { parts = { 'field1' } })
 ---
 - error: Primary index of the space 'test' can not contain nullable parts
 ...
-s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+-- this is allowed, but the actual part is_nullable stays false
+pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+---
+...
+pk:drop()
 ---
-- error: Field 1 is nullable in space format, but not nullable in index parts
 ...
 format[1].is_nullable = false
 ---
@@ -1462,9 +1465,10 @@ s:insert({1, NULL})
 format[1].is_nullable = true
 ---
 ...
+-- the format is allowed since in primary index parts
+-- is_nullable is still set to false
 s:format(format)
 ---
-- error: Field 1 is nullable in space format, but not nullable in index parts
 ...
 format[1].is_nullable = false
 ---
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index a128f6000..94ae7d5aa 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -534,7 +534,9 @@ format[1] = { name = 'field1', type = 'unsigned', is_nullable = true }
 format[2] = { name = 'field2', type = 'unsigned', is_nullable = true }
 s = box.schema.space.create('test', {engine = engine, format = format})
 s:create_index('primary', { parts = { 'field1' } })
-s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+-- this is allowed, but the actual part is_nullable stays false
+pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+pk:drop()
 format[1].is_nullable = false
 s:format(format)
 s:create_index('primary', { parts = {{'field1', is_nullable = true}} })
@@ -545,6 +547,8 @@ i.parts
 -- Check that is_nullable can't be set to false on non-empty space
 s:insert({1, NULL})
 format[1].is_nullable = true
+-- the format is allowed since in primary index parts
+-- is_nullable is still set to false
 s:format(format)
 format[1].is_nullable = false
 format[2].is_nullable = false
diff --git a/test/engine/null.result b/test/engine/null.result
index 4abf85021..e0ad7df8e 100644
--- a/test/engine/null.result
+++ b/test/engine/null.result
@@ -70,48 +70,8 @@ ok
 ---
 - false
 ...
--- Conflict of is_nullable in format and in parts.
-parts[1].is_nullable = false
----
-...
 sk = s:create_index('sk', { parts = parts }) -- Fail.
 ---
-- error: Field 2 is nullable in space format, but not nullable in index parts
-...
--- Try skip nullable in format and specify in part.
-parts[1].is_nullable = true
----
-...
-sk = s:create_index('sk', { parts = parts }) -- Ok.
----
-...
-format[2].is_nullable = nil
----
-...
-s:format(format) -- Fail.
----
-- error: Field 2 is not nullable in space format, but nullable in index parts
-...
-sk:drop()
----
-...
--- Try to set nullable in part with no format.
-s:format({})
----
-...
-sk = s:create_index('sk', { parts = parts })
----
-...
--- And then set format with no nullable.
-s:format(format) -- Fail.
----
-- error: Field 2 is not nullable in space format, but nullable in index parts
-...
-format[2].is_nullable = true
----
-...
-s:format(format) -- Ok.
----
 ...
 -- Test insert.
 s:insert{1, 1}
@@ -1560,3 +1520,168 @@ i2:select{}
 s:drop()
 ---
 ...
+-- gh-3430 allow different nullability in space format and indices.
+-- resulting field nullability is the strictest of the two.
+s = box.schema.space.create('test', {engine=engine})
+---
+...
+pk = s:create_index('primary', {parts={1, 'unsigned'}})
+---
+...
+sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
+---
+...
+format = {}
+---
+...
+format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
+---
+...
+format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
+---
+...
+s:format(format)
+---
+...
+-- field 2 is not nullable
+s:insert{5}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s:insert{5, nil}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s:insert{5, box.NULL}
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- this is allowed
+---
+...
+-- without space format setting this fails.
+s:insert{5, box.NULL}
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5, nil}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s:insert{5}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+---
+...
+format[2].is_nullable = true
+---
+...
+s:format(format) -- this is also allowed
+---
+...
+-- inserts still fail due to not nullable index parts
+s:insert{5, box.NULL}
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5, nil}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s:insert{5}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}}
+---
+...
+-- now the field really is nullable
+-- success
+s:insert{5, box.NULL}
+---
+- [5, null]
+...
+s:insert{6}
+---
+- [6]
+...
+s:insert{7, 8}
+---
+- [7, 8]
+...
+s:select{}
+---
+- - [5, null]
+  - [6]
+  - [7, 8]
+...
+-- now check we can set nullability to false step by step
+_ = s:delete{5}
+---
+...
+_ = s:delete{6}
+---
+...
+format[2].is_nullable = false
+---
+...
+s:format(format)
+---
+...
+s:insert{5, box.NULL} -- fail
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5} -- fail
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+format[2].is_nullable = true
+---
+...
+s:format(format)
+---
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+---
+...
+s:insert{5, box.NULL} -- fail
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5} -- fail
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+format[2].is_nullable = false
+---
+...
+s:format(format)
+---
+...
+s:select{}
+---
+- - [7, 8]
+...
+s:insert{5} -- fail
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s:insert{9, 10} -- success
+---
+- [9, 10]
+...
+s:drop()
+---
+...
diff --git a/test/engine/null.test.lua b/test/engine/null.test.lua
index 7f5a7dd33..0287dc588 100644
--- a/test/engine/null.test.lua
+++ b/test/engine/null.test.lua
@@ -35,25 +35,8 @@ pk = s:create_index('pk')
 ok = pcall(s.create_index, s, 'sk', { parts = parts, type = 'hash' }) -- Fail.
 ok
 
--- Conflict of is_nullable in format and in parts.
-parts[1].is_nullable = false
 sk = s:create_index('sk', { parts = parts }) -- Fail.
 
--- Try skip nullable in format and specify in part.
-parts[1].is_nullable = true
-sk = s:create_index('sk', { parts = parts }) -- Ok.
-format[2].is_nullable = nil
-s:format(format) -- Fail.
-sk:drop()
-
--- Try to set nullable in part with no format.
-s:format({})
-sk = s:create_index('sk', { parts = parts })
--- And then set format with no nullable.
-s:format(format) -- Fail.
-format[2].is_nullable = true
-s:format(format) -- Ok.
-
 -- Test insert.
 
 s:insert{1, 1}
@@ -461,3 +444,60 @@ i2:select{}
 i2:select{box.NULL, 50}
 i2:select{}
 s:drop()
+
+-- gh-3430 allow different nullability in space format and indices.
+-- resulting field nullability is the strictest of the two.
+s = box.schema.space.create('test', {engine=engine})
+pk = s:create_index('primary', {parts={1, 'unsigned'}})
+sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
+format = {}
+format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
+format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
+s:format(format)
+-- field 2 is not nullable
+s:insert{5}
+s:insert{5, nil}
+s:insert{5, box.NULL}
+
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- this is allowed
+-- without space format setting this fails.
+s:insert{5, box.NULL}
+s:insert{5, nil}
+s:insert{5}
+
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+format[2].is_nullable = true
+s:format(format) -- this is also allowed
+-- inserts still fail due to not nullable index parts
+s:insert{5, box.NULL}
+s:insert{5, nil}
+s:insert{5}
+
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}}
+-- now the field really is nullable
+-- success
+s:insert{5, box.NULL}
+s:insert{6}
+s:insert{7, 8}
+s:select{}
+
+-- now check we can set nullability to false step by step
+_ = s:delete{5}
+_ = s:delete{6}
+
+format[2].is_nullable = false
+s:format(format)
+s:insert{5, box.NULL} -- fail
+s:insert{5} -- fail
+format[2].is_nullable = true
+s:format(format)
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+s:insert{5, box.NULL} -- fail
+s:insert{5} -- fail
+format[2].is_nullable = false
+s:format(format)
+s:select{}
+s:insert{5} -- fail
+s:insert{9, 10} -- success
+
+s:drop()
-- 
2.15.2 (Apple Git-101.1)

[tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format.

[Vladislav Shpilevoy]

Hi! Thanks for the patch! See 8 comments below.

1. Please, prefix the commit title with a subsystem name to
which the patch is related. (Here it is 'box: ' I think).

> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 2e19d2e3c..a640c23d3 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -84,12 +84,11 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
>   			if (part->fieldno >= field_count) {
>   				field->is_nullable = part->is_nullable;
>   			} else if (field->is_nullable != part->is_nullable) {
> -				diag_set(ClientError, ER_NULLABLE_MISMATCH,
> -					 part->fieldno + TUPLE_INDEX_BASE,
> -					 field->is_nullable ? "nullable" :
> -					 "not nullable", part->is_nullable ?
> -					 "nullable" : "not nullable");
> -				return -1;
> +				/*

2. On the branch here a leading white space is. Please, remove.

3. Please, wrap the comment by 66 symbols.

4. ER_NULLABLE_MISMATCH is now unused. Please, replace it with
a placeholder in errcode.h file. (For examples grep ER_UNUSED in
the file).

> +				 * In case of mismatch set the most strict
> +				 * option for is_nullable.
> +				 */
> +				field->is_nullable = false;
>   			}
>   
>   			/*
> diff --git a/test/engine/ddl.result b/test/engine/ddl.result
> index 0cb9ed6c2..c8fd8e6f2 100644
> --- a/test/engine/ddl.result
> +++ b/test/engine/ddl.result
> @@ -1431,9 +1431,12 @@ s:create_index('primary', { parts = { 'field1' } })
>   ---
>   - error: Primary index of the space 'test' can not contain nullable parts
>   ...
> -s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
> +-- this is allowed, but the actual part is_nullable stays false

5. Please, in comments start sentences from a capital letter and finish
with the dot.

> +pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
> +---
> +...
> +pk:drop()
>   ---
> -- error: Field 1 is nullable in space format, but not nullable in index parts
>   ...
>   format[1].is_nullable = false
>   ---
> diff --git a/test/engine/null.result b/test/engine/null.result
> index 4abf85021..e0ad7df8e 100644
> --- a/test/engine/null.result
> +++ b/test/engine/null.result
> @@ -70,48 +70,8 @@ ok
>   ---
>   - false
>   ...
> --- Conflict of is_nullable in format and in parts.
> -parts[1].is_nullable = false
> ----
> -...
>   sk = s:create_index('sk', { parts = parts }) -- Fail.

6. Now the comment is wrong - this line does not fail.

>   ---
> -- error: Field 2 is nullable in space format, but not nullable in index parts
> -...
> --- Try skip nullable in format and specify in part.
> -parts[1].is_nullable = true
> ----
> -...
> -sk = s:create_index('sk', { parts = parts }) -- Ok.
> ----
> -...
> -format[2].is_nullable = nil
> ----
> -...
> -s:format(format) -- Fail.
> ----
> -- error: Field 2 is not nullable in space format, but nullable in index parts
> -...
> -sk:drop()
> ----
> -...
> --- Try to set nullable in part with no format.
> -s:format({})
> ----
> -...
> -sk = s:create_index('sk', { parts = parts })
> ----
> -...
> --- And then set format with no nullable.
> -s:format(format) -- Fail.
> ----
> -- error: Field 2 is not nullable in space format, but nullable in index parts
> -...
> -format[2].is_nullable = true
> ----
> -...
> -s:format(format) -- Ok.
> ----
>   ...
>   -- Test insert.
>   s:insert{1, 1}
> @@ -1560,3 +1520,168 @@ i2:select{}
>   s:drop()
>   ---
>   ...
> +-- gh-3430 allow different nullability in space format and indices.
> +-- resulting field nullability is the strictest of the two.
> +s = box.schema.space.create('test', {engine=engine})
> +---
> +...
> +pk = s:create_index('primary', {parts={1, 'unsigned'}})
> +---
> +...
> +sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
> +---
> +...
> +format = {}
> +---
> +...
> +format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
> +---
> +...
> +format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
> +---
> +...
> +s:format(format)
> +---
> +...
> +-- field 2 is not nullable
> +s:insert{5}
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +s:insert{5, nil}

7. {5, nil} == {5}, so I think you can omit this insertion. It is
covered by the previous one.

8. What if I insert NULL into the space and then set this field to
be not nullable in format or one of indexes? Please, add a test.

[tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format.

[Sergey Petrenko]

Hi! Thank you for review. I fixed your comments.
Please find the new diff below.

> 8 авг. 2018 г., в 15:50, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
> 
> Hi! Thanks for the patch! See 8 comments below.
> 
> 1. Please, prefix the commit title with a subsystem name to
> which the patch is related. (Here it is 'box: ' I think).

Done.

> 
>> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
>> index 2e19d2e3c..a640c23d3 100644
>> --- a/src/box/tuple_format.c
>> +++ b/src/box/tuple_format.c
>> @@ -84,12 +84,11 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
>>  			if (part->fieldno >= field_count) {
>>  				field->is_nullable = part->is_nullable;
>>  			} else if (field->is_nullable != part->is_nullable) {
>> -				diag_set(ClientError, ER_NULLABLE_MISMATCH,
>> -					 part->fieldno + TUPLE_INDEX_BASE,
>> -					 field->is_nullable ? "nullable" :
>> -					 "not nullable", part->is_nullable ?
>> -					 "nullable" : "not nullable");
>> -				return -1;
>> +				/*
> 
> 2. On the branch here a leading white space is. Please, remove.
> 
> 3. Please, wrap the comment by 66 symbols.
> 
> 4. ER_NULLABLE_MISMATCH is now unused. Please, replace it with
> a placeholder in errcode.h file. (For examples grep ER_UNUSED in
> the file).

Done, done, done.

>> +				 * In case of mismatch set the most strict
>> +				 * option for is_nullable.
>> +				 */
>> +				field->is_nullable = false;
>>  			}
>>    			/*
>> diff --git a/test/engine/ddl.result b/test/engine/ddl.result
>> index 0cb9ed6c2..c8fd8e6f2 100644
>> --- a/test/engine/ddl.result
>> +++ b/test/engine/ddl.result
>> @@ -1431,9 +1431,12 @@ s:create_index('primary', { parts = { 'field1' } })
>>  ---
>>  - error: Primary index of the space 'test' can not contain nullable parts
>>  ...
>> -s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
>> +-- this is allowed, but the actual part is_nullable stays false
> 
> 5. Please, in comments start sentences from a capital letter and finish
> with the dot.

Done. 

> 
>> +pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
>> +---
>> +...
>> +pk:drop()
>>  ---
>> -- error: Field 1 is nullable in space format, but not nullable in index parts
>>  ...
>>  format[1].is_nullable = false
>>  ---
>> diff --git a/test/engine/null.result b/test/engine/null.result
>> index 4abf85021..e0ad7df8e 100644
>> --- a/test/engine/null.result
>> +++ b/test/engine/null.result
>> @@ -70,48 +70,8 @@ ok
>>  ---
>>  - false
>>  ...
>> --- Conflict of is_nullable in format and in parts.
>> -parts[1].is_nullable = false
>> ----
>> -...
>>  sk = s:create_index('sk', { parts = parts }) -- Fail.
> 
> 6. Now the comment is wrong - this line does not fail.

Fixed.

> 
>>  ---
>> -- error: Field 2 is nullable in space format, but not nullable in index parts
>> -...
>> --- Try skip nullable in format and specify in part.
>> -parts[1].is_nullable = true
>> ----
>> -...
>> -sk = s:create_index('sk', { parts = parts }) -- Ok.
>> ----
>> -...
>> -format[2].is_nullable = nil
>> ----
>> -...
>> -s:format(format) -- Fail.
>> ----
>> -- error: Field 2 is not nullable in space format, but nullable in index parts
>> -...
>> -sk:drop()
>> ----
>> -...
>> --- Try to set nullable in part with no format.
>> -s:format({})
>> ----
>> -...
>> -sk = s:create_index('sk', { parts = parts })
>> ----
>> -...
>> --- And then set format with no nullable.
>> -s:format(format) -- Fail.
>> ----
>> -- error: Field 2 is not nullable in space format, but nullable in index parts
>> -...
>> -format[2].is_nullable = true
>> ----
>> -...
>> -s:format(format) -- Ok.
>> ----
>>  ...
>>  -- Test insert.
>>  s:insert{1, 1}
>> @@ -1560,3 +1520,168 @@ i2:select{}
>>  s:drop()
>>  ---
>>  ...
>> +-- gh-3430 allow different nullability in space format and indices.
>> +-- resulting field nullability is the strictest of the two.
>> +s = box.schema.space.create('test', {engine=engine})
>> +---
>> +...
>> +pk = s:create_index('primary', {parts={1, 'unsigned'}})
>> +---
>> +...
>> +sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
>> +---
>> +...
>> +format = {}
>> +---
>> +...
>> +format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
>> +---
>> +...
>> +format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
>> +---
>> +...
>> +s:format(format)
>> +---
>> +...
>> +-- field 2 is not nullable
>> +s:insert{5}
>> +---
>> +- error: Tuple field count 1 is less than required by space format or defined indexes
>> +    (expected at least 2)
>> +...
>> +s:insert{5, nil}
> 
> 7. {5, nil} == {5}, so I think you can omit this insertion. It is
> covered by the previous one.

Done.

> 8. What if I insert NULL into the space and then set this field to
> be not nullable in format or one of indexes? Please, add a test.

Added an extensive test, setting in format fails, setting in index fails.

Here’s diff:

 src/box/errcode.h         |   2 +-
 src/box/tuple_format.c    |  12 +--
 test/box/misc.result      |   5 +-
 test/engine/ddl.result    |  10 ++-
 test/engine/ddl.test.lua  |   6 +-
 test/engine/null.result   | 213 +++++++++++++++++++++++++++++++++++++---------
 test/engine/null.test.lua |  78 +++++++++++++----
 7 files changed, 255 insertions(+), 71 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 3d5f66af8..4115e6b65 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -205,7 +205,7 @@ struct errcode_record {
 	/*150 */_(ER_CANT_CREATE_COLLATION,	"Failed to initialize collation: %s.") \
 	/*151 */_(ER_WRONG_COLLATION_OPTIONS,	"Wrong collation options (field %u): %s") \
 	/*152 */_(ER_NULLABLE_PRIMARY,		"Primary index of the space '%s' can not contain nullable parts") \
-	/*153 */_(ER_NULLABLE_MISMATCH,		"Field %d is %s in space format, but %s in index parts") \
+	/*153 */_(ER_UNUSED,			"") \
 	/*154 */_(ER_TRANSACTION_YIELD,		"Transaction has been aborted by a fiber yield") \
 	/*155 */_(ER_NO_SUCH_GROUP,		"Replication group '%s' does not exist") \
 
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 2e19d2e3c..8a008858e 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -84,12 +84,12 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
 			if (part->fieldno >= field_count) {
 				field->is_nullable = part->is_nullable;
 			} else if (field->is_nullable != part->is_nullable) {
-				diag_set(ClientError, ER_NULLABLE_MISMATCH,
-					 part->fieldno + TUPLE_INDEX_BASE,
-					 field->is_nullable ? "nullable" :
-					 "not nullable", part->is_nullable ?
-					 "nullable" : "not nullable");
-				return -1;
+				/*
+				 * In case of mismatch set the
+				 * most strict option for
+				 * is_nullable.
+				 */
+				field->is_nullable = false;
 			}
 
 			/*
diff --git a/test/box/misc.result b/test/box/misc.result
index 4895a78a2..8934e0c87 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -398,13 +398,12 @@ t;
   - 'box.error.UPDATE_ARG_TYPE : 26'
   - 'box.error.CROSS_ENGINE_TRANSACTION : 81'
   - 'box.error.FORMAT_MISMATCH_INDEX_PART : 27'
-  - 'box.error.injection : table: <address>
   - 'box.error.FUNCTION_TX_ACTIVE : 30'
+  - 'box.error.injection : table: <address>
   - 'box.error.ITERATOR_TYPE : 72'
-  - 'box.error.TRANSACTION_YIELD : 154'
   - 'box.error.NO_SUCH_ENGINE : 57'
   - 'box.error.COMMIT_IN_SUB_STMT : 122'
-  - 'box.error.NULLABLE_MISMATCH : 153'
+  - 'box.error.TRANSACTION_YIELD : 154'
   - 'box.error.UNSUPPORTED : 5'
   - 'box.error.LAST_DROP : 15'
   - 'box.error.SPACE_FIELD_IS_DUPLICATE : 149'
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 0cb9ed6c2..c114fb98b 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -1431,9 +1431,12 @@ s:create_index('primary', { parts = { 'field1' } })
 ---
 - error: Primary index of the space 'test' can not contain nullable parts
 ...
-s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+-- This is allowed, but the actual part is_nullable stays false.
+pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+---
+...
+pk:drop()
 ---
-- error: Field 1 is nullable in space format, but not nullable in index parts
 ...
 format[1].is_nullable = false
 ---
@@ -1462,9 +1465,10 @@ s:insert({1, NULL})
 format[1].is_nullable = true
 ---
 ...
+-- The format is allowed since in primary index parts
+-- is_nullable is still set to false.
 s:format(format)
 ---
-- error: Field 1 is nullable in space format, but not nullable in index parts
 ...
 format[1].is_nullable = false
 ---
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index a128f6000..57aa756ab 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -534,7 +534,9 @@ format[1] = { name = 'field1', type = 'unsigned', is_nullable = true }
 format[2] = { name = 'field2', type = 'unsigned', is_nullable = true }
 s = box.schema.space.create('test', {engine = engine, format = format})
 s:create_index('primary', { parts = { 'field1' } })
-s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+-- This is allowed, but the actual part is_nullable stays false.
+pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
+pk:drop()
 format[1].is_nullable = false
 s:format(format)
 s:create_index('primary', { parts = {{'field1', is_nullable = true}} })
@@ -545,6 +547,8 @@ i.parts
 -- Check that is_nullable can't be set to false on non-empty space
 s:insert({1, NULL})
 format[1].is_nullable = true
+-- The format is allowed since in primary index parts
+-- is_nullable is still set to false.
 s:format(format)
 format[1].is_nullable = false
 format[2].is_nullable = false
diff --git a/test/engine/null.result b/test/engine/null.result
index 4abf85021..48d678443 100644
--- a/test/engine/null.result
+++ b/test/engine/null.result
@@ -70,49 +70,9 @@ ok
 ---
 - false
 ...
--- Conflict of is_nullable in format and in parts.
-parts[1].is_nullable = false
----
-...
-sk = s:create_index('sk', { parts = parts }) -- Fail.
----
-- error: Field 2 is nullable in space format, but not nullable in index parts
-...
--- Try skip nullable in format and specify in part.
-parts[1].is_nullable = true
----
-...
-sk = s:create_index('sk', { parts = parts }) -- Ok.
----
-...
-format[2].is_nullable = nil
----
-...
-s:format(format) -- Fail.
----
-- error: Field 2 is not nullable in space format, but nullable in index parts
-...
-sk:drop()
----
-...
--- Try to set nullable in part with no format.
-s:format({})
----
-...
 sk = s:create_index('sk', { parts = parts })
 ---
 ...
--- And then set format with no nullable.
-s:format(format) -- Fail.
----
-- error: Field 2 is not nullable in space format, but nullable in index parts
-...
-format[2].is_nullable = true
----
-...
-s:format(format) -- Ok.
----
-...
 -- Test insert.
 s:insert{1, 1}
 ---
@@ -1560,3 +1520,176 @@ i2:select{}
 s:drop()
 ---
 ...
+-- gh-3430 allow different nullability in space format and indices.
+-- Resulting field nullability is the strictest of the two.
+s = box.schema.space.create('test', {engine=engine})
+---
+...
+pk = s:create_index('primary', {parts={1, 'unsigned'}})
+---
+...
+sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
+---
+...
+format = {}
+---
+...
+format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
+---
+...
+format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
+---
+...
+s:format(format)
+---
+...
+-- Field 2 is not nullable.
+s:insert{5}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s:insert{5, box.NULL}
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- This is allowed.
+---
+...
+-- Without space format setting this fails.
+s:insert{5, box.NULL}
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+---
+...
+format[2].is_nullable = true
+---
+...
+s:format(format) -- This is also allowed.
+---
+...
+-- inserts still fail due to not nullable index parts.
+s:insert{5, box.NULL}
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5}
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}}
+---
+...
+-- Now the field really is nullable.
+-- Success.
+s:insert{5, box.NULL}
+---
+- [5, null]
+...
+s:insert{6}
+---
+- [6]
+...
+s:insert{7, 8}
+---
+- [7, 8]
+...
+s:select{}
+---
+- - [5, null]
+  - [6]
+  - [7, 8]
+...
+-- Check that we cannot set field nullability to false when the
+-- space has tuples with NULL in this field.
+format[2].is_nullable = false
+---
+...
+s:format(format) -- Fail.
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Fail.
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+_ = s:delete{5}
+---
+...
+s:format(format) -- Still fail.
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Still fail.
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+-- Now check we can set nullability to false step by step.
+_ = s:delete{6}
+---
+...
+format[2].is_nullable = false
+---
+...
+s:format(format)
+---
+...
+s:insert{5, box.NULL} -- Fail.
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5} -- Fail.
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+format[2].is_nullable = true
+---
+...
+s:format(format)
+---
+...
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+---
+...
+s:insert{5, box.NULL} -- Fail.
+---
+- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+...
+s:insert{5} -- Fail.
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+format[2].is_nullable = false
+---
+...
+s:format(format)
+---
+...
+s:select{}
+---
+- - [7, 8]
+...
+s:insert{5} -- Fail.
+---
+- error: Tuple field count 1 is less than required by space format or defined indexes
+    (expected at least 2)
+...
+s:insert{9, 10} -- Success.
+---
+- [9, 10]
+...
+s:drop()
+---
+...
diff --git a/test/engine/null.test.lua b/test/engine/null.test.lua
index 7f5a7dd33..5eb068499 100644
--- a/test/engine/null.test.lua
+++ b/test/engine/null.test.lua
@@ -35,24 +35,7 @@ pk = s:create_index('pk')
 ok = pcall(s.create_index, s, 'sk', { parts = parts, type = 'hash' }) -- Fail.
 ok
 
--- Conflict of is_nullable in format and in parts.
-parts[1].is_nullable = false
-sk = s:create_index('sk', { parts = parts }) -- Fail.
-
--- Try skip nullable in format and specify in part.
-parts[1].is_nullable = true
-sk = s:create_index('sk', { parts = parts }) -- Ok.
-format[2].is_nullable = nil
-s:format(format) -- Fail.
-sk:drop()
-
--- Try to set nullable in part with no format.
-s:format({})
 sk = s:create_index('sk', { parts = parts })
--- And then set format with no nullable.
-s:format(format) -- Fail.
-format[2].is_nullable = true
-s:format(format) -- Ok.
 
 -- Test insert.
 
@@ -461,3 +444,64 @@ i2:select{}
 i2:select{box.NULL, 50}
 i2:select{}
 s:drop()
+
+-- gh-3430 allow different nullability in space format and indices.
+-- Resulting field nullability is the strictest of the two.
+s = box.schema.space.create('test', {engine=engine})
+pk = s:create_index('primary', {parts={1, 'unsigned'}})
+sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
+format = {}
+format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
+format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
+s:format(format)
+-- Field 2 is not nullable.
+s:insert{5}
+s:insert{5, box.NULL}
+
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- This is allowed.
+-- Without space format setting this fails.
+s:insert{5, box.NULL}
+s:insert{5}
+
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+format[2].is_nullable = true
+s:format(format) -- This is also allowed.
+-- inserts still fail due to not nullable index parts.
+s:insert{5, box.NULL}
+s:insert{5}
+
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}}
+-- Now the field really is nullable.
+-- Success.
+s:insert{5, box.NULL}
+s:insert{6}
+s:insert{7, 8}
+s:select{}
+
+-- Check that we cannot set field nullability to false when the
+-- space has tuples with NULL in this field.
+format[2].is_nullable = false
+s:format(format) -- Fail.
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Fail.
+_ = s:delete{5}
+s:format(format) -- Still fail.
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Still fail.
+-- Now check we can set nullability to false step by step.
+_ = s:delete{6}
+
+format[2].is_nullable = false
+s:format(format)
+s:insert{5, box.NULL} -- Fail.
+s:insert{5} -- Fail.
+format[2].is_nullable = true
+s:format(format)
+s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
+s:insert{5, box.NULL} -- Fail.
+s:insert{5} -- Fail.
+format[2].is_nullable = false
+s:format(format)
+s:select{}
+s:insert{5} -- Fail.
+s:insert{9, 10} -- Success.
+
+s:drop()
-- 
2.15.2 (Apple Git-101.1)

[tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format.

[Vladislav Shpilevoy]

Hi! Thanks for the fixes! LGTM.

On 09/08/2018 10:09, Sergey Petrenko wrote:
> Hi! Thank you for review. I fixed your comments.
> Please find the new diff below.
> 
>> 8 авг. 2018 г., в 15:50, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
>>
>> Hi! Thanks for the patch! See 8 comments below.
>>
>> 1. Please, prefix the commit title with a subsystem name to
>> which the patch is related. (Here it is 'box: ' I think).
> 
> Done.
> 
>>
>>> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
>>> index 2e19d2e3c..a640c23d3 100644
>>> --- a/src/box/tuple_format.c
>>> +++ b/src/box/tuple_format.c
>>> @@ -84,12 +84,11 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
>>>   			if (part->fieldno >= field_count) {
>>>   				field->is_nullable = part->is_nullable;
>>>   			} else if (field->is_nullable != part->is_nullable) {
>>> -				diag_set(ClientError, ER_NULLABLE_MISMATCH,
>>> -					 part->fieldno + TUPLE_INDEX_BASE,
>>> -					 field->is_nullable ? "nullable" :
>>> -					 "not nullable", part->is_nullable ?
>>> -					 "nullable" : "not nullable");
>>> -				return -1;
>>> +				/*
>>
>> 2. On the branch here a leading white space is. Please, remove.
>>
>> 3. Please, wrap the comment by 66 symbols.
>>
>> 4. ER_NULLABLE_MISMATCH is now unused. Please, replace it with
>> a placeholder in errcode.h file. (For examples grep ER_UNUSED in
>> the file).
> 
> Done, done, done.
> 
>>> +				 * In case of mismatch set the most strict
>>> +				 * option for is_nullable.
>>> +				 */
>>> +				field->is_nullable = false;
>>>   			}
>>>     			/*
>>> diff --git a/test/engine/ddl.result b/test/engine/ddl.result
>>> index 0cb9ed6c2..c8fd8e6f2 100644
>>> --- a/test/engine/ddl.result
>>> +++ b/test/engine/ddl.result
>>> @@ -1431,9 +1431,12 @@ s:create_index('primary', { parts = { 'field1' } })
>>>   ---
>>>   - error: Primary index of the space 'test' can not contain nullable parts
>>>   ...
>>> -s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
>>> +-- this is allowed, but the actual part is_nullable stays false
>>
>> 5. Please, in comments start sentences from a capital letter and finish
>> with the dot.
> 
> Done.
> 
>>
>>> +pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
>>> +---
>>> +...
>>> +pk:drop()
>>>   ---
>>> -- error: Field 1 is nullable in space format, but not nullable in index parts
>>>   ...
>>>   format[1].is_nullable = false
>>>   ---
>>> diff --git a/test/engine/null.result b/test/engine/null.result
>>> index 4abf85021..e0ad7df8e 100644
>>> --- a/test/engine/null.result
>>> +++ b/test/engine/null.result
>>> @@ -70,48 +70,8 @@ ok
>>>   ---
>>>   - false
>>>   ...
>>> --- Conflict of is_nullable in format and in parts.
>>> -parts[1].is_nullable = false
>>> ----
>>> -...
>>>   sk = s:create_index('sk', { parts = parts }) -- Fail.
>>
>> 6. Now the comment is wrong - this line does not fail.
> 
> Fixed.
> 
>>
>>>   ---
>>> -- error: Field 2 is nullable in space format, but not nullable in index parts
>>> -...
>>> --- Try skip nullable in format and specify in part.
>>> -parts[1].is_nullable = true
>>> ----
>>> -...
>>> -sk = s:create_index('sk', { parts = parts }) -- Ok.
>>> ----
>>> -...
>>> -format[2].is_nullable = nil
>>> ----
>>> -...
>>> -s:format(format) -- Fail.
>>> ----
>>> -- error: Field 2 is not nullable in space format, but nullable in index parts
>>> -...
>>> -sk:drop()
>>> ----
>>> -...
>>> --- Try to set nullable in part with no format.
>>> -s:format({})
>>> ----
>>> -...
>>> -sk = s:create_index('sk', { parts = parts })
>>> ----
>>> -...
>>> --- And then set format with no nullable.
>>> -s:format(format) -- Fail.
>>> ----
>>> -- error: Field 2 is not nullable in space format, but nullable in index parts
>>> -...
>>> -format[2].is_nullable = true
>>> ----
>>> -...
>>> -s:format(format) -- Ok.
>>> ----
>>>   ...
>>>   -- Test insert.
>>>   s:insert{1, 1}
>>> @@ -1560,3 +1520,168 @@ i2:select{}
>>>   s:drop()
>>>   ---
>>>   ...
>>> +-- gh-3430 allow different nullability in space format and indices.
>>> +-- resulting field nullability is the strictest of the two.
>>> +s = box.schema.space.create('test', {engine=engine})
>>> +---
>>> +...
>>> +pk = s:create_index('primary', {parts={1, 'unsigned'}})
>>> +---
>>> +...
>>> +sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
>>> +---
>>> +...
>>> +format = {}
>>> +---
>>> +...
>>> +format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
>>> +---
>>> +...
>>> +format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
>>> +---
>>> +...
>>> +s:format(format)
>>> +---
>>> +...
>>> +-- field 2 is not nullable
>>> +s:insert{5}
>>> +---
>>> +- error: Tuple field count 1 is less than required by space format or defined indexes
>>> +    (expected at least 2)
>>> +...
>>> +s:insert{5, nil}
>>
>> 7. {5, nil} == {5}, so I think you can omit this insertion. It is
>> covered by the previous one.
> 
> Done.
> 
>> 8. What if I insert NULL into the space and then set this field to
>> be not nullable in format or one of indexes? Please, add a test.
> 
> Added an extensive test, setting in format fails, setting in index fails.
> 
> Here’s diff:
> 
>   src/box/errcode.h         |   2 +-
>   src/box/tuple_format.c    |  12 +--
>   test/box/misc.result      |   5 +-
>   test/engine/ddl.result    |  10 ++-
>   test/engine/ddl.test.lua  |   6 +-
>   test/engine/null.result   | 213 +++++++++++++++++++++++++++++++++++++---------
>   test/engine/null.test.lua |  78 +++++++++++++----
>   7 files changed, 255 insertions(+), 71 deletions(-)
> 
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 3d5f66af8..4115e6b65 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -205,7 +205,7 @@ struct errcode_record {
>   	/*150 */_(ER_CANT_CREATE_COLLATION,	"Failed to initialize collation: %s.") \
>   	/*151 */_(ER_WRONG_COLLATION_OPTIONS,	"Wrong collation options (field %u): %s") \
>   	/*152 */_(ER_NULLABLE_PRIMARY,		"Primary index of the space '%s' can not contain nullable parts") \
> -	/*153 */_(ER_NULLABLE_MISMATCH,		"Field %d is %s in space format, but %s in index parts") \
> +	/*153 */_(ER_UNUSED,			"") \
>   	/*154 */_(ER_TRANSACTION_YIELD,		"Transaction has been aborted by a fiber yield") \
>   	/*155 */_(ER_NO_SUCH_GROUP,		"Replication group '%s' does not exist") \
>   
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 2e19d2e3c..8a008858e 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -84,12 +84,12 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
>   			if (part->fieldno >= field_count) {
>   				field->is_nullable = part->is_nullable;
>   			} else if (field->is_nullable != part->is_nullable) {
> -				diag_set(ClientError, ER_NULLABLE_MISMATCH,
> -					 part->fieldno + TUPLE_INDEX_BASE,
> -					 field->is_nullable ? "nullable" :
> -					 "not nullable", part->is_nullable ?
> -					 "nullable" : "not nullable");
> -				return -1;
> +				/*
> +				 * In case of mismatch set the
> +				 * most strict option for
> +				 * is_nullable.
> +				 */
> +				field->is_nullable = false;
>   			}
>   
>   			/*
> diff --git a/test/box/misc.result b/test/box/misc.result
> index 4895a78a2..8934e0c87 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -398,13 +398,12 @@ t;
>     - 'box.error.UPDATE_ARG_TYPE : 26'
>     - 'box.error.CROSS_ENGINE_TRANSACTION : 81'
>     - 'box.error.FORMAT_MISMATCH_INDEX_PART : 27'
> -  - 'box.error.injection : table: <address>
>     - 'box.error.FUNCTION_TX_ACTIVE : 30'
> +  - 'box.error.injection : table: <address>
>     - 'box.error.ITERATOR_TYPE : 72'
> -  - 'box.error.TRANSACTION_YIELD : 154'
>     - 'box.error.NO_SUCH_ENGINE : 57'
>     - 'box.error.COMMIT_IN_SUB_STMT : 122'
> -  - 'box.error.NULLABLE_MISMATCH : 153'
> +  - 'box.error.TRANSACTION_YIELD : 154'
>     - 'box.error.UNSUPPORTED : 5'
>     - 'box.error.LAST_DROP : 15'
>     - 'box.error.SPACE_FIELD_IS_DUPLICATE : 149'
> diff --git a/test/engine/ddl.result b/test/engine/ddl.result
> index 0cb9ed6c2..c114fb98b 100644
> --- a/test/engine/ddl.result
> +++ b/test/engine/ddl.result
> @@ -1431,9 +1431,12 @@ s:create_index('primary', { parts = { 'field1' } })
>   ---
>   - error: Primary index of the space 'test' can not contain nullable parts
>   ...
> -s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
> +-- This is allowed, but the actual part is_nullable stays false.
> +pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
> +---
> +...
> +pk:drop()
>   ---
> -- error: Field 1 is nullable in space format, but not nullable in index parts
>   ...
>   format[1].is_nullable = false
>   ---
> @@ -1462,9 +1465,10 @@ s:insert({1, NULL})
>   format[1].is_nullable = true
>   ---
>   ...
> +-- The format is allowed since in primary index parts
> +-- is_nullable is still set to false.
>   s:format(format)
>   ---
> -- error: Field 1 is nullable in space format, but not nullable in index parts
>   ...
>   format[1].is_nullable = false
>   ---
> diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
> index a128f6000..57aa756ab 100644
> --- a/test/engine/ddl.test.lua
> +++ b/test/engine/ddl.test.lua
> @@ -534,7 +534,9 @@ format[1] = { name = 'field1', type = 'unsigned', is_nullable = true }
>   format[2] = { name = 'field2', type = 'unsigned', is_nullable = true }
>   s = box.schema.space.create('test', {engine = engine, format = format})
>   s:create_index('primary', { parts = { 'field1' } })
> -s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
> +-- This is allowed, but the actual part is_nullable stays false.
> +pk = s:create_index('primary', { parts = {{'field1', is_nullable = false}} })
> +pk:drop()
>   format[1].is_nullable = false
>   s:format(format)
>   s:create_index('primary', { parts = {{'field1', is_nullable = true}} })
> @@ -545,6 +547,8 @@ i.parts
>   -- Check that is_nullable can't be set to false on non-empty space
>   s:insert({1, NULL})
>   format[1].is_nullable = true
> +-- The format is allowed since in primary index parts
> +-- is_nullable is still set to false.
>   s:format(format)
>   format[1].is_nullable = false
>   format[2].is_nullable = false
> diff --git a/test/engine/null.result b/test/engine/null.result
> index 4abf85021..48d678443 100644
> --- a/test/engine/null.result
> +++ b/test/engine/null.result
> @@ -70,49 +70,9 @@ ok
>   ---
>   - false
>   ...
> --- Conflict of is_nullable in format and in parts.
> -parts[1].is_nullable = false
> ----
> -...
> -sk = s:create_index('sk', { parts = parts }) -- Fail.
> ----
> -- error: Field 2 is nullable in space format, but not nullable in index parts
> -...
> --- Try skip nullable in format and specify in part.
> -parts[1].is_nullable = true
> ----
> -...
> -sk = s:create_index('sk', { parts = parts }) -- Ok.
> ----
> -...
> -format[2].is_nullable = nil
> ----
> -...
> -s:format(format) -- Fail.
> ----
> -- error: Field 2 is not nullable in space format, but nullable in index parts
> -...
> -sk:drop()
> ----
> -...
> --- Try to set nullable in part with no format.
> -s:format({})
> ----
> -...
>   sk = s:create_index('sk', { parts = parts })
>   ---
>   ...
> --- And then set format with no nullable.
> -s:format(format) -- Fail.
> ----
> -- error: Field 2 is not nullable in space format, but nullable in index parts
> -...
> -format[2].is_nullable = true
> ----
> -...
> -s:format(format) -- Ok.
> ----
> -...
>   -- Test insert.
>   s:insert{1, 1}
>   ---
> @@ -1560,3 +1520,176 @@ i2:select{}
>   s:drop()
>   ---
>   ...
> +-- gh-3430 allow different nullability in space format and indices.
> +-- Resulting field nullability is the strictest of the two.
> +s = box.schema.space.create('test', {engine=engine})
> +---
> +...
> +pk = s:create_index('primary', {parts={1, 'unsigned'}})
> +---
> +...
> +sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
> +---
> +...
> +format = {}
> +---
> +...
> +format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
> +---
> +...
> +format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
> +---
> +...
> +s:format(format)
> +---
> +...
> +-- Field 2 is not nullable.
> +s:insert{5}
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +s:insert{5, box.NULL}
> +---
> +- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
> +...
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- This is allowed.
> +---
> +...
> +-- Without space format setting this fails.
> +s:insert{5, box.NULL}
> +---
> +- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
> +...
> +s:insert{5}
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
> +---
> +...
> +format[2].is_nullable = true
> +---
> +...
> +s:format(format) -- This is also allowed.
> +---
> +...
> +-- inserts still fail due to not nullable index parts.
> +s:insert{5, box.NULL}
> +---
> +- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
> +...
> +s:insert{5}
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}}
> +---
> +...
> +-- Now the field really is nullable.
> +-- Success.
> +s:insert{5, box.NULL}
> +---
> +- [5, null]
> +...
> +s:insert{6}
> +---
> +- [6]
> +...
> +s:insert{7, 8}
> +---
> +- [7, 8]
> +...
> +s:select{}
> +---
> +- - [5, null]
> +  - [6]
> +  - [7, 8]
> +...
> +-- Check that we cannot set field nullability to false when the
> +-- space has tuples with NULL in this field.
> +format[2].is_nullable = false
> +---
> +...
> +s:format(format) -- Fail.
> +---
> +- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
> +...
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Fail.
> +---
> +- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
> +...
> +_ = s:delete{5}
> +---
> +...
> +s:format(format) -- Still fail.
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Still fail.
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +-- Now check we can set nullability to false step by step.
> +_ = s:delete{6}
> +---
> +...
> +format[2].is_nullable = false
> +---
> +...
> +s:format(format)
> +---
> +...
> +s:insert{5, box.NULL} -- Fail.
> +---
> +- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
> +...
> +s:insert{5} -- Fail.
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +format[2].is_nullable = true
> +---
> +...
> +s:format(format)
> +---
> +...
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
> +---
> +...
> +s:insert{5, box.NULL} -- Fail.
> +---
> +- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
> +...
> +s:insert{5} -- Fail.
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +format[2].is_nullable = false
> +---
> +...
> +s:format(format)
> +---
> +...
> +s:select{}
> +---
> +- - [7, 8]
> +...
> +s:insert{5} -- Fail.
> +---
> +- error: Tuple field count 1 is less than required by space format or defined indexes
> +    (expected at least 2)
> +...
> +s:insert{9, 10} -- Success.
> +---
> +- [9, 10]
> +...
> +s:drop()
> +---
> +...
> diff --git a/test/engine/null.test.lua b/test/engine/null.test.lua
> index 7f5a7dd33..5eb068499 100644
> --- a/test/engine/null.test.lua
> +++ b/test/engine/null.test.lua
> @@ -35,24 +35,7 @@ pk = s:create_index('pk')
>   ok = pcall(s.create_index, s, 'sk', { parts = parts, type = 'hash' }) -- Fail.
>   ok
>   
> --- Conflict of is_nullable in format and in parts.
> -parts[1].is_nullable = false
> -sk = s:create_index('sk', { parts = parts }) -- Fail.
> -
> --- Try skip nullable in format and specify in part.
> -parts[1].is_nullable = true
> -sk = s:create_index('sk', { parts = parts }) -- Ok.
> -format[2].is_nullable = nil
> -s:format(format) -- Fail.
> -sk:drop()
> -
> --- Try to set nullable in part with no format.
> -s:format({})
>   sk = s:create_index('sk', { parts = parts })
> --- And then set format with no nullable.
> -s:format(format) -- Fail.
> -format[2].is_nullable = true
> -s:format(format) -- Ok.
>   
>   -- Test insert.
>   
> @@ -461,3 +444,64 @@ i2:select{}
>   i2:select{box.NULL, 50}
>   i2:select{}
>   s:drop()
> +
> +-- gh-3430 allow different nullability in space format and indices.
> +-- Resulting field nullability is the strictest of the two.
> +s = box.schema.space.create('test', {engine=engine})
> +pk = s:create_index('primary', {parts={1, 'unsigned'}})
> +sk = s:create_index('secondary', {parts={2, 'unsigned', is_nullable=false}})
> +format = {}
> +format[1] = {name = 'first', type = 'unsigned', is_nullable = false}
> +format[2] = {name = 'second', type = 'unsigned', is_nullable = false}
> +s:format(format)
> +-- Field 2 is not nullable.
> +s:insert{5}
> +s:insert{5, box.NULL}
> +
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- This is allowed.
> +-- Without space format setting this fails.
> +s:insert{5, box.NULL}
> +s:insert{5}
> +
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
> +format[2].is_nullable = true
> +s:format(format) -- This is also allowed.
> +-- inserts still fail due to not nullable index parts.
> +s:insert{5, box.NULL}
> +s:insert{5}
> +
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}}
> +-- Now the field really is nullable.
> +-- Success.
> +s:insert{5, box.NULL}
> +s:insert{6}
> +s:insert{7, 8}
> +s:select{}
> +
> +-- Check that we cannot set field nullability to false when the
> +-- space has tuples with NULL in this field.
> +format[2].is_nullable = false
> +s:format(format) -- Fail.
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Fail.
> +_ = s:delete{5}
> +s:format(format) -- Still fail.
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Still fail.
> +-- Now check we can set nullability to false step by step.
> +_ = s:delete{6}
> +
> +format[2].is_nullable = false
> +s:format(format)
> +s:insert{5, box.NULL} -- Fail.
> +s:insert{5} -- Fail.
> +format[2].is_nullable = true
> +s:format(format)
> +s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
> +s:insert{5, box.NULL} -- Fail.
> +s:insert{5} -- Fail.
> +format[2].is_nullable = false
> +s:format(format)
> +s:select{}
> +s:insert{5} -- Fail.
> +s:insert{9, 10} -- Success.
> +
> +s:drop()
> 

Re: [tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format.

[Vladimir Davydov]

Pushed to 1.10