From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 924D52582D for ; Tue, 7 Aug 2018 10:48:54 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id sT_0JJ4KXr8e for ; Tue, 7 Aug 2018 10:48:54 -0400 (EDT) Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 2A74821BC3 for ; Tue, 7 Aug 2018 10:48:54 -0400 (EDT) From: Serge Petrenko Subject: [tarantool-patches] [PATCH] Allow nullability mismatch in space indices and format. Date: Tue, 7 Aug 2018 17:48:29 +0300 Message-Id: <20180807144829.34260-1-sergepetrenko@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, 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)