[PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables

[imeevma@tarantool.org]

Hi! Thank you for review. My answers and new version below.

On 4/3/19 11:35 AM, Vladimir Davydov wrote:
> On Tue, Apr 02, 2019 at 12:25:49PM +0300, imeevma@tarantool.org wrote:
>> These tables won't be used anymore and should be deleted.
>
> Please mention that this breaks backward compatibility.
>
Added to commit-message.

>> diff --git a/src/box/sql.c b/src/box/sql.c
>> index 4fac020..7beeee1 100644
>> --- a/src/box/sql.c
>> +++ b/src/box/sql.c
>> @@ -87,7 +87,7 @@ sql_load_schema()
>>  	 * 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");
>
> I don't understand this change: even though you removed the tables you
> still expect them to be accessible by name here and in a few other
> places. At any rate, the comment above needs to be refreshed.
>
I did this to free BOX_SQL_STAT1_ID and BOX_SQL_STAT4_ID so I
could remove them. These functions are currently unused. Moved
this fix to a different commit.

>> @@ -2478,19 +2460,17 @@ sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
>>  		}
>>  		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;
>
> Nit: a space is missing.
>
Fixed.

> Anyway, this change looks like it doesn't have anything to do with stat
> tables removal. If so, please factor it out into a separate patch with a
> proper justification.
>
Actually it does. When I removed the cleanup of the statistics
tables from the functions mentioned, the SQL tests started to fail
due to a segmentation fault. I moved this fix to a different
commit.


New version:

commit 8907dd9a025b0cafe9858289e4e3e2d64618f12b
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Wed Apr 3 19:05:09 2019 +0300

    box: remove _sql_stat1 and _sql_stat4 system tables
    
    These tables won't be used anymore and should be deleted.
    
    This patch breaks backward compatibility.
    
    Part of #2843

diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
index 1dba789..871a93f 100644
Binary files a/src/box/bootstrap.snap and b/src/box/bootstrap.snap differ
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 9dfc97b..5a5875e 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -556,10 +556,6 @@ box_lua_space_init(struct lua_State *L)
 	lua_setfield(L, -2, "CLUSTER_ID");
 	lua_pushnumber(L, BOX_TRIGGER_ID);
 	lua_setfield(L, -2, "TRIGGER_ID");
-	lua_pushnumber(L, BOX_SQL_STAT1_ID);
-	lua_setfield(L, -2, "SQL_STAT1_ID");
-	lua_pushnumber(L, BOX_SQL_STAT4_ID);
-	lua_setfield(L, -2, "SQL_STAT4_ID");
 	lua_pushnumber(L, BOX_FK_CONSTRAINT_ID);
 	lua_setfield(L, -2, "FK_CONSTRAINT_ID");
 	lua_pushnumber(L, BOX_TRUNCATE_ID);
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 275aaf7..89d6e3d 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -553,33 +553,6 @@ local function upgrade_to_2_1_0()
     _index:insert{_trigger.id, 1, 'space_id', 'tree', { unique = false },
                   {{1, 'unsigned'}}}
 
