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

[Serge Petrenko <sergepetrenko@tarantool.org>]

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)