[tarantool-patches] [PATCH] sql: fix recovery of VIEW space

[tarantool-patches] [PATCH] sql: fix recovery of VIEW space

[Nikita Pettik]

During creation of VIEW space, string containing its definition (i.e.
"SELECT ...") is parsed to fetch names of referenced spaces. By those
names real struct space objects are found using schema_find_id().
This function processes lookup in _space using its secondary index.
On the other hand, secondary indexes of _space are unavailable during
this stage of recovery, so this lookup fails and whole recovery process
aborts.

It is worth mentioning that now we can fetch space directly from
in-memory cache using its name (originally, when view reference counter
was introduced, we couldn't do this due to absence of name-cache). So,
to fix this issue, let's use space_by_name() instead of schema_find_id()

Closes #3814
---
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3814-view-update-references-recovery
Issue: https://github.com/tarantool/tarantool/issues/3814

 src/box/alter.cc       | 10 ++--------
 test/sql/view.result   | 28 ++++++++++++++++++++++++++++
 test/sql/view.test.lua | 13 +++++++++++++
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 431da12da..080a72b9f 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1501,13 +1501,8 @@ update_view_references(struct Select *select, int update_value,
 		const char *space_name = sql_src_list_entry_name(list, i);
 		if (space_name == NULL)
 			continue;
-		uint32_t space_id;
-		if (schema_find_id(BOX_SPACE_ID, 2, space_name,
-				   strlen(space_name), &space_id) != 0) {
-			sqlSrcListDelete(sql_get(), list);
-			return -1;
-		}
-		if (space_id == BOX_ID_NIL) {
+		struct space *space = space_by_name(space_name);
+		if (space == NULL) {
 			if (! suppress_error) {
 				assert(not_found_space != NULL);
 				*not_found_space = tt_sprintf("%s", space_name);
@@ -1516,7 +1511,6 @@ update_view_references(struct Select *select, int update_value,
 			}
 			continue;
 		}
-		struct space *space = space_by_id(space_id);
 		assert(space->def->view_ref_count > 0 || update_value > 0);
 		space->def->view_ref_count += update_value;
 	}
diff --git a/test/sql/view.result b/test/sql/view.result
index fd8fe00e3..e99a9bd7e 100644
--- a/test/sql/view.result
+++ b/test/sql/view.result
@@ -241,6 +241,34 @@ box.sql.execute("DROP TABLE c;")
 box.sql.execute("DROP TABLE b;")
 ---
 ...
+-- gh-3814: make sure that recovery of view processed without
+-- unexpected errors.
+--
+box.snapshot()
+---
+- ok
+...
+box.sql.execute("CREATE TABLE t2 (id INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE VIEW v2 AS SELECT * FROM t2;")
+---
+...
+test_run:cmd('restart server default')
+box.sql.execute("DROP TABLE t2;")
+---
+- error: 'Can''t drop space ''T2'': other views depend on this space'
+...
+box.sql.execute("SELECT * FROM v2;")
+---
+- []
+...
+box.space.V2:drop()
+---
+...
+box.space.T2:drop()
+---
+...
 -- Cleanup
 box.sql.execute("DROP VIEW v1;");
 ---
diff --git a/test/sql/view.test.lua b/test/sql/view.test.lua
index d05fb4a8c..592d7889c 100644
--- a/test/sql/view.test.lua
+++ b/test/sql/view.test.lua
@@ -104,6 +104,19 @@ box.space.BCV:drop()
 box.sql.execute("DROP TABLE c;")
 box.sql.execute("DROP TABLE b;")
 
+-- gh-3814: make sure that recovery of view processed without
+-- unexpected errors.
+--
+box.snapshot()
+box.sql.execute("CREATE TABLE t2 (id INT PRIMARY KEY);")
+box.sql.execute("CREATE VIEW v2 AS SELECT * FROM t2;")
+test_run:cmd('restart server default')
+
+box.sql.execute("DROP TABLE t2;")
+box.sql.execute("SELECT * FROM v2;")
+box.space.V2:drop()
+box.space.T2:drop()
+
 -- Cleanup
 box.sql.execute("DROP VIEW v1;");
 box.sql.execute("DROP TABLE t1;");
-- 
2.15.1

[tarantool-patches] Re: [PATCH] sql: fix recovery of VIEW space

[Kirill Yukhin]

Hello,

On 19 Mar 20:21, Nikita Pettik wrote:
> During creation of VIEW space, string containing its definition (i.e.
> "SELECT ...") is parsed to fetch names of referenced spaces. By those
> names real struct space objects are found using schema_find_id().
> This function processes lookup in _space using its secondary index.
> On the other hand, secondary indexes of _space are unavailable during
> this stage of recovery, so this lookup fails and whole recovery process
> aborts.
> 
> It is worth mentioning that now we can fetch space directly from
> in-memory cache using its name (originally, when view reference counter
> was introduced, we couldn't do this due to absence of name-cache). So,
> to fix this issue, let's use space_by_name() instead of schema_find_id()
> 
> Closes #3814
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3814-view-update-references-recovery
> Issue: https://github.com/tarantool/tarantool/issues/3814

Patch is OK. I've cheked it into 2.1 branch.

--
Regards, Kirill Yukhin