[PATCH 10/12] vinyl: remove superfluous ddl checks

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

There's no need to iterate over all indexes in vinyl_space_prepare_alter
and check if any of them is going to be rebuilt - alter_space_do already
does that for us. All we need to do is raise an error if the build_index
callback is invoked for a non-empty space after recovery is complete.
The tuple format check is pointless too, as it is already done by
CheckSpaceFormat AlterSpaceOp. Remove them.
---
 src/box/vinyl.c       | 73 ++++++++++++++++-----------------------------------
 test/box/alter.result |  4 +--
 test/vinyl/ddl.result | 26 +++++++++---------
 test/vinyl/gh.result  |  2 +-
 4 files changed, 38 insertions(+), 67 deletions(-)

diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index 0c475af4..47804ecc 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -976,58 +976,12 @@ vinyl_init_system_space(struct space *space)
 static int
 vinyl_space_prepare_alter(struct space *old_space, struct space *new_space)
 {
+	(void)new_space;
 	struct vy_env *env = vy_env(old_space->engine);
 
 	if (vinyl_check_wal(env, "DDL") != 0)
 		return -1;
-	/*
-	 * The space with no indexes can contain no rows.
-	 * Allow alter.
-	 */
-	if (old_space->index_count == 0)
-		return 0;
-	struct vy_lsm *pk = vy_lsm(old_space->index[0]);
-	/*
-	 * During WAL recovery, the space may be not empty. But we
-	 * open existing indexes, not creating new ones. Allow
-	 * alter.
-	 */
-	if (env->status != VINYL_ONLINE)
-		return 0;
-	/* The space is empty. Allow alter. */
-	if (pk->stat.disk.count.rows == 0 &&
-	    pk->stat.memory.count.rows == 0)
-		return 0;
-	if (old_space->index_count == new_space->index_count) {
-		/* Check index_defs to be unchanged. */
-		for (uint32_t i = 0; i < old_space->index_count; ++i) {
-			struct index_def *old_def, *new_def;
-			old_def = space_index_def(old_space, i);
-			new_def = space_index_def(new_space, i);
-			/*
-			 * We do not support a full rebuild in
-			 * vinyl yet.
-			 */
-			if (index_def_change_requires_rebuild(old_def,
-							      new_def)) {
-				diag_set(ClientError, ER_UNSUPPORTED, "Vinyl",
-					 "changing the definition of "
-					 "a non-empty index");
-				return -1;
-			}
-		}
-	}
-	if (old_space->index_count < new_space->index_count) {
-		diag_set(ClientError, ER_UNSUPPORTED, "Vinyl",
-			 "adding an index to a non-empty space");
-		return -1;
-	}
-	if (! tuple_format1_can_store_format2_tuples(new_space->format,
-						     old_space->format)) {
-		diag_set(ClientError, ER_UNSUPPORTED, "Vinyl",
-			 "changing space format of a non-empty space");
-		return -1;
-	}
+
 	return 0;
 }
 
@@ -1036,7 +990,6 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format)
 {
 	(void)format;
 	struct vy_env *env = vy_env(space->engine);
-	/* @sa vy_prepare_alter_space for checks below. */
 	if (space->index_count == 0)
 		return 0;
 	struct vy_lsm *pk = vy_lsm(space->index[0]);
@@ -1045,7 +998,7 @@ vinyl_space_check_format(struct space *space, struct tuple_format *format)
 	if (pk->stat.disk.count.rows == 0 && pk->stat.memory.count.rows == 0)
 		return 0;
 	diag_set(ClientError, ER_UNSUPPORTED, "Vinyl",
-		 "adding new fields to a non-empty space");
+		 "changing format of a non-empty space");
 	return -1;
 }
 
@@ -1095,6 +1048,24 @@ vinyl_space_build_index(struct space *space, struct index *index,
 			struct tuple_format *format)
 {
 	(void)format;
+
+	struct vy_env *env = vy_env(space->engine);
+	struct vy_lsm *pk = vy_lsm(space->index[0]);
+
+	/*
+	 * During local recovery we are loading existing indexes
+	 * from disk, not building new ones.
+	 */
+	if (env->status != VINYL_INITIAL_RECOVERY_LOCAL &&
+	    env->status != VINYL_FINAL_RECOVERY_LOCAL) {
+		if (pk->stat.disk.count.rows != 0 ||
+		    pk->stat.memory.count.rows != 0) {
+			diag_set(ClientError, ER_UNSUPPORTED, "Vinyl",
+				 "building an index for a non-empty space");
+			return -1;
+		}
+	}
+
 	/*
 	 * Unlike Memtx, Vinyl does not need building of a secondary index.
 	 * This is true because of two things:
@@ -1114,7 +1085,7 @@ vinyl_space_build_index(struct space *space, struct index *index,
 		return -1;
 
 	/* Set pointer to the primary key for the new index. */
-	vy_lsm_update_pk(vy_lsm(index), vy_lsm(space_index(space, 0)));
+	vy_lsm_update_pk(vy_lsm(index), pk);
 	return 0;
 }
 