-    local stat1_ft = {{name='tbl', type='string'},
-                      {name='idx', type='string'},
-                      {name='stat', type='string'}}
-    local stat4_ft = {{name='tbl', type='string'},
-                      {name='idx', type='string'},
-                      {name='neq', type='string'},
-                      {name='nlt', type='string'},
-                      {name='ndlt', type='string'},
-                      {name='sample', type='scalar'}}
-
-    log.info("create space _sql_stat1")
-    _space:insert{box.schema.SQL_STAT1_ID, ADMIN, '_sql_stat1', 'memtx', 0,
-                  MAP, stat1_ft}
-
-    log.info("create index primary on _sql_stat1")
-    _index:insert{box.schema.SQL_STAT1_ID, 0, 'primary', 'tree',
-                  {unique = true}, {{0, 'string'}, {1, 'string'}}}
-
-    log.info("create space _sql_stat4")
-    _space:insert{box.schema.SQL_STAT4_ID, ADMIN, '_sql_stat4', 'memtx', 0,
-                  MAP, stat4_ft}
-
-    log.info("create index primary on _sql_stat4")
-    _index:insert{box.schema.SQL_STAT4_ID, 0, 'primary', 'tree',
-                  {unique = true}, {{0, 'string'}, {1, 'string'},
-                                    {5, 'scalar'}}}
-
     local fk_constr_ft = {{name='name', type='string'},
                           {name='child_id', type='unsigned'},
                           {name='parent_id', type='unsigned'},
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 79d0de4..9a55c2f 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -452,22 +452,6 @@ schema_init()
 	sc_space_new(BOX_INDEX_ID, "_index", key_parts, 2,
 		     &alter_space_on_replace_index, &on_stmt_begin_index);
 
-	/* _sql_stat1 - a simpler statistics on space, seen in SQL. */
-	key_parts[0].fieldno = 0; /* space name */
-	key_parts[0].type = FIELD_TYPE_STRING;
-	key_parts[1].fieldno = 1; /* index name */
-	key_parts[1].type = FIELD_TYPE_STRING;
-	sc_space_new(BOX_SQL_STAT1_ID, "_sql_stat1", key_parts, 2, NULL, NULL);
-
-	/* _sql_stat4 - extensive statistics on space, seen in SQL. */
-	key_parts[0].fieldno = 0; /* space name */
-	key_parts[0].type = FIELD_TYPE_STRING;
-	key_parts[1].fieldno = 1; /* index name */
-	key_parts[1].type = FIELD_TYPE_STRING;
-	key_parts[2].fieldno = 5; /* sample */
-	key_parts[2].type = FIELD_TYPE_SCALAR;
-	sc_space_new(BOX_SQL_STAT4_ID, "_sql_stat4", key_parts, 3, NULL, NULL);
-
 	/* _fk_сonstraint - foreign keys constraints. */
 	key_parts[0].fieldno = 0; /* constraint name */
 	key_parts[0].type = FIELD_TYPE_STRING;
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index a760ecc..eeeeb95 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -106,9 +106,6 @@ enum {
 	BOX_TRUNCATE_ID = 330,
 	/** Space id of _space_sequence. */
 	BOX_SPACE_SEQUENCE_ID = 340,
-	/** Space ids for SQL statictics. */
-	BOX_SQL_STAT1_ID = 348,
-	BOX_SQL_STAT4_ID = 349,
 	/** Space id of _fk_constraint. */
 	BOX_FK_CONSTRAINT_ID = 356,
 	/** End of the reserved range of system spaces. */
diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
index a914db5..62a78d6 100755
--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -403,8 +403,8 @@ do
             check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1", "\n", 3)
             check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1 --replica 2", "\n", 3)
             check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 2", "\n", 0)
-            check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 23)
-            check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 49)
+            check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 21)
+            check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 47)
         end)
     end)
 
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index fa6099d..379f6c5 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -73,11 +73,6 @@ box.space._space:select{}
         'type': 'unsigned'}]]
   - [340, 1, '_space_sequence', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
       {'name': 'sequence_id', 'type': 'unsigned'}, {'name': 'is_generated', 'type': 'boolean'}]]
-  - [348, 1, '_sql_stat1', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-        'type': 'string'}, {'name': 'stat', 'type': 'string'}]]
-  - [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-        'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'},
-      {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]]
   - [356, 1, '_fk_constraint', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'},
       {'name': 'child_id', 'type': 'unsigned'}, {'name': 'parent_id', 'type': 'unsigned'},
       {'name': 'is_deferred', 'type': 'boolean'}, {'name': 'match', 'type': 'string'},
@@ -133,9 +128,6 @@ box.space._index:select{}
   - [330, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
   - [340, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
   - [340, 1, 'sequence', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [348, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string']]]
-  - [349, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string'], [
-        5, 'scalar']]]
   - [356, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]]
   - [356, 1, 'child_id', 'tree', {'unique': false}, [[1, 'unsigned']]]
 ...
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 4ffeb38..36ebfae 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -813,11 +813,6 @@ box.space._space:select()
         'type': 'unsigned'}]]
   - [340, 1, '_space_sequence', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
       {'name': 'sequence_id', 'type': 'unsigned'}, {'name': 'is_generated', 'type': 'boolean'}]]
