[PATCH v2 2/3] sql: remove space_by_id() from analyze.c

[imeevma@tarantool.org]

Removal of space_by_id() from associated with ANALIZE statement
functions allows us to remove _sql_stat1 and _sql_stat4 tables.
This should not breake anything as ANALYZE is currently disabled.

Part of #2843
Follow up #4069
---
 src/box/sql.c                           |  8 +-------
 src/box/sql.h                           |  5 +++++
 src/box/sql/analyze.c                   | 17 +++++++++++------
 src/box/sql/build.c                     | 28 +++-------------------------
 test/sql-tap/gh-3350-skip-scan.test.lua | 10 +++++-----
 test/sql-tap/suite.ini                  |  1 +
 6 files changed, 26 insertions(+), 43 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 855c2b7..7cfb5e5 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -81,13 +81,7 @@ void
 sql_load_schema()
 {
 	assert(db->init.busy == 0);
-	/*
-	 * This function is called before version upgrade.
-	 * Old versions (< 2.0) lack system spaces containing
-	 * statistics (_sql_stat1 and _sql_stat4). Thus, we can
-	 * skip statistics loading.
-	 */
-	struct space *stat = space_by_id(BOX_SQL_STAT1_ID);
+	struct space *stat = space_by_name("_sql_stat1");
 	assert(stat != NULL);
 	if (stat->def->field_count == 0)
 		return;
diff --git a/src/box/sql.h b/src/box/sql.h
index 400360f..262a48b 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -41,6 +41,11 @@ extern "C" {
 void
 sql_init();
 
+/**
+ * Initialize SQL statistic system.
+ *
+ * Currently unused.
+ */
 void
 sql_load_schema();
 
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 0663c66..0c02050 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -127,7 +127,6 @@ static void
 vdbe_emit_stat_space_open(struct Parse *parse, const char *table_name)
 {
 	const char *stat_names[] = {"_sql_stat1", "_sql_stat4"};
-	const uint32_t stat_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};
 	struct Vdbe *v = sqlGetVdbe(parse);
 	assert(v != NULL);
 	assert(sqlVdbeDb(v) == parse->db);
@@ -137,7 +136,9 @@ vdbe_emit_stat_space_open(struct Parse *parse, const char *table_name)
 			vdbe_emit_stat_space_clear(parse, space_name, NULL,
 						   table_name);
 		} else {
-			sqlVdbeAddOp1(v, OP_Clear, stat_ids[i]);
+			struct space *stat_space = space_by_name(stat_names[i]);
+			assert(stat_space != NULL);
+			sqlVdbeAddOp1(v, OP_Clear, stat_space->def->id);
 		}
 	}
 }
@@ -766,9 +767,9 @@ static void
 vdbe_emit_analyze_space(struct Parse *parse, struct space *space)
 {
 	assert(space != NULL);
-	struct space *stat1 = space_by_id(BOX_SQL_STAT1_ID);
+	struct space *stat1 = space_by_name("_sql_stat1");
 	assert(stat1 != NULL);
-	struct space *stat4 = space_by_id(BOX_SQL_STAT4_ID);
+	struct space *stat4 = space_by_name("_sql_stat4");
 	assert(stat4 != NULL);
 
 	/* Register to hold Stat4Accum object. */
@@ -1374,7 +1375,9 @@ load_stat_from_space(struct sql *db, const char *sql_select_prepare,
 		     const char *sql_select_load, struct index_stat *stats)
 {
 	struct index **indexes = NULL;
-	uint32_t index_count = box_index_len(BOX_SQL_STAT4_ID, 0);
+	struct space *stat_space = space_by_name("_sql_stat4");
+	assert(stat_space != NULL);
+	uint32_t index_count = box_index_len(stat_space->def->id, 0);
 	if (index_count > 0) {
 		size_t alloc_size = sizeof(struct index *) * index_count;
 		indexes = region_alloc(&fiber()->gc, alloc_size);
@@ -1683,7 +1686,9 @@ stat_copy(struct index_stat *dest, const struct index_stat *src)
 int
 sql_analysis_load(struct sql *db)
 {
-	ssize_t index_count = box_index_len(BOX_SQL_STAT1_ID, 0);
+	struct space *stat_space = space_by_name("_sql_stat1");
+	assert(stat_space != NULL);
+	ssize_t index_count = box_index_len(stat_space->def->id, 0);
 	if (index_count < 0)
 		return SQL_TARANTOOL_ERROR;
 	if (box_txn_begin() != 0)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index c475b34..a06ba3e 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1384,23 +1384,6 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
 }
 
 /**
- * Remove entries from the _sql_stat1 and _sql_stat4
- * system spaces after a DROP INDEX or DROP TABLE command.
- *
- * @param parse      The parsing context.
- * @param table_name The table to be dropped or
- *                   the table that contains index to be dropped.
- * @param idx_name   Index to be dropped.
- */
-static void
-sql_clear_stat_spaces(struct Parse *parse, const char *table_name,
-		      const char *idx_name)
-{
-	vdbe_emit_stat_space_clear(parse, "_sql_stat4", idx_name, table_name);
-	vdbe_emit_stat_space_clear(parse, "_sql_stat1", idx_name, table_name);
-}
-
-/**
  * Generate VDBE program to remove entry from _fk_constraint space.
  *
  * @param parse_context Parsing context.
@@ -1601,15 +1584,14 @@ sql_drop_table(struct Parse *parse_context)
 	/*
 	 * Generate code to remove the table from Tarantool
 	 * and internal SQL tables. Basically, it consists
-	 * from 3 stages:
-	 * 1. Delete statistics from _stat1 and _stat4 tables.
-	 * 2. In case of presence of FK constraints, i.e. current
+	 * from 2 stages:
+	 * 1. In case of presence of FK constraints, i.e. current
 	 *    table is child or parent, then start new transaction
 	 *    and erase from table all data row by row. On each
 	 *    deletion check whether any FK violations have
 	 *    occurred. If ones take place, then rollback
 	 *    transaction and halt VDBE.
-	 * 3. Drop table by truncating (if step 2 was skipped),
+	 * 2. Drop table by truncating (if step 1 was skipped),
 	 *    removing indexes from _index space and eventually
 	 *    tuple with corresponding space_id from _space.
 	 */
@@ -1623,7 +1605,6 @@ sql_drop_table(struct Parse *parse_context)
 			goto exit_drop_table;
 		}
 	}
-	sql_clear_stat_spaces(parse_context, space_name, NULL);
 	sql_code_drop_table(parse_context, space, is_view);
 
  exit_drop_table:
@@ -2550,15 +2531,12 @@ sql_drop_index(struct Parse *parse_context)
 		}
 		goto exit_drop_index;
 	}
