[tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats

  • From: "n.pettik" <korablev@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx
  • Date: Tue, 10 Jul 2018 20:52:47 +0300


On 10 Jul 2018, at 20:08, Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx> 
wrote:

Now we manually generate AST structures to drop outdated
stats from _sql_stat1 and _sql_stat4 spaces instead of
starting sqlite3NestedParse. This function become totally

Typo: becomes.


--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -116,71 +116,53 @@
+vdbe_emit_stat_space_open(struct Parse *parse, int stat_cursor,
+                       const char *index_name, const char *table_name)
{
-     const char *aTable[] = {
-             "_sql_stat1",
-             "_sql_stat4",
-             NULL};
-     int i;
-     sqlite3 *db = pParse->db;
-     Vdbe *v = sqlite3GetVdbe(pParse);
-     int aRoot[ArraySize(aTable)];
-     u8 aCreateTbl[ArraySize(aTable)];
+     const char *space_names[] = {"_sql_stat1", "_sql_stat4"};

You already have argument ’table_name’, so it looks confusing.
Lets name it like ‘stat_names’. The same for space_ids.

+     const uint32_t space_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};
+     struct Vdbe *v = sqlite3GetVdbe(parse);
+     assert(v != NULL);
+     assert(sqlite3VdbeDb(v) == parse->db);

-     if (v == 0)
-             return;
-     assert(sqlite3VdbeDb(v) == db);
-
-     /* Create new statistic tables if they do not exist, or clear them
-      * if they do already exist.
+     /*
+      * Create new statistic tables if they do not exist, or

But comment below says that ’tables already exist since they are system’.

+      * clear them if they do already exist.
       */
-     for (i = 0; aTable[i]; i++) {
-             const char *zTab = aTable[i];
-             Table *pStat;
-             /* The table already exists, because it is a system space */
-             pStat = sqlite3HashFind(&db->pSchema->tblHash, zTab);
-             assert(pStat != NULL);
-             aRoot[i] = pStat->tnum;
-             aCreateTbl[i] = 0;
-             if (zWhere) {
-                     sqlite3NestedParse(pParse,
-                                        "DELETE FROM \"%s\" WHERE \"%s\"=%Q",
-                                        zTab, zWhereType, zWhere);
+     for (uint i = 0; i < lengthof(space_names); ++i) {
+             const char *space_name = space_names[i];
+             /*
+              * The table already exists, because it is a
+              * system space.
+              */
+             assert(sqlite3HashFind(&parse->db->pSchema->tblHash,
+                                    space_name) != NULL);
+             if (table_name != NULL || index_name != NULL) {

I don’t understand situation when index_name != NULL but table_name == NULL.
In our SQL index name is local to table, (i.e. indexes with the same name could
exist within different tables). I see that vdbe_emit_stat_space_open() is 
called twice
in analyzeTable() and analyzeTable() in turn always called with  pOnlyIdx == 
NULL.
Hence, in vdbe_emit_stat_space_open() index_name is always == NULL.
Lets remove this dead code and excess arg.

+                     vdbe_emit_stat_space_clear(parse, space_name,
+                                                index_name, table_name);
              } else {
-                     /*
-                      * The sql_stat[134] table already exists.
-                      * Delete all rows.
-                      */
-                     sqlite3VdbeAddOp2(v, OP_Clear,
-                                       SQLITE_PAGENO_TO_SPACEID(aRoot[i]), 
0);
+                     sqlite3VdbeAddOp2(v, OP_Clear, space_ids[i], 0);

OP_Clear uses only first operand, so you can use sqlite3VdbeAddOp1();


Other related posts:

  1. Home
  2. tarantool-patches
  3. Archive
  4. 07-2018