-  - [348, 1, '_sql_stat1', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-        'type': 'string'}, {'name': 'stat', 'type': 'string'}]]
-  - [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-        'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'},
-      {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]]
   - [356, 1, '_fk_constraint', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'},
       {'name': 'child_id', 'type': 'unsigned'}, {'name': 'parent_id', 'type': 'unsigned'},
       {'name': 'is_deferred', 'type': 'boolean'}, {'name': 'match', 'type': 'string'},
diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result
index fd8b142..ae04266 100644
--- a/test/box/access_sysview.result
+++ b/test/box/access_sysview.result
@@ -230,11 +230,11 @@ box.session.su('guest')
 ...
 #box.space._vspace:select{}
 ---
-- 24
+- 22
 ...
 #box.space._vindex:select{}
 ---
-- 50
+- 48
 ...
 #box.space._vuser:select{}
 ---
@@ -262,7 +262,7 @@ box.session.su('guest')
 ...
 #box.space._vindex:select{}
 ---
-- 50
+- 48
 ...
 #box.space._vuser:select{}
 ---
diff --git a/test/box/alter.result b/test/box/alter.result
index 37bc51c..c1b1de1 100644
--- a/test/box/alter.result
+++ b/test/box/alter.result
@@ -228,9 +228,6 @@ _index:select{}
   - [330, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
   - [340, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
   - [340, 1, 'sequence', 'tree', {'unique': false}, [[1, 'unsigned']]]
-  - [348, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string']]]
-  - [349, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string'], [
-        5, 'scalar']]]
   - [356, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'unsigned']]]
   - [356, 1, 'child_id', 'tree', {'unique': false}, [[1, 'unsigned']]]
 ...
diff --git a/test/sql/delete.result b/test/sql/delete.result
index 30f6b67..59178ba 100644
--- a/test/sql/delete.result
+++ b/test/sql/delete.result
@@ -79,9 +79,9 @@ box.execute("DROP TABLE t2;")
 -- gh-2201: TRUNCATE TABLE operation.
 --
 -- can't truncate system table.
-box.execute("TRUNCATE TABLE \"_sql_stat1\";")
+box.execute("TRUNCATE TABLE \"_fk_constraint\";")
 ---
-- error: Can't truncate a system space, space '_sql_stat1'
+- error: Can't truncate a system space, space '_fk_constraint'
 ...
 box.execute("CREATE TABLE t1(id INT PRIMARY KEY, a INT, b TEXT);")
 ---
diff --git a/test/sql/delete.test.lua b/test/sql/delete.test.lua
index 96a18b7..61685e7 100644
--- a/test/sql/delete.test.lua
+++ b/test/sql/delete.test.lua
@@ -44,7 +44,7 @@ box.execute("DROP TABLE t2;")
 --
 
 -- can't truncate system table.
-box.execute("TRUNCATE TABLE \"_sql_stat1\";")
+box.execute("TRUNCATE TABLE \"_fk_constraint\";")
 
 box.execute("CREATE TABLE t1(id INT PRIMARY KEY, a INT, b TEXT);")
 box.execute("INSERT INTO t1 VALUES(1, 1, 'one');")
diff --git a/test/sql/triggers.result b/test/sql/triggers.result
index bb8d80c..7dc44ea 100644
--- a/test/sql/triggers.result
+++ b/test/sql/triggers.result
@@ -267,10 +267,10 @@ box.space._trigger:insert(tuple)
 ---
 - error: 'SQL error: cannot create AFTER trigger on view: V1'
 ...
-space_id =  box.space._sql_stat1.id
+space_id =  box.space._fk_constraint.id
 ---
 ...
-tuple = {"T1T", space_id, {sql = [[create trigger t1t instead of update on "_sql_stat1" for each row begin delete from t1 WHERE a=old.a+2; end;]]}}
+tuple = {"T1T", space_id, {sql = [[create trigger t1t instead of update on "_fk_constraint" for each row begin delete from t1 WHERE a=old.a+2; end;]]}}
 ---
 ...
 box.space._trigger:insert(tuple)
