[PATCH 01/12] alter: introduce CheckSpaceFormat AlterSpaceOp for validating format

[Vladimir Davydov <vdavydov.dev@gmail.com>]

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