-	struct index *index = space_index(space, index_id);
-	assert(index != NULL);
 
 	/*
 	 * Generate code to remove entry from _index space
 	 * But firstly, delete statistics since schema
 	 * changes after DDL.
 	 */
-	sql_clear_stat_spaces(parse_context, table_name, index->def->name);
 	int record_reg = ++parse_context->nMem;
 	int space_id_reg = ++parse_context->nMem;
 	int index_id_reg = ++parse_context->nMem;
diff --git a/test/sql-tap/gh-3350-skip-scan.test.lua b/test/sql-tap/gh-3350-skip-scan.test.lua
index c326f7c..4cecfe0 100755
--- a/test/sql-tap/gh-3350-skip-scan.test.lua
+++ b/test/sql-tap/gh-3350-skip-scan.test.lua
@@ -32,7 +32,7 @@ test:do_execsql_test(
             (SELECT int_to_char(0), 'xyz', 'zyx', '*', 0, 0 UNION ALL
             SELECT int_to_char(f+1), b, c, d, (e+1) % 2, f+1 FROM data WHERE f<1024)
             INSERT INTO t1 SELECT a, b, c, d, e, f FROM data;
-            -- ANALYZE;
+            ANALYZE;
             SELECT COUNT(*) FROM t1 WHERE a < 'aaad';
             DROP TABLE t1;
         ]], {
@@ -49,7 +49,7 @@ test:do_execsql_test(
             (SELECT int_to_char(0), 'xyz', 'zyx', '*', 0, 0 UNION ALL
             SELECT int_to_char(f+1), b, c, d, (e+1) % 2, f+1 FROM data WHERE f<1024)
             INSERT INTO t2 SELECT a, b, c, d, e, f FROM data;
-            -- ANALYZE;
+            ANALYZE;
             SELECT COUNT(*) FROM t2 WHERE f < 500;
             DROP TABLE t2;
         ]], {
@@ -68,7 +68,7 @@ test:do_execsql_test(
             (SELECT int_to_char(0), 'xyz', 'zyx', '*', 0, 0 UNION ALL
             SELECT int_to_char(f+1), b, c, d, (e+1) % 2, f+1 FROM data WHERE f<1024)
             INSERT INTO t3 SELECT a, b, c, d, e, f FROM data;
-            -- ANALYZE;
+            ANALYZE;
             SELECT COUNT(*) FROM t3 WHERE f < 500;
             DROP INDEX i31 on t3;
             DROP TABLE t3;
@@ -93,11 +93,11 @@ test:do_execsql_test(
             INSERT INTO t1 VALUES(5, 'def',567,8,9);
             INSERT INTO t1 VALUES(6, 'def',345,9,10);
             INSERT INTO t1 VALUES(7, 'bcd',100,6,11);
-            -- ANALYZE;
+            ANALYZE;
             DELETE FROM "_sql_stat1";
             DELETE FROM "_sql_stat4";
             INSERT INTO "_sql_stat1" VALUES('T1','T1ABC','10000 5000 2000 10');
-            -- ANALYZE t2;
+            ANALYZE t2;
             SELECT a,b,c,d FROM t1 WHERE b=345;
         ]], {
             "abc", 345, 7, 8, "def", 345, 9, 10
diff --git a/test/sql-tap/suite.ini b/test/sql-tap/suite.ini
index 3b1dcce..352bbab 100644
--- a/test/sql-tap/suite.ini
+++ b/test/sql-tap/suite.ini
@@ -21,6 +21,7 @@ disabled = selectA.test.lua ;
            analyzeE.test.lua ;
            analyzeF.test.lua ;
            collation_unicode.test.lua ;
+           gh-3350-skip-scan.test.lua ;
 
 lua_libs = lua/sqltester.lua ../sql/lua/sql_tokenizer.lua ../box/lua/identifier.lua
 is_parallel = True
-- 
2.7.4