diff --git a/test/box/alter.result b/test/box/alter.result
index 5e3cbf63..945b0cfd 100644
--- a/test/box/alter.result
+++ b/test/box/alter.result
@@ -1244,7 +1244,7 @@ t[5] = 1
 ...
 box.space._space:replace(t)
 ---
-- error: Vinyl does not support changing space format of a non-empty space
+- error: Vinyl does not support changing format of a non-empty space
 ...
 s:drop()
 ---
@@ -1558,7 +1558,7 @@ format[2] = {name = 'field2', type = 'unsigned'}
 ...
 s:format(format)
 ---
-- error: Vinyl does not support changing space format of a non-empty space
+- error: Vinyl does not support changing format of a non-empty space
 ...
 s:drop()
 ---
diff --git a/test/vinyl/ddl.result b/test/vinyl/ddl.result
index 456977e5..4ea94d36 100644
--- a/test/vinyl/ddl.result
+++ b/test/vinyl/ddl.result
@@ -83,11 +83,11 @@ space:insert({1})
 -- fail because of wrong tuple format {1}, but need {1, ...}
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ---
-- error: Vinyl does not support adding an index to a non-empty space
+- error: Vinyl does not support building an index for a non-empty space
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 #box.space._index:select({space.id})
 ---
@@ -112,11 +112,11 @@ space:insert({1, 2})
 ...
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ---
-- error: Vinyl does not support adding an index to a non-empty space
+- error: Vinyl does not support building an index for a non-empty space
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 #box.space._index:select({space.id})
 ---
@@ -141,11 +141,11 @@ space:insert({1, 2})
 ...
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ---
-- error: Vinyl does not support adding an index to a non-empty space
+- error: Vinyl does not support building an index for a non-empty space
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 #box.space._index:select({space.id})
 ---
@@ -161,11 +161,11 @@ space:delete({1})
 -- must fail because vy_mems have data
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ---
-- error: Vinyl does not support adding an index to a non-empty space
+- error: Vinyl does not support building an index for a non-empty space
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 box.snapshot()
 ---
@@ -232,11 +232,11 @@ while space.index.primary:info().run_count ~= 2 do fiber.sleep(0.01) end
 -- must fail because vy_runs have data
 index2 = space:create_index('secondary', { parts = {2, 'unsigned'} })
 ---
-- error: Vinyl does not support adding an index to a non-empty space
+- error: Vinyl does not support building an index for a non-empty space
 ...
 space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 -- After compaction the REPLACE + DELETE + DELETE = nothing, so
 -- the space is now empty and can be altered.
@@ -291,7 +291,7 @@ space:auto_increment{3}
 ...
 box.space._index:replace{space.id, 0, 'pk', 'tree', {unique=true}, {{0, 'unsigned'}, {1, 'unsigned'}}}
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 space:select{}
 ---
@@ -774,7 +774,7 @@ format[2].is_nullable = false
 ...
 space:format(format)
 ---
-- error: Vinyl does not support changing space format of a non-empty space
+- error: Vinyl does not support changing format of a non-empty space
 ...
 space:drop()
 ---
@@ -832,7 +832,7 @@ s.index.secondary.unique
 ...
 s.index.secondary:alter{unique = true} -- error
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 s.index.secondary.unique
 ---
diff --git a/test/vinyl/gh.result b/test/vinyl/gh.result
index 9c72acc8..47695acb 100644
--- a/test/vinyl/gh.result
+++ b/test/vinyl/gh.result
@@ -144,7 +144,7 @@ s:insert{5, 5}
 ...
 s.index.primary:alter({parts={2,'unsigned'}})
 ---
-- error: Vinyl does not support changing the definition of a non-empty index
+- error: Vinyl does not support building an index for a non-empty space
 ...
 s:drop()
 ---
-- 
2.11.0