diff --git a/test/sql/triggers.test.lua b/test/sql/triggers.test.lua
index e50699b..7749402 100644
--- a/test/sql/triggers.test.lua
+++ b/test/sql/triggers.test.lua
@@ -89,8 +89,8 @@ box.space._trigger:insert(tuple)
 tuple = {"V1T", space_id, {sql = [[create trigger v1t AFTER update on v1 for each row begin delete from t1 WHERE a=old.a+2; end;]]}}
 box.space._trigger:insert(tuple)
 
-space_id =  box.space._sql_stat1.id
-tuple = {"T1T", space_id, {sql = [[create trigger t1t instead of update on "_sql_stat1" for each row begin delete from t1 WHERE a=old.a+2; end;]]}}
+space_id =  box.space._fk_constraint.id
+tuple = {"T1T", space_id, {sql = [[create trigger t1t instead of update on "_fk_constraint" for each row begin delete from t1 WHERE a=old.a+2; end;]]}}
 box.space._trigger:insert(tuple)
 
 box.execute("DROP VIEW V1;")
diff --git a/test/sql/upgrade.result b/test/sql/upgrade.result
index cec8813..9569674 100644
--- a/test/sql/upgrade.result
+++ b/test/sql/upgrade.result
@@ -29,30 +29,10 @@ box.space._space.index['name']:get('_trigger')
 - [328, 1, '_trigger', 'memtx', 0, {}, [{'name': 'name', 'type': 'string'}, {'name': 'space_id',
       'type': 'unsigned'}, {'name': 'opts', 'type': 'map'}]]
 ...
-box.space._space.index['name']:get('_sql_stat1')
----
-- [348, 1, '_sql_stat1', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-      'type': 'string'}, {'name': 'stat', 'type': 'string'}]]
-...
-box.space._space.index['name']:get('_sql_stat4')
----
-- [349, 1, '_sql_stat4', 'memtx', 0, {}, [{'name': 'tbl', 'type': 'string'}, {'name': 'idx',
-      'type': 'string'}, {'name': 'neq', 'type': 'string'}, {'name': 'nlt', 'type': 'string'},
-    {'name': 'ndlt', 'type': 'string'}, {'name': 'sample', 'type': 'scalar'}]]
-...
 box.space._index:get({box.space._space.index['name']:get('_trigger').id, 0})
 ---
 - [328, 0, 'primary', 'tree', {'unique': true}, [[0, 'string']]]
 ...
-box.space._index:get({box.space._space.index['name']:get('_sql_stat1').id, 0})
----
-- [348, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string']]]
-...
-box.space._index:get({box.space._space.index['name']:get('_sql_stat4').id, 0})
----
-- [349, 0, 'primary', 'tree', {'unique': true}, [[0, 'string'], [1, 'string'], [5,
-      'scalar']]]
-...
 box.space._schema:format()
 ---
 - [{'type': 'string', 'name': 'key'}, {'type': 'any', 'name': 'value', 'is_nullable': true}]
diff --git a/test/sql/upgrade.test.lua b/test/sql/upgrade.test.lua
index 351e79d..e6cdae1 100644
--- a/test/sql/upgrade.test.lua
+++ b/test/sql/upgrade.test.lua
@@ -10,12 +10,8 @@ test_run:switch('upgrade')
 
 -- test system tables
 box.space._space.index['name']:get('_trigger')
-box.space._space.index['name']:get('_sql_stat1')
-box.space._space.index['name']:get('_sql_stat4')
 
 box.space._index:get({box.space._space.index['name']:get('_trigger').id, 0})
-box.space._index:get({box.space._space.index['name']:get('_sql_stat1').id, 0})
-box.space._index:get({box.space._space.index['name']:get('_sql_stat4').id, 0})
 
 box.space._schema:format()
 
diff --git a/test/wal_off/alter.result b/test/wal_off/alter.result
index b4c6a92..becdf13 100644
--- a/test/wal_off/alter.result
+++ b/test/wal_off/alter.result
@@ -28,7 +28,7 @@ end;
 ...
 #spaces;
 ---
-- 65509
+- 65511
 ...
 -- cleanup
 for k, v in pairs(spaces) do
-- 
2.7.4