Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH] sql: increment VIEW counter for tables within sub-select
@ 2018-12-03 15:00 Nikita Pettik
  2018-12-03 16:19 ` [tarantool-patches] " Vladislav Shpilevoy
  2018-12-14 10:34 ` Kirill Yukhin
  0 siblings, 2 replies; 3+ messages in thread
From: Nikita Pettik @ 2018-12-03 15:00 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik

VIEW can be created not only in form of AS SELECT but also in form of AS
VALUES (...). In the latter case space's view reference counters were
not incremented. Furthermore, if view came with sub-select, reference
counters of tables within sub-select were not updated as well. These
problems occurred due to simplified traverse over AST: only tables
within FROM clause were accounted. This patch fixes them introducing
complete pass over AST using walker.

Closes #3815
---
Branch: https://github.com/tarantool/tarantool/commits/np/gh-3815-drop-view-as-values
Issue: https://github.com/tarantool/tarantool/issues/3815

 src/box/alter.cc        | 18 +++++++----
 src/box/sql.h           | 29 ++++++++++-------
 src/box/sql/build.c     |  3 --
 src/box/sql/select.c    | 85 ++++++++++++++++++++++++++++---------------------
 src/box/sql/sqliteInt.h |  1 -
 test/sql/view.result    | 58 +++++++++++++++++++++++++++++++++
 test/sql/view.test.lua  | 24 ++++++++++++++
 7 files changed, 160 insertions(+), 58 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 029da029e..eca217511 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1504,20 +1504,25 @@ update_view_references(struct Select *select, int update_value,
 		       bool suppress_error, const char **not_found_space)
 {
 	assert(update_value == 1 || update_value == -1);
-	sql_select_expand_from_tables(select);
-	int from_tables_count = sql_select_from_table_count(select);
+	struct SrcList *list = sql_select_expand_from_tables(select);
+	if (list == NULL)
+		return -1;
+	int from_tables_count = sql_src_list_entry_count(list);
 	for (int i = 0; i < from_tables_count; ++i) {
-		const char *space_name = sql_select_from_table_name(select, i);
+		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)
-			return -1;
+				   strlen(space_name), &space_id) != 0) {
+				sqlite3SrcListDelete(sql_get(), list);
+				return -1;
+		}
 		if (space_id == BOX_ID_NIL) {
 			if (! suppress_error) {
 				assert(not_found_space != NULL);
-				*not_found_space = space_name;
+				*not_found_space = tt_sprintf("%s", space_name);
+				sqlite3SrcListDelete(sql_get(), list);
 				return -1;
 			}
 			continue;
@@ -1526,6 +1531,7 @@ update_view_references(struct Select *select, int update_value,
 		assert(space->def->view_ref_count > 0 || update_value > 0);
 		space->def->view_ref_count += update_value;
 	}
+	sqlite3SrcListDelete(sql_get(), list);
 	return 0;
 }
 
diff --git a/src/box/sql.h b/src/box/sql.h
index ec6d9d3cc..028a15245 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -374,35 +374,42 @@ void
 sql_select_delete(struct sqlite3 *db, struct Select *select);
 
 /**
- * Expand all spaces names from 'FROM' clause, including
- * ones from subqueries, and add those names to the original
- * select.
+ * Collect all table names within given select, including
+ * ones from sub-queries, expressions, FROM clause etc.
+ * Return those names as a list.
  *
- * @param select Select to be expanded.
+ * @param select Select to be investigated.
+ * @retval List containing all involved table names.
+ *         NULL in case of OOM.
  */
-void
+struct SrcList *
 sql_select_expand_from_tables(struct Select *select);
 
 /**
  * Temporary getter in order to avoid including sqliteInt.h
  * in alter.cc.
  *
- * @param select Select to be examined.
+ * @param list List to be examined.
  * @retval Count of 'FROM' tables in given select.
  */
 int
-sql_select_from_table_count(const struct Select *select);
+sql_src_list_entry_count(const struct SrcList *list);
 
 /**
  * Temporary getter in order to avoid including sqliteInt.h
  * in alter.cc.
  *
- * @param select Select to be examined.
- * @param i Ordinal number of 'FROM' table.
- * @retval Name of i-th 'FROM' table.
+ * @param list List to be examined.
+ * @param i Ordinal number of entry.
+ * @retval Name of i-th entry.
  */
 const char *
-sql_select_from_table_name(const struct Select *select, int i);
+sql_src_list_entry_name(const struct SrcList *list, int i);
+
+/** Delete an entire SrcList including all its substructure. */
+void
+sqlite3SrcListDelete(struct sqlite3 *db, struct SrcList *list);
+
 
 #if defined(__cplusplus)
 } /* extern "C" { */
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 52f0bde15..7f2a2eca0 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3128,9 +3128,6 @@ sqlite3SrcListAssignCursors(Parse * pParse, SrcList * pList)
 	}
 }
 
