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

  • From: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>
  • To: tarantool-patches@xxxxxxxxxxxxx, Nikita Pettik <korablev@xxxxxxxxxxxxx>
  • Date: Wed, 11 Jul 2018 10:22:33 +0300

Thank you for review.

Typo: becomes.
Ok, tnx.

You already have argument ’table_name’, so it looks confusing.
Lets name it like ‘stat_names’. The same for space_ids.
-       const char *space_names[] = {"_sql_stat1", "_sql_stat4"};
-       const uint32_t space_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};
+       const char *stat_names[] = {"_sql_stat1", "_sql_stat4"};
+       const uint32_t stat_ids[] = {BOX_SQL_STAT1_ID, BOX_SQL_STAT4_ID};

But comment below says that ’tables already exist since they are system’.
Hmm, loks like some legacy. 
-
-       /*
-        * Create new statistic tables if they do not exist, or
-        * clear them if they do already exist.
-        */


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

-               if (table_name != NULL || index_name != NULL) {
+               if (table_name != NULL) {

/**
 * Generate code that will do an analysis of a single table in
 * a database.
 *
 * @param parse Parser context.
 * @param table Target table to analyze.
 */
static void
vdbe_emit_analyze_table(struct Parse *parse, struct Table *table)
{
        assert(table != NULL);
        sql_set_multi_write(parse, false);
        int stat_cursor = parse->nTab;
        parse->nTab += 3;
        vdbe_emit_stat_space_open(parse, stat_cursor, NULL, table->def->name);
        analyzeOneTable(parse, table, NULL, stat_cursor, parse->nMem + 1,
                        parse->nTab);
        loadAnalysis(parse);
}


OP_Clear uses only first operand, so you can use sqlite3VdbeAddOp1();
-                       sqlite3VdbeAddOp2(v, OP_Clear, stat_ids[i], 0);
+                       sqlite3VdbeAddOp1(v, OP_Clear, stat_ids[i]);


Other related posts:

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