From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH 01/12] alter: introduce CheckSpaceFormat AlterSpaceOp for validating format Date: Sat, 7 Apr 2018 16:37:58 +0300 Message-Id: <167afc9c7e0fc57a01d5bfadccf1057bd3ee7416.1523105106.git.vdavydov.dev@gmail.com> In-Reply-To: References: In-Reply-To: References: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: ModifySpaceFormat combines two functions: tuple dictionary update and format check. We don't need to update tuple dictionary when we modify an index, but since we don't have a separate operation for validating tuple format, we issue ModifySpaceFormat. This is confusing. Let's delegate format checking to a new operation CheckSpaceFormat. --- src/box/alter.cc | 50 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index b9d3c2d5..9fea81ad 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -915,7 +915,6 @@ public: ModifySpaceFormat(struct alter_space *alter, struct space_def *new_def) : AlterSpaceOp(alter), new_dict(NULL), old_dict(NULL), new_def(new_def) {} - virtual void alter(struct alter_space *alter); virtual void alter_def(struct alter_space *alter); virtual void commit(struct alter_space *alter, int64_t lsn); virtual ~ModifySpaceFormat(); @@ -937,21 +936,6 @@ ModifySpaceFormat::alter_def(struct alter_space *alter) } void -ModifySpaceFormat::alter(struct alter_space *alter) -{ - struct space *new_space = alter->new_space; - struct space *old_space = alter->old_space; - struct tuple_format *new_format = new_space->format; - struct tuple_format *old_format = old_space->format; - if (old_format != NULL) { - assert(new_format != NULL); - if (! tuple_format1_can_store_format2_tuples(new_format, - old_format)) - space_check_format_xc(new_space, old_space); - } -} - -void ModifySpaceFormat::commit(struct alter_space *alter, int64_t lsn) { (void) alter; @@ -969,6 +953,33 @@ ModifySpaceFormat::~ModifySpaceFormat() } } +/** + * This operation does not modify the space, it just checks that + * tuples stored in it conform to the new format. + */ +class CheckSpaceFormat: public AlterSpaceOp +{ +public: + CheckSpaceFormat(struct alter_space *alter) + :AlterSpaceOp(alter) {} + virtual void alter(struct alter_space *alter); +}; + +void +CheckSpaceFormat::alter(struct alter_space *alter) +{ + struct space *new_space = alter->new_space; + struct space *old_space = alter->old_space; + struct tuple_format *new_format = new_space->format; + struct tuple_format *old_format = old_space->format; + if (old_format != NULL) { + assert(new_format != NULL); + if (!tuple_format1_can_store_format2_tuples(new_format, + old_format)) + space_check_format_xc(new_space, old_space); + } +} + /** Change non-essential properties of a space. */ class ModifySpace: public AlterSpaceOp { @@ -1610,6 +1621,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event) old_space->index_count, def->fields, def->field_count); + (void) new CheckSpaceFormat(alter); (void) new ModifySpaceFormat(alter, def); (void) new ModifySpace(alter, def); def_guard.is_active = false; @@ -1798,10 +1810,12 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event) old_index->def); index_def_guard.is_active = false; } else { - (void) new ModifySpaceFormat(alter, old_space->def); /* - * Operation can be done without index rebuild. + * Operation can be done without index rebuild, + * but we still need to check that tuples stored + * in the space conform to the new format. */ + (void) new CheckSpaceFormat(alter); (void) new ModifyIndex(alter, index_def, old_index->def); index_def_guard.is_active = false; -- 2.11.0