* [tarantool-patches] [PATCH] Allow nullability mismatch in space indices and format. @ 2018-08-07 14:48 Serge Petrenko 2018-08-08 12:50 ` [tarantool-patches] " Vladislav Shpilevoy 0 siblings, 1 reply; 5+ messages in thread From: Serge Petrenko @ 2018-08-07 14:48 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy, 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) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format. 2018-08-07 14:48 [tarantool-patches] [PATCH] Allow nullability mismatch in space indices and format Serge Petrenko @ 2018-08-08 12:50 ` Vladislav Shpilevoy 2018-08-09 7:09 ` Sergey Petrenko 0 siblings, 1 reply; 5+ messages in thread From: Vladislav Shpilevoy @ 2018-08-08 12:50 UTC (permalink / raw) To: Serge Petrenko, tarantool-patches 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. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format. 2018-08-08 12:50 ` [tarantool-patches] " Vladislav Shpilevoy @ 2018-08-09 7:09 ` Sergey Petrenko 2018-08-13 13:42 ` Vladislav Shpilevoy 0 siblings, 1 reply; 5+ messages in thread From: Sergey Petrenko @ 2018-08-09 7:09 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches 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) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format. 2018-08-09 7:09 ` Sergey Petrenko @ 2018-08-13 13:42 ` Vladislav Shpilevoy 2018-08-16 9:37 ` Vladimir Davydov 0 siblings, 1 reply; 5+ messages in thread From: Vladislav Shpilevoy @ 2018-08-13 13:42 UTC (permalink / raw) To: Sergey Petrenko; +Cc: tarantool-patches 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() > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [tarantool-patches] Re: [PATCH] Allow nullability mismatch in space indices and format. 2018-08-13 13:42 ` Vladislav Shpilevoy @ 2018-08-16 9:37 ` Vladimir Davydov 0 siblings, 0 replies; 5+ messages in thread From: Vladimir Davydov @ 2018-08-16 9:37 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: Sergey Petrenko, tarantool-patches Pushed to 1.10 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-08-16 9:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-07 14:48 [tarantool-patches] [PATCH] Allow nullability mismatch in space indices and format Serge Petrenko 2018-08-08 12:50 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-09 7:09 ` Sergey Petrenko 2018-08-13 13:42 ` Vladislav Shpilevoy 2018-08-16 9:37 ` Vladimir Davydov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox