<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hello. Consider comments below.</div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On 3 Apr 2018, at 21:07, N.Tatunov <<a href="mailto:hollow653@gmail.com" class="">hollow653@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Currently dropping an index leads to removal of<br class="">all the entries containing the certain index name<br class="">in "_sql_statN" tables. Thus far analyze routine was fixed<br class="">so it seems that the indexes from the different tables but<br class="">with the same names should work more properly.<br class=""><br class="">Closes: #3264<br class="">---<br class=""><br class="">Branch: <a href="https://github.com/tarantool/tarantool" class="">https://github.com/tarantool/tarantool</a><br class=""></div></div></blockquote><div><br class=""></div><div>This is link to the whole repo.</div><div>You should place here link to your particular branch.</div><div>Example:</div><a href="https://github.com/tarantool/tarantool/tree/N_Tatunov/gh-3264-stat-table-entries-removal" class="">https://github.com/tarantool/tarantool/tree/N_Tatunov/gh-3264-stat-table-entries-removal</a></div><div><br class=""><blockquote type="cite" class=""><div class=""><div class="">Issue: <a href="https://github.com/tarantool/tarantool/issues/3264" class="">https://github.com/tarantool/tarantool/issues/3264</a><br class=""><br class=""> src/box/sql/build.c                    |  36 ++++++----<br class=""> test/sql/sql-statN-index-drop.result   | 127 +++++++++++++++++++++++++++++++++<br class=""> test/sql/sql-statN-index-drop.test.lua |  54 ++++++++++++++<br class=""> 3 files changed, 205 insertions(+), 12 deletions(-)<br class=""> create mode 100644 test/sql/sql-statN-index-drop.result<br class=""> create mode 100644 test/sql/sql-statN-index-drop.test.lua<br class=""><br class="">diff --git a/src/box/sql/build.c b/src/box/sql/build.c<br class="">index 5e3ed0f..baaade2 100644<br class="">--- a/src/box/sql/build.c<br class="">+++ b/src/box/sql/build.c<br class="">@@ -2213,19 +2213,31 @@ sqliteViewResetAll(sqlite3 * db)<br class=""> static void<br class=""> sqlite3ClearStatTables(Parse * pParse,<span class="Apple-tab-span" style="white-space:pre">  </span>/* The parsing context */<br class=""> <span class="Apple-tab-span" style="white-space:pre">     </span><span class="Apple-tab-span" style="white-space:pre">    </span>       const char *zType,<span class="Apple-tab-span" style="white-space:pre">     </span>/* "idx" or "tbl" */<br class="">-<span class="Apple-tab-span" style="white-space:pre">      </span><span class="Apple-tab-span" style="white-space:pre">    </span>       const char *zName<span class="Apple-tab-span" style="white-space:pre">      </span>/* Name of index or table */<br class="">+<span class="Apple-tab-span" style="white-space:pre">  </span><span class="Apple-tab-span" style="white-space:pre">    </span>       const char *zName,<span class="Apple-tab-span" style="white-space:pre">     </span>/* Name of index or table */<br class="">+<span class="Apple-tab-span" style="white-space:pre">  </span><span class="Apple-tab-span" style="white-space:pre">    </span>       const char *zTable   /* Name of the indexed table*/<br class="">     )<br class=""></div></div></blockquote><div><br class=""></div><div>Way you has chosen is very confusing.</div><div>Here table name is passed as third arg and index is NULL:</div><div><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space: pre;">  </span>sqlite3ClearStatTables(pParse, "tbl", pTab->zName, NULL);</blockquote></div><div>But here - vice versa:</div><div><blockquote type="cite" class="">+<span class="Apple-tab-span" style="white-space: pre;"> </span>sqlite3ClearStatTables(pParse, "idx", pIndex->zName, pIndex->pTable->zName);</blockquote><br class="">Lets strictly specify, that third arg always is a table's name,</div><div>and fourth one - index.</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""> {<br class=""> <span class="Apple-tab-span" style="white-space:pre">        </span>int i;<br class="">-<span class="Apple-tab-span" style="white-space:pre">        </span>for (i = 1; i <= 4; i++) {<br class="">-<span class="Apple-tab-span" style="white-space:pre"> </span><span class="Apple-tab-span" style="white-space:pre">    </span>char zTab[24];<br class="">-<span class="Apple-tab-span" style="white-space:pre">        </span><span class="Apple-tab-span" style="white-space:pre">    </span>sqlite3_snprintf(sizeof(zTab), zTab, "_sql_stat%d", i);<br class="">-<span class="Apple-tab-span" style="white-space:pre">     </span><span class="Apple-tab-span" style="white-space:pre">    </span>if (sqlite3FindTable(pParse->db, zTab)) {<br class="">-<span class="Apple-tab-span" style="white-space:pre">  </span><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span>sqlite3NestedParse(pParse,<br class="">-<span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span>   "DELETE FROM \"%s\" WHERE \"%s\"=%Q",<br class="">-<span class="Apple-tab-span" style="white-space:pre">      </span><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span><span class="Apple-tab-span" style="white-space:pre">    </span>   zTab, zType, zName);<br class="">-<span class="Apple-tab-span" style="white-space:pre">     </span><span class="Apple-tab-span" style="white-space:pre">    </span>}<br class="">-<span class="Apple-tab-span" style="white-space:pre">     </span>}<br class="">+<span class="Apple-tab-span" style="white-space:pre">     </span>if(strcmp(zType, "idx") == 0)<br class="">+        for (i = 1; i <= 4; i++) {<br class=""></div></div></blockquote><div><br class=""></div><div>Actually, it is legacy code: it derived from SQLite codebase.</div><div>In SQLite really can exist all four[1-4] tables. But in our SQL</div><div>only _stat1 and _stat4 tables are valid tables. </div><br class=""><blockquote type="cite" class=""><div class=""><div class="">+            char zTab[24];<br class="">+            sqlite3_snprintf(sizeof(zTab), zTab, "_sql_stat%d", i);<br class="">+            if (sqlite3FindTable(pParse->db, zTab)) {<br class=""></div></div></blockquote><div><br class=""></div><div>Statistic tables are considered to be system.</div><div>This means that they are created when Tarantool is launching.</div><div>Thus, they always exist and you don’t need to check if table exists.</div><div>In this case existence is checked due to the fact that tables</div><div>_stat2 and _stat3 does not exist. So I propose you to involve only</div><div>required tables and get rid of this check. </div><br class=""><blockquote type="cite" class=""><div class=""><div class="">+                sqlite3NestedParse(pParse,<br class="">+                        "DELETE FROM \"%s\" WHERE (\"%s\"=%Q AND "<br class="">+                        "\"tbl\"=%Q)",<br class="">+                        zTab, zType, zName, zTable);<br class="">+            }<br class="">+        } else<br class="">+        for (i = 1; i <= 4; i++) {<br class="">+            char zTab[24];<br class="">+            sqlite3_snprintf(sizeof(zTab), zTab, "_sql_stat%d", i);<br class="">+            if (sqlite3FindTable(pParse->db, zTab)) {<br class="">+                sqlite3NestedParse(pParse,<br class="">+                        "DELETE FROM \"%s\" WHERE \"%s\"=%Q",<br class="">+                        zTab, zType, zName);<br class="">+            }<br class="">+        }<br class=""> }<br class=""></div></div></blockquote><div><br class=""></div><div><div><div>Overall comment: you may notice that now SQL codebase is written using</div><div>legacy codestyle (including obsolete var declaration style and</div><div>Hungarian notation). I suggest you to rewrite whole function using </div><div>Tarantool (aka Linux Kernel) codestyle and eventually rename it to</div><div>'sql_clear_stat_tables'. Also, we are using comments for functions</div><div>in doxygen format. You can check it out in any src/box source file.</div></div></div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""> /*<br class="">@@ -2415,7 +2427,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)<br class=""> <span class="Apple-tab-span" style="white-space:pre">        </span> */<br class=""><br class=""> <span class="Apple-tab-span" style="white-space:pre">      </span>sqlite3BeginWriteOperation(pParse, 1);<br class="">-<span class="Apple-tab-span" style="white-space:pre">        </span>sqlite3ClearStatTables(pParse, "tbl", pTab->zName);<br class="">+<span class="Apple-tab-span" style="white-space:pre">      </span>sqlite3ClearStatTables(pParse, "tbl", pTab->zName, NULL);<br class=""> <span class="Apple-tab-span" style="white-space:pre">        </span>sqlite3FkDropTable(pParse, pName, pTab);<br class=""> <span class="Apple-tab-span" style="white-space:pre">      </span>sqlite3CodeDropTable(pParse, pTab, isView);<br class=""><br class="">@@ -3417,7 +3429,7 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)<br class=""> <span class="Apple-tab-span" style="white-space:pre">    </span> * But firstly, delete statistics since schema<br class=""> <span class="Apple-tab-span" style="white-space:pre">        </span> * changes after DDL.<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span> */<br class="">-<span class="Apple-tab-span" style="white-space:pre">   </span>sqlite3ClearStatTables(pParse, "idx", pIndex->zName);<br class="">+<span class="Apple-tab-span" style="white-space:pre">    </span>sqlite3ClearStatTables(pParse, "idx", pIndex->zName, pIndex->pTable->zName);<br class=""> <span class="Apple-tab-span" style="white-space:pre">       </span>int record_reg = ++pParse->nMem;<br class=""> <span class="Apple-tab-span" style="white-space:pre">   </span>int space_id_reg = ++pParse->nMem;<br class=""> <span class="Apple-tab-span" style="white-space:pre"> </span>sqlite3VdbeAddOp2(v, OP_Integer, SQLITE_PAGENO_TO_SPACEID(pIndex->tnum),<br class="">diff --git a/test/sql/sql-statN-index-drop.result b/test/sql/sql-statN-index-drop.result<br class="">new file mode 100644<br class="">index 0000000..707d7e0<br class="">--- /dev/null<br class="">+++ b/test/sql/sql-statN-index-drop.result<br class=""></div></div></blockquote><div><br class=""></div><div><div>Actually, you don’t have to create separate test file for this issue:</div><div>you can add these tests to one of sql-tap/analyze[1-11].test.lua</div><div>But it is up to you.</div></div><br class=""><blockquote type="cite" class=""><div class=""><div class="">@@ -0,0 +1,127 @@<br class="">+test_run = require('test_run').new()<br class="">+---<br class="">+...<br class="">+-- Initializing some things.<br class="">+box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("CREATE TABLE t2(id PRIMARY KEY, a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("CREATE INDEX i1 ON t1(a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("CREATE INDEX i1 ON t2(a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("INSERT INTO t1 VALUES(1, 2);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("INSERT INTO t2 VALUES(1, 2);")<br class="">+---<br class="">+...<br class="">+-- Analyze.<br class="">+box.sql.execute("ANALYZE;")<br class="">+---<br class="">+...<br class="">+-- Checking the data.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+---<br class="">+- - ['T1', 'I1', '1', '0', '0', !!binary kQI=]<br class="">+  - ['T1', 'T1', '1', '0', '0', !!binary kQE=]<br class="">+  - ['T2', 'I1', '1', '0', '0', !!binary kQI=]<br class="">+  - ['T2', 'T2', '1', '0', '0', !!binary kQE=]<br class="">+...<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+---<br class="">+- - ['T1', 'I1', '1 1']<br class="">+  - ['T1', 'T1', '1 1']<br class="">+  - ['T2', 'I1', '1 1']<br class="">+  - ['T2', 'T2', '1 1']<br class="">+...<br class="">+-- Dropping an index<br class="">+box.sql.execute("DROP INDEX i1 ON t1;")<br class="">+---<br class="">+...<br class="">+-- Checking the DROP INDEX results.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+---<br class="">+- - ['T1', 'T1', '1', '0', '0', !!binary kQE=]<br class="">+  - ['T2', 'I1', '1', '0', '0', !!binary kQI=]<br class="">+  - ['T2', 'T2', '1', '0', '0', !!binary kQE=]<br class="">+...<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+---<br class="">+- - ['T1', 'T1', '1 1']<br class="">+  - ['T2', 'I1', '1 1']<br class="">+  - ['T2', 'T2', '1 1']<br class="">+...<br class="">+--Cleaning up.<br class="">+box.sql.execute("DROP TABLE t1;")<br class="">+---<br class="">+...<br class="">+box.sql.execute("DROP TABLE t2;")<br class="">+---<br class="">+...<br class="">+-- Same test but dropping an INDEX ON t2.<br class="">+box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("CREATE TABLE t2(id PRIMARY KEY, a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("CREATE INDEX i1 ON t1(a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("CREATE INDEX i1 ON t2(a);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("INSERT INTO t1 VALUES(1, 2);")<br class="">+---<br class="">+...<br class="">+box.sql.execute("INSERT INTO t2 VALUES(1, 2);")<br class="">+---<br class="">+...<br class="">+-- Analyze.<br class="">+box.sql.execute("ANALYZE;")<br class="">+---<br class="">+...<br class="">+-- Checking the data.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+---<br class="">+- - ['T1', 'I1', '1', '0', '0', !!binary kQI=]<br class="">+  - ['T1', 'T1', '1', '0', '0', !!binary kQE=]<br class="">+  - ['T2', 'I1', '1', '0', '0', !!binary kQI=]<br class="">+  - ['T2', 'T2', '1', '0', '0', !!binary kQE=]<br class="">+...<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+---<br class="">+- - ['T1', 'I1', '1 1']<br class="">+  - ['T1', 'T1', '1 1']<br class="">+  - ['T2', 'I1', '1 1']<br class="">+  - ['T2', 'T2', '1 1']<br class="">+...<br class="">+-- Dropping an index<br class="">+box.sql.execute("DROP INDEX i1 ON t2;")<br class="">+---<br class="">+...<br class="">+-- Checking the DROP INDEX results.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+---<br class="">+- - ['T1', 'I1', '1', '0', '0', !!binary kQI=]<br class="">+  - ['T1', 'T1', '1', '0', '0', !!binary kQE=]<br class="">+  - ['T2', 'T2', '1', '0', '0', !!binary kQE=]<br class="">+...<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+---<br class="">+- - ['T1', 'I1', '1 1']<br class="">+  - ['T1', 'T1', '1 1']<br class="">+  - ['T2', 'T2', '1 1']<br class="">+...<br class="">+--Cleaning up.<br class="">+box.sql.execute("DROP TABLE t1;")<br class="">+---<br class="">+...<br class="">+box.sql.execute("DROP TABLE t2;")<br class="">+---<br class="">+...<br class="">diff --git a/test/sql/sql-statN-index-drop.test.lua b/test/sql/sql-statN-index-drop.test.lua<br class="">new file mode 100644<br class="">index 0000000..9b919de<br class="">--- /dev/null<br class="">+++ b/test/sql/sql-statN-index-drop.test.lua<br class="">@@ -0,0 +1,54 @@<br class="">+test_run = require('test_run').new()<br class="">+<br class="">+-- Initializing some things.<br class="">+box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a);")<br class="">+box.sql.execute("CREATE TABLE t2(id PRIMARY KEY, a);")<br class="">+box.sql.execute("CREATE INDEX i1 ON t1(a);")<br class="">+box.sql.execute("CREATE INDEX i1 ON t2(a);")<br class="">+box.sql.execute("INSERT INTO t1 VALUES(1, 2);")<br class="">+box.sql.execute("INSERT INTO t2 VALUES(1, 2);")<br class="">+<br class="">+-- Analyze.<br class="">+box.sql.execute("ANALYZE;")<br class="">+<br class="">+-- Checking the data.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+<br class="">+-- Dropping an index<br class=""></div></div></blockquote><div><br class=""></div><div>Nitpicking: put a dot at the end of sentence.</div><div class=""><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><div class="">+box.sql.execute("DROP INDEX i1 ON t1;")<br class="">+<br class="">+-- Checking the DROP INDEX results.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+<br class="">+--Cleaning up.<br class="">+box.sql.execute("DROP TABLE t1;")<br class="">+box.sql.execute("DROP TABLE t2;")<br class="">+<br class="">+-- Same test but dropping an INDEX ON t2.<br class="">+<br class="">+box.sql.execute("CREATE TABLE t1(id PRIMARY KEY, a);")<br class="">+box.sql.execute("CREATE TABLE t2(id PRIMARY KEY, a);")<br class="">+box.sql.execute("CREATE INDEX i1 ON t1(a);")<br class="">+box.sql.execute("CREATE INDEX i1 ON t2(a);")<br class="">+box.sql.execute("INSERT INTO t1 VALUES(1, 2);")<br class="">+box.sql.execute("INSERT INTO t2 VALUES(1, 2);")<br class="">+<br class="">+-- Analyze.<br class="">+box.sql.execute("ANALYZE;")<br class="">+<br class="">+-- Checking the data.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+<br class="">+-- Dropping an index<br class=""></div></div></blockquote><div><br class=""></div><div>Nitpicking: put a dot at the end of sentence.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="">+box.sql.execute("DROP INDEX i1 ON t2;")<br class="">+<br class="">+-- Checking the DROP INDEX results.<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat4\";")<br class="">+box.sql.execute("SELECT * FROM \"_sql_stat1\";")<br class="">+<br class="">+--Cleaning up.<br class="">+box.sql.execute("DROP TABLE t1;")<br class="">+box.sql.execute("DROP TABLE t2;")<br class="">-- <br class="">2.7.4<br class=""><br class=""><br class=""></div></div></blockquote></div><br class=""></body></html>