-/*
- * Delete an entire SrcList including all its substructure.
- */
 void
 sqlite3SrcListDelete(sqlite3 * db, SrcList * pList)
 {
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index ca709b44f..92ea0d14e 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -229,18 +229,18 @@ sql_select_delete(sqlite3 *db, Select *p)
 }
 
 int
-sql_select_from_table_count(const struct Select *select)
+sql_src_list_entry_count(const struct SrcList *list)
 {
-	assert(select != NULL && select->pSrc != NULL);
-	return select->pSrc->nSrc;
+	assert(list != NULL);
+	return list->nSrc;
 }
 
 const char *
-sql_select_from_table_name(const struct Select *select, int i)
+sql_src_list_entry_name(const struct SrcList *list, int i)
 {
-	assert(select != NULL && select->pSrc != NULL);
-	assert(i >= 0 && i < select->pSrc->nSrc);
-	return select->pSrc->a[i].zName;
+	assert(list != NULL);
+	assert(i >= 0 && i < list->nSrc);
+	return list->a[i].zName;
 }
 
 /*
@@ -258,7 +258,9 @@ findRightmost(Select * p)
 /**
  * Work the same as sqlite3SrcListAppend(), but before adding to
  * list provide check on name duplicates: only values with unique
- * names are appended.
+ * names are appended. Moreover, names of tables are not
+ * normalized: it is parser's business and in struct Select they
+ * are already in uppercased or unquoted form.
  *
  * @param db Database handler.
  * @param list List of entries.
@@ -277,45 +279,54 @@ src_list_append_unique(struct sqlite3 *db, struct SrcList *list,
 		if (name != NULL && strcmp(new_name, name) == 0)
 			return list;
 	}
-	struct Token token = { new_name, strlen(new_name), 0 };
-	return sqlite3SrcListAppend(db, list, &token);
+	list = sqlite3SrcListEnlarge(db, list, 1, list->nSrc);
+	if (db->mallocFailed) {
+		sqlite3SrcListDelete(db, list);
+		return NULL;
+	}
+	struct SrcList_item *pItem = &list->a[list->nSrc - 1];
+	pItem->zName = sqlite3DbStrNDup(db, new_name, strlen(new_name));
+	if (pItem->zName == NULL) {
+		sqlite3SrcListDelete(db, list);
+		return NULL;
+	}
+	return list;
 }
 
-/**
- * This function is an inner call of recursive traverse through
- * select AST starting from interface function
- * sql_select_expand_from_tables().
- *
- * @param top_select The root of AST.
- * @param sub_select sub-select of current level recursion.
- */
-static void
-expand_names_sub_select(struct Select *top_select, struct Select *sub_select)
+static int
+select_collect_table_names(struct Walker *walker, struct Select *select)
 {
-	assert(top_select != NULL);
-	assert(sub_select != NULL);
-	struct SrcList_item *sub_src = sub_select->pSrc->a;
-	for (int i = 0; i < sub_select->pSrc->nSrc; ++i, ++sub_src) {
-		if (sub_src->zName == NULL) {
-			expand_names_sub_select(top_select, sub_src->pSelect);
-		} else {
-			top_select->pSrc =
-				src_list_append_unique(sql_get(),
-						       top_select->pSrc,
-						       sub_src->zName);
-		}
+	assert(walker != NULL);
+	assert(select != NULL);
+	for (int i = 0; i < select->pSrc->nSrc; ++i) {
+		if (select->pSrc->a[i].zName == NULL)
+			continue;
+		walker->u.pSrcList =
+			src_list_append_unique(sql_get(), walker->u.pSrcList,
+					       select->pSrc->a[i].zName);
+		if (walker->u.pSrcList == NULL)
+			return WRC_Abort;
 	}
+	return WRC_Continue;
 }
 
-void
+struct SrcList *
 sql_select_expand_from_tables(struct Select *select)
 {
 	assert(select != NULL);
-	struct SrcList_item *src = select->pSrc->a;
-	for (int i = 0; i < select->pSrc->nSrc; ++i, ++src) {
-		if (select->pSrc->a[i].zName == NULL)
-			expand_names_sub_select(select, src->pSelect);
+	struct Walker walker;
+	struct SrcList *table_names = sql_alloc_src_list(sql_get());
+	if (table_names == NULL)
+		return NULL;
+	memset(&walker, 0, sizeof(walker));
+	walker.xExprCallback = sqlite3ExprWalkNoop;
+	walker.xSelectCallback = select_collect_table_names;
+	walker.u.pSrcList = table_names;
+	if (sqlite3WalkSelect(&walker, select) != 0) {
+		sqlite3SrcListDelete(sql_get(), walker.u.pSrcList);
+		return NULL;
 	}
+	return walker.u.pSrcList;
 }
 
 /*
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index dbf58d967..1ec52b875 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3478,7 +3478,6 @@ int sqlite3IndexedByLookup(Parse *, struct SrcList_item *);
 void sqlite3SrcListShiftJoinType(SrcList *);
 void sqlite3SrcListAssignCursors(Parse *, SrcList *);
 void sqlite3IdListDelete(sqlite3 *, IdList *);
-void sqlite3SrcListDelete(sqlite3 *, SrcList *);
 
 /**
  * Create a new index for an SQL table.  name is the name of the
diff --git a/test/sql/view.result b/test/sql/view.result
index b211bcb2e..2effe4092 100644
--- a/test/sql/view.result
+++ b/test/sql/view.result
@@ -148,6 +148,64 @@ box.sql.execute("DROP VIEW v2;");
 box.sql.execute("DROP TABLE t2;");
 ---
 ...
+-- gh-3815: AS VALUES syntax didn't incerement VIEW reference
+-- counter. Moreover, tables within sub-select were not accounted
+-- as well.
+--
+box.sql.execute("CREATE TABLE b (s1 INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE VIEW bv (wombat) AS VALUES ((SELECT 'k' FROM b));")
+---
+...
+box.sql.execute("DROP TABLE b;")
+---
+- error: 'Can''t drop space ''B'': other views depend on this space'
+...
+box.sql.execute("DROP VIEW bv;")
+---
+...
+box.sql.execute("DROP TABLE b;")
+---
+...
+box.sql.execute("CREATE TABLE b (s1 INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE TABLE c (s1 INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE VIEW bcv AS SELECT * FROM b WHERE s1 IN (SELECT * FROM c);")
+---
+...
+box.sql.execute("DROP TABLE c;")
+---
+- error: 'Can''t drop space ''C'': other views depend on this space'
+...
+box.sql.execute("DROP VIEW bcv;")
+---
+...
+box.sql.execute("DROP TABLE c;")
+---
+...
+box.sql.execute("CREATE TABLE c (s1 INT PRIMARY KEY);")
+---
+...
+box.sql.execute("CREATE VIEW bcv(x, y) AS VALUES((SELECT 'k' FROM b), (VALUES((SELECT 1 FROM b WHERE s1 IN (VALUES((SELECT 1 + c.s1 FROM c)))))))")
+---
+...
+box.sql.execute("DROP TABLE c;")
+---
+- error: 'Can''t drop space ''C'': other views depend on this space'
+...
+box.space.BCV:drop()
+---
+...
+box.sql.execute("DROP TABLE c;")
+---
+...
+box.sql.execute("DROP TABLE b;")
+---
+...
 -- Cleanup
 box.sql.execute("DROP VIEW v1;");
 ---
diff --git a/test/sql/view.test.lua b/test/sql/view.test.lua
index a6269a1bf..cbca201cd 100644
--- a/test/sql/view.test.lua
+++ b/test/sql/view.test.lua
@@ -65,6 +65,30 @@ box.sql.execute("DROP TABLE t2;");
 box.sql.execute("DROP VIEW v2;");
 box.sql.execute("DROP TABLE t2;");
 
+-- gh-3815: AS VALUES syntax didn't incerement VIEW reference
+-- counter. Moreover, tables within sub-select were not accounted
+-- as well.
+--
+box.sql.execute("CREATE TABLE b (s1 INT PRIMARY KEY);")
+box.sql.execute("CREATE VIEW bv (wombat) AS VALUES ((SELECT 'k' FROM b));")
+box.sql.execute("DROP TABLE b;")
+box.sql.execute("DROP VIEW bv;")
+box.sql.execute("DROP TABLE b;")
+
+box.sql.execute("CREATE TABLE b (s1 INT PRIMARY KEY);")
+box.sql.execute("CREATE TABLE c (s1 INT PRIMARY KEY);")
+box.sql.execute("CREATE VIEW bcv AS SELECT * FROM b WHERE s1 IN (SELECT * FROM c);")
+box.sql.execute("DROP TABLE c;")
+box.sql.execute("DROP VIEW bcv;")
+box.sql.execute("DROP TABLE c;")
+
+box.sql.execute("CREATE TABLE c (s1 INT PRIMARY KEY);")
+box.sql.execute("CREATE VIEW bcv(x, y) AS VALUES((SELECT 'k' FROM b), (VALUES((SELECT 1 FROM b WHERE s1 IN (VALUES((SELECT 1 + c.s1 FROM c)))))))")
+box.sql.execute("DROP TABLE c;")
+box.space.BCV:drop()
+box.sql.execute("DROP TABLE c;")
+box.sql.execute("DROP TABLE b;")
+
 -- Cleanup
 box.sql.execute("DROP VIEW v1;");
 box.sql.execute("DROP TABLE t1;");
-- 
2.15.1

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-12-14 10:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-03 15:00 [tarantool-patches] [PATCH] sql: increment VIEW counter for tables within sub-select Nikita Pettik
2018-12-03 16:19 ` [tarantool-patches] " Vladislav Shpilevoy
2018-12-14 10:34 ` Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox