Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables.
@ 2019-04-03 16:58 imeevma
  2019-04-03 16:58 ` [PATCH v2 1/3] sql: allocate memory for index_id in VDBE imeevma
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: imeevma @ 2019-04-03 16:58 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches

System tables _sql_stat1 and _sql_stat4 become unused and should
be dropped.

This patch breaks backward compatibility!

https://github.com/tarantool/tarantool/issues/2843
https://github.com/tarantool/tarantool/tree/imeevma/gh-2843-drop-_sql_stat-tables

v1:
https://www.freelists.org/post/tarantool-patches/PATCH-v1-11-sql-remove-sql-stat1-and-sql-stat4-system-tables

Changes in new version:
1) Original patch divided into three.
2) A bit changed commit message and comments.

Mergen Imeev (3):
  sql: allocate memory for index_id in VDBE
  sql: remove space_by_id() from analyze.c
  box: remove _sql_stat1 and _sql_stat4 system tables

 src/box/bootstrap.snap                  | Bin 4477 -> 4374 bytes
 src/box/lua/space.cc                    |   4 ----
 src/box/lua/upgrade.lua                 |  27 ------------------------
 src/box/schema.cc                       |  16 --------------
 src/box/schema_def.h                    |   3 ---
 src/box/sql.c                           |   8 +------
 src/box/sql.h                           |   5 +++++
 src/box/sql/analyze.c                   |  17 +++++++++------
 src/box/sql/build.c                     |  36 +++++++-------------------------
 test/app-tap/tarantoolctl.test.lua      |   4 ++--
 test/box-py/bootstrap.result            |   8 -------
 test/box/access_misc.result             |   5 -----
 test/box/access_sysview.result          |   6 +++---
 test/box/alter.result                   |   3 ---
 test/sql-tap/gh-3350-skip-scan.test.lua |  10 ++++-----
 test/sql-tap/suite.ini                  |   1 +
 test/sql/delete.result                  |   4 ++--
 test/sql/delete.test.lua                |   2 +-
 test/sql/triggers.result                |   4 ++--
 test/sql/triggers.test.lua              |   4 ++--
 test/sql/upgrade.result                 |  20 ------------------
 test/sql/upgrade.test.lua               |   4 ----
 test/wal_off/alter.result               |   2 +-
 23 files changed, 44 insertions(+), 149 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/3] sql: allocate memory for index_id in VDBE
  2019-04-03 16:58 [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables imeevma
@ 2019-04-03 16:58 ` imeevma
  2019-04-04 16:00   ` Vladimir Davydov
  2019-04-03 16:58 ` [PATCH v2 2/3] sql: remove space_by_id() from analyze.c imeevma
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: imeevma @ 2019-04-03 16:58 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches

Currently, the memory for index_id is not allocated in VDBE code
in the sql_code_drop_table() and sql_drop_index() functions. This
may lead to SEGMENTATION FAULT.

Needed for #2843
---
 src/box/sql/build.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 7724e94..c475b34 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1473,6 +1473,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 	 */
 	int idx_rec_reg = ++parse_context->nMem;
 	int space_id_reg = ++parse_context->nMem;
+	int index_id_reg = ++parse_context->nMem;
 	int space_id = space->def->id;
 	sqlVdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
 	sqlVdbeAddOp1(v, OP_CheckViewReferences, space_id_reg);
@@ -1523,7 +1524,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 			for (uint32_t i = 1; i < index_count; ++i) {
 				sqlVdbeAddOp2(v, OP_Integer,
 						  space->index[i]->def->iid,
-						  space_id_reg + 1);
+						  index_id_reg);
 				sqlVdbeAddOp3(v, OP_MakeRecord,
 						  space_id_reg, 2, idx_rec_reg);
 				sqlVdbeAddOp2(v, OP_SDelete, BOX_INDEX_ID,
@@ -1533,7 +1534,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 					     space->index[i]->def->iid));
 			}
 		}
-		sqlVdbeAddOp2(v, OP_Integer, 0, space_id_reg + 1);
+		sqlVdbeAddOp2(v, OP_Integer, 0, index_id_reg);
 		sqlVdbeAddOp3(v, OP_MakeRecord, space_id_reg, 2,
 				  idx_rec_reg);
 		sqlVdbeAddOp2(v, OP_SDelete, BOX_INDEX_ID, idx_rec_reg);
@@ -2560,8 +2561,9 @@ sql_drop_index(struct Parse *parse_context)
 	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;
 	sqlVdbeAddOp2(v, OP_Integer, space->def->id, space_id_reg);
-	sqlVdbeAddOp2(v, OP_Integer, index_id, space_id_reg + 1);
+	sqlVdbeAddOp2(v, OP_Integer, index_id, index_id_reg);
 	sqlVdbeAddOp3(v, OP_MakeRecord, space_id_reg, 2, record_reg);
 	sqlVdbeAddOp2(v, OP_SDelete, BOX_INDEX_ID, record_reg);
 	sqlVdbeChangeP5(v, OPFLAG_NCHANGE);
-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 2/3] sql: remove space_by_id() from analyze.c
  2019-04-03 16:58 [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables imeevma
  2019-04-03 16:58 ` [PATCH v2 1/3] sql: allocate memory for index_id in VDBE imeevma
@ 2019-04-03 16:58 ` imeevma
  2019-04-04 16:03   ` Vladimir Davydov
  2019-04-03 16:58 ` [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables imeevma
  2019-04-05 11:36 ` [tarantool-patches] [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables Kirill Yukhin
  3 siblings, 1 reply; 14+ messages in thread
From: imeevma @ 2019-04-03 16:58 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables
  2019-04-03 16:58 [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables imeevma
  2019-04-03 16:58 ` [PATCH v2 1/3] sql: allocate memory for index_id in VDBE imeevma
  2019-04-03 16:58 ` [PATCH v2 2/3] sql: remove space_by_id() from analyze.c imeevma
@ 2019-04-03 16:58 ` imeevma
  2019-04-03 17:19   ` Vladimir Davydov
  2019-04-04 16:11   ` Vladimir Davydov
  2019-04-05 11:36 ` [tarantool-patches] [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables Kirill Yukhin
  3 siblings, 2 replies; 14+ messages in thread
From: imeevma @ 2019-04-03 16:58 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables
  2019-04-03 16:58 ` [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables imeevma
@ 2019-04-03 17:19   ` Vladimir Davydov
  2019-04-03 17:38     ` Re[2]: " Мерген Имеев
  2019-04-04 16:11   ` Vladimir Davydov
  1 sibling, 1 reply; 14+ messages in thread
From: Vladimir Davydov @ 2019-04-03 17:19 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

On Wed, Apr 03, 2019 at 07:58:38PM +0300, imeevma@tarantool.org wrote:
> >> 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.

Okay. Still, I don't understand how this is supposed to work at all:
the tables have been removed, but the code using them is still there -
it just refers to them by name instead of id now.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re[2]: [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables
  2019-04-03 17:19   ` Vladimir Davydov
@ 2019-04-03 17:38     ` Мерген Имеев
  2019-04-03 17:58       ` Vladimir Davydov
  0 siblings, 1 reply; 14+ messages in thread
From: Мерген Имеев @ 2019-04-03 17:38 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]




>Среда,  3 апреля 2019, 20:19 +03:00 от Vladimir Davydov <vdavydov.dev@gmail.com>:
>
>On Wed, Apr 03, 2019 at 07:58:38PM +0300,  imeevma@tarantool.org wrote:
>> >> 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.
>
>Okay. Still, I don't understand how this is supposed to work at all:
>the tables have been removed, but the code using them is still there -
>it just refers to them by name instead of id now.
Currently I do not know how to change these functions in another
way, since there are no spaces to use instead of _sql_stat1 and
_sql_stat4. I will rework these functions later when a new concept
of SQL statistics appears.

[-- Attachment #2: Type: text/html, Size: 2011 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables
  2019-04-03 17:38     ` Re[2]: " Мерген Имеев
@ 2019-04-03 17:58       ` Vladimir Davydov
  2019-04-03 18:04         ` [tarantool-patches] Re: [tarantool-patches] " Мерген Имеев
  0 siblings, 1 reply; 14+ messages in thread
From: Vladimir Davydov @ 2019-04-03 17:58 UTC (permalink / raw)
  To: Мерген
	Имеев
  Cc: tarantool-patches

On Wed, Apr 03, 2019 at 08:38:45PM +0300, Мерген Имеев wrote:
> 
> 
> 
> >Среда,  3 апреля 2019, 20:19 +03:00 от Vladimir Davydov <vdavydov.dev@gmail.com>:
> >
> >On Wed, Apr 03, 2019 at 07:58:38PM +0300,  imeevma@tarantool.org wrote:
> >> >> 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.
> >
> >Okay. Still, I don't understand how this is supposed to work at all:
> >the tables have been removed, but the code using them is still there -
> >it just refers to them by name instead of id now.
> Currently I do not know how to change these functions in another
> way, since there are no spaces to use instead of _sql_stat1 and
> _sql_stat4. I will rework these functions later when a new concept
> of SQL statistics appears.

That is, these functions aren't supposed to run, right?
If so, it's okay to fix it as you did, I guess.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [tarantool-patches] Re: [tarantool-patches] Re: [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables
  2019-04-03 17:58       ` Vladimir Davydov
@ 2019-04-03 18:04         ` Мерген Имеев
  0 siblings, 0 replies; 14+ messages in thread
From: Мерген Имеев @ 2019-04-03 18:04 UTC (permalink / raw)
  To: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 1870 bytes --]




>Среда,  3 апреля 2019, 20:58 +03:00 от Vladimir Davydov <vdavydov.dev@gmail.com>:
>
>On Wed, Apr 03, 2019 at 08:38:45PM +0300, Мерген Имеев wrote:
>> 
>> 
>> 
>> >Среда,  3 апреля 2019, 20:19 +03:00 от Vladimir Davydov < vdavydov.dev@gmail.com >:
>> >
>> >On Wed, Apr 03, 2019 at 07:58:38PM +0300,  imeevma@tarantool.org wrote:
>> >> >> 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.
>> >
>> >Okay. Still, I don't understand how this is supposed to work at all:
>> >the tables have been removed, but the code using them is still there -
>> >it just refers to them by name instead of id now.
>> Currently I do not know how to change these functions in another
>> way, since there are no spaces to use instead of _sql_stat1 and
>> _sql_stat4. I will rework these functions later when a new concept
>> of SQL statistics appears.
>
>That is, these functions aren't supposed to run, right?
>If so, it's okay to fix it as you did, I guess.
>
Right. They were disabled in issue #4069
https://github.com/tarantool/tarantool/issues/4069  

[-- Attachment #2: Type: text/html, Size: 2814 bytes --]

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] sql: allocate memory for index_id in VDBE
  2019-04-03 16:58 ` [PATCH v2 1/3] sql: allocate memory for index_id in VDBE imeevma
@ 2019-04-04 16:00   ` Vladimir Davydov
  0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Davydov @ 2019-04-04 16:00 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

On Wed, Apr 03, 2019 at 07:58:32PM +0300, imeevma@tarantool.org wrote:
> Currently, the memory for index_id is not allocated in VDBE code
> in the sql_code_drop_table() and sql_drop_index() functions. This
> may lead to SEGMENTATION FAULT.
> 
> Needed for #2843
> ---
>  src/box/sql/build.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)

Ack

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] sql: remove space_by_id() from analyze.c
  2019-04-03 16:58 ` [PATCH v2 2/3] sql: remove space_by_id() from analyze.c imeevma
@ 2019-04-04 16:03   ` Vladimir Davydov
  2019-04-04 17:41     ` Vladimir Davydov
  0 siblings, 1 reply; 14+ messages in thread
From: Vladimir Davydov @ 2019-04-04 16:03 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

On Wed, Apr 03, 2019 at 07:58:34PM +0300, imeevma@tarantool.org wrote:
> 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);
> -}

I would leave this function and instead comment its body so that we
don't forget to resurrect this code when we reimplement stat tables.
Other than that, looks good to me.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables
  2019-04-03 16:58 ` [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables imeevma
  2019-04-03 17:19   ` Vladimir Davydov
@ 2019-04-04 16:11   ` Vladimir Davydov
  2019-04-04 18:18     ` Mergen Imeev
  1 sibling, 1 reply; 14+ messages in thread
From: Vladimir Davydov @ 2019-04-04 16:11 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

On Wed, Apr 03, 2019 at 07:58:38PM +0300, imeevma@tarantool.org wrote:
> New version:
> 
> commit 8907dd9a025b0cafe9858289e4e3e2d64618f12b
> Author: Mergen Imeev <imeevma@gmail.com>
> Date:   Wed Apr 3 19:05:09 2019 +0300

When you insert a patch into an email like this, please use
`git show --format=email` so that one can apply it.

> 
>     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.

This statement deserves an explanation:

	Note, this patch breaks backward compatibility between 2.1.1 and 2.1.2,
	but that's okay as 2.1.1 was beta and we didn't recommend anyone to use
	it.

Or something like this.

The patch itself is okay although I'd squash it with patch 2.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] sql: remove space_by_id() from analyze.c
  2019-04-04 16:03   ` Vladimir Davydov
@ 2019-04-04 17:41     ` Vladimir Davydov
  0 siblings, 0 replies; 14+ messages in thread
From: Vladimir Davydov @ 2019-04-04 17:41 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

On Thu, Apr 04, 2019 at 07:03:49PM +0300, Vladimir Davydov wrote:
> On Wed, Apr 03, 2019 at 07:58:34PM +0300, imeevma@tarantool.org wrote:
> > 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);
> > -}
> 
> I would leave this function and instead comment its body so that we
> don't forget to resurrect this code when we reimplement stat tables.
> Other than that, looks good to me.

On the second thought, it doesn't make much sense to leave this function
as we'll have to rewrite it anyway when we reintroduce stat tables. And
it's pretty obvious that we should clear stat entries on space/index
drop so there's no need in this placeholder.

That said, the patch is okay to commit as is IMO.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables
  2019-04-04 16:11   ` Vladimir Davydov
@ 2019-04-04 18:18     ` Mergen Imeev
  0 siblings, 0 replies; 14+ messages in thread
From: Mergen Imeev @ 2019-04-04 18:18 UTC (permalink / raw)
  To: Vladimir Davydov; +Cc: tarantool-patches

Thank you for review! My answers and new patch below. I squashed
2 ans 3 patches.

On Thu, Apr 04, 2019 at 07:11:20PM +0300, Vladimir Davydov wrote:
> On Wed, Apr 03, 2019 at 07:58:38PM +0300, imeevma@tarantool.org wrote:
> > New version:
> > 
> > commit 8907dd9a025b0cafe9858289e4e3e2d64618f12b
> > Author: Mergen Imeev <imeevma@gmail.com>
> > Date:   Wed Apr 3 19:05:09 2019 +0300
> 
> When you insert a patch into an email like this, please use
> `git show --format=email` so that one can apply it.
> 
Will do, thanks.

> > 
> >     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.
> 
> This statement deserves an explanation:
> 
> 	Note, this patch breaks backward compatibility between 2.1.1 and 2.1.2,
> 	but that's okay as 2.1.1 was beta and we didn't recommend anyone to use
> 	it.
> 
> Or something like this.
> 
Fixed, added replaced old note by this one.

> The patch itself is okay although I'd squash it with patch 2.
Fixed, squashed.

New patch:

From 10ddf20b41dcf9297b2ca9d0883c2537866a3649 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Wed, 3 Apr 2019 12:45:11 +0300
Subject: [PATCH] box: remove _sql_stat1 and _sql_stat4 system tables

These tables won't be used anymore and should be deleted.

Note, this patch breaks backward compatibility between 2.1.1 and
2.1.2, but that's okay as 2.1.1 was beta and we didn't recommend
anyone to use it.

Part of #2843
Follow up #4069

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 ca793e4..23782e5 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -561,10 +561,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/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/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-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
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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [tarantool-patches] [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables.
  2019-04-03 16:58 [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables imeevma
                   ` (2 preceding siblings ...)
  2019-04-03 16:58 ` [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables imeevma
@ 2019-04-05 11:36 ` Kirill Yukhin
  3 siblings, 0 replies; 14+ messages in thread
From: Kirill Yukhin @ 2019-04-05 11:36 UTC (permalink / raw)
  To: tarantool-patches; +Cc: vdavydov.dev

Hello,

On 03 Apr 19:58, imeevma@tarantool.org wrote:
> System tables _sql_stat1 and _sql_stat4 become unused and should
> be dropped.
> 
> This patch breaks backward compatibility!
> 
> https://github.com/tarantool/tarantool/issues/2843
> https://github.com/tarantool/tarantool/tree/imeevma/gh-2843-drop-_sql_stat-tables
> 
> v1:
> https://www.freelists.org/post/tarantool-patches/PATCH-v1-11-sql-remove-sql-stat1-and-sql-stat4-system-tables
> 
> Changes in new version:
> 1) Original patch divided into three.
> 2) A bit changed commit message and comments.
> 
> Mergen Imeev (3):
>   sql: allocate memory for index_id in VDBE
>   sql: remove space_by_id() from analyze.c
>   box: remove _sql_stat1 and _sql_stat4 system tables

I've checked your patches into master and 2.1 branch.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-04-05 11:36 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-03 16:58 [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables imeevma
2019-04-03 16:58 ` [PATCH v2 1/3] sql: allocate memory for index_id in VDBE imeevma
2019-04-04 16:00   ` Vladimir Davydov
2019-04-03 16:58 ` [PATCH v2 2/3] sql: remove space_by_id() from analyze.c imeevma
2019-04-04 16:03   ` Vladimir Davydov
2019-04-04 17:41     ` Vladimir Davydov
2019-04-03 16:58 ` [PATCH v2 3/3] box: remove _sql_stat1 and _sql_stat4 system tables imeevma
2019-04-03 17:19   ` Vladimir Davydov
2019-04-03 17:38     ` Re[2]: " Мерген Имеев
2019-04-03 17:58       ` Vladimir Davydov
2019-04-03 18:04         ` [tarantool-patches] Re: [tarantool-patches] " Мерген Имеев
2019-04-04 16:11   ` Vladimir Davydov
2019-04-04 18:18     ` Mergen Imeev
2019-04-05 11:36 ` [tarantool-patches] [PATCH v2 0/3] box: drop _sql_stat1 and _sql_stat4 tables Kirill Yukhin

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