[tarantool-patches] Re: [PATCH 3/7] sql: remove struct Table from analyze routine

n.pettik korablev at tarantool.org
Mon Sep 3 02:52:03 MSK 2018


>>  static void
>> -analyzeOneTable(Parse * pParse,	/* Parser context */
>> -		Table * pTab,	/* Table whose indices are to be analyzed */
>> -		int iStatCur,	/* Index of VdbeCursor that writes the _sql_stat1 table */
>> -		int iMem,	/* Available memory locations begin here */
>> -		int iTab	/* Next available cursor */
>> -    )
>> +vdbe_emit_analyze_space(struct Parse *pParse, struct space *space,
>> +			int stat_cursor)
> 
> 1. Once you started this function formatting, please, finish it.
> Use parse instead of pParse. Use reg_stat4 instead of regStat4 etc.

Ok, check it out:

--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -768,63 +768,53 @@ callStatGet(Vdbe * v, int regStat4, int iParam, int regOut)
  * Generate code to do an analysis of all indices associated with
  * a single table.
  *
- * @param pParse Current parsing context.
+ * @param parse Current parsing context.
  * @param space Space to be analyzed.
  * @param stat_cursor Cursor pointing to spaces containing
  *        statistics: _sql_stat1 (stat_cursor) and
  *        _sql_stat4 (stat_cursor + 1).
  */
 static void
-vdbe_emit_analyze_space(struct Parse *pParse, struct space *space,
+vdbe_emit_analyze_space(struct Parse *parse, struct space *space,
                        int stat_cursor)
 {
-       sqlite3 *db = pParse->db;       /* Database handle */
-       int iIdxCur;            /* Cursor open on index being analyzed */
-       int iTabCur;            /* Table cursor */
-       Vdbe *v;                /* The virtual machine being built up */
-       int i;                  /* Loop counter */
+       assert(space != NULL);
        /* Register to hold Stat4Accum object. */
-       int regStat4 = ++pParse->nMem;
+       int stat4_reg = ++parse->nMem;
        /* Index of changed index field. */
-       int regChng = ++pParse->nMem;
+       int chng_reg = ++parse->nMem;
        /* Key argument passed to stat_push(). */
-       int regKey = ++pParse->nMem;
+       int key_reg = ++parse->nMem;
        /* Temporary use register. */
-       int regTemp = ++pParse->nMem;
+       int tmp_reg = ++parse->nMem;
        /* Register containing table name. */
-       int regTabname = ++pParse->nMem;
+       int tab_name_reg = ++parse->nMem;
        /* Register containing index name. */
-       int regIdxname = ++pParse->nMem;
+       int idx_name_reg = ++parse->nMem;
        /* Value for the stat column of _sql_stat1. */
-       int regStat1 = ++pParse->nMem;
+       int stat1_reg = ++parse->nMem;
        /* MUST BE LAST (see below). */
-       int regPrev = ++pParse->nMem;
-
-       assert(space != NULL);
-       v = sqlite3GetVdbe(pParse);
-       assert(v != NULL);
-       if (sqlite3_strlike("\\_%", space->def->name, '\\') == 0) {
-               /* Do not gather statistics on system tables */
+       int prev_reg = ++parse->nMem;
+       /* Do not gather statistics on system tables. */
+       if (space_is_system(space))
                return;
-       }
-
        /*
         * Open a read-only cursor on the table. Also allocate
         * a cursor number to use for scanning indexes.
         */
-       iTabCur = pParse->nTab;
-       pParse->nTab += 2;
+       int tab_cursor = parse->nTab;
+       parse->nTab += 2;
        assert(space->index_count != 0);
-       sqlite3VdbeAddOp4(v, OP_OpenRead, iTabCur, 0, 0, (void *) space,
+       struct Vdbe *v = sqlite3GetVdbe(parse);
+       assert(v != NULL);
+       sqlite3VdbeAddOp4(v, OP_OpenRead, tab_cursor, 0, 0, (void *) space,
                          P4_SPACEPTR);
-       sqlite3VdbeLoadString(v, regTabname, space->def->name);
+       sqlite3VdbeLoadString(v, tab_name_reg, space->def->name);
        for (uint32_t j = 0; j < space->index_count; ++j) {
                struct index *idx = space->index[j];
-               int addrRewind; /* Address of "OP_Rewind iIdxCur" */
-               int addrNextRow;        /* Address of "next_row:" */
-               const char *idx_name;   /* Name of the index */
-
-               /* Primary indexes feature automatically generated
+               const char *idx_name;
+               /*
+                * Primary indexes feature automatically generated
                 * names. Thus, for the sake of clarity, use
                 * instead more familiar table name.
                 */
@@ -833,109 +823,114 @@ vdbe_emit_analyze_space(struct Parse *pParse, struct space *space,
                else
                        idx_name = idx->def->name;
                int part_count = idx->def->key_def->part_count;
-
                /* Populate the register containing the index name. */
-               sqlite3VdbeLoadString(v, regIdxname, idx_name);
+               sqlite3VdbeLoadString(v, idx_name_reg, idx_name);
                VdbeComment((v, "Analysis for %s.%s", space->def->name,
                            idx_name));
-
                /*
                 * Pseudo-code for loop that calls stat_push():
                 *
                 *   Rewind csr
                 *   if eof(csr) goto end_of_scan;
-                *   regChng = 0
+                *   chng_reg = 0
                 *   goto chng_addr_0;
                 *
                 *  next_row:
-                *   regChng = 0
-                *   if( idx(0) != regPrev(0) ) goto chng_addr_0
-                *   regChng = 1
-                *   if( idx(1) != regPrev(1) ) goto chng_addr_1
+                *   chng_reg = 0
+                *   if( idx(0) != prev_reg(0) ) goto chng_addr_0
+                *   chng_reg = 1
+                *   if( idx(1) != prev_reg(1) ) goto chng_addr_1
                 *   ...
-                *   regChng = N
+                *   chng_reg = N
                 *   goto chng_addr_N
                 *
                 *  chng_addr_0:
-                *   regPrev(0) = idx(0)
+                *   prev_reg(0) = idx(0)
                 *  chng_addr_1:
-                *   regPrev(1) = idx(1)
+                *   prev_reg(1) = idx(1)
                 *  ...
                 *
-                *  endDistinctTest:
-                *   regKey = idx(key)
-                *   stat_push(P, regChng, regKey)
+                *  distinct_addr:
+                *   key_reg = idx(key)
+                *   stat_push(P, chng_reg, key_reg)
                 *   Next csr
                 *   if !eof(csr) goto next_row;
                 *
                 *  end_of_scan:
                 */
 
-               /* Make sure there are enough memory cells allocated to accommodate
-                * the regPrev array and a trailing key (the key slot is required
-                * when building a record to insert into the sample column of
-                * the _sql_stat4 table).
+               /*
+                * Make sure there are enough memory cells
+                * allocated to accommodate the prev_reg array
+                * and a trailing key (the key slot is required
+                * when building a record to insert into
+                * the sample column of the _sql_stat4 table).
                 */
-               pParse->nMem = MAX(pParse->nMem, regPrev + part_count);
-
-               /* Open a read-only cursor on the index being analyzed. */
+               parse->nMem = MAX(parse->nMem, prev_reg + part_count);
+               /* Open a cursor on the index being analyzed. */
+               int idx_cursor;
                if (j != 0) {
-                       iIdxCur = pParse->nTab - 1;
-                       sqlite3VdbeAddOp4(v, OP_OpenRead, iIdxCur,
+                       idx_cursor = parse->nTab - 1;
+                       sqlite3VdbeAddOp4(v, OP_OpenRead, idx_cursor,
                                          idx->def->iid, 0,
                                          (void *) space, P4_SPACEPTR);
                        VdbeComment((v, "%s", idx->def->name));
                } else {
                        /* We have already opened cursor on PK. */
-                       iIdxCur = iTabCur;
+                       idx_cursor = tab_cursor;
                }
-
-               /* Invoke the stat_init() function. The arguments are:
-                *
-                *    (1) the number of columns in the index
-                *        (including the number of PK columns)
-                *    (2) the number of columns in the key without the pk
-                *    (3) the number of rows in the index
-                *
+               /*
+                * Invoke the stat_init() function.
+                * The arguments are:
+                *  (1) The number of columns in the index
+                *      (including the number of PK columns);
+                *  (2) The number of columns in the key
+                *       without the pk;
+                *  (3) the number of rows in the index;
+                * FIXME: for Tarantool first and second args
+                * are the same.
                 *
                 * The third argument is only used for STAT4
                 */
-               sqlite3VdbeAddOp2(v, OP_Count, iIdxCur, regStat4 + 3);
-               sqlite3VdbeAddOp2(v, OP_Integer, part_count, regStat4 + 1);
-               sqlite3VdbeAddOp2(v, OP_Integer, part_count, regStat4 + 2);
-               sqlite3VdbeAddOp4(v, OP_Function0, 0, regStat4 + 1, regStat4,
+               sqlite3VdbeAddOp2(v, OP_Count, idx_cursor, stat4_reg + 3);
+               sqlite3VdbeAddOp2(v, OP_Integer, part_count, stat4_reg + 1);
+               sqlite3VdbeAddOp2(v, OP_Integer, part_count, stat4_reg + 2);
+               sqlite3VdbeAddOp4(v, OP_Function0, 0, stat4_reg + 1, stat4_reg,
                                  (char *)&statInitFuncdef, P4_FUNCDEF);
                sqlite3VdbeChangeP5(v, 3);
-
-               /* Implementation of the following:
+               /*
+                * Implementation of the following:
                 *
                 *   Rewind csr
                 *   if eof(csr) goto end_of_scan;
-                *   regChng = 0
+                *   chng_reg = 0
                 *   goto next_push_0;
-                *
                 */
-               addrRewind = sqlite3VdbeAddOp1(v, OP_Rewind, iIdxCur);
-               VdbeCoverage(v);
-               sqlite3VdbeAddOp2(v, OP_Integer, 0, regChng);
-               int endDistinctTest = sqlite3VdbeMakeLabel(v);
+               int rewind_addr = sqlite3VdbeAddOp1(v, OP_Rewind, idx_cursor);
+               sqlite3VdbeAddOp2(v, OP_Integer, 0, chng_reg);
+               int distinct_addr = sqlite3VdbeMakeLabel(v);
                /* Array of jump instruction addresses. */
-               int *aGotoChng =
-                       sqlite3DbMallocRawNN(db, sizeof(int) * part_count);
-               if (aGotoChng == NULL)
-                       continue;
+               int *jump_addrs = region_alloc(&parse->region,
+                                              sizeof(int) * part_count);
+               if (jump_addrs == NULL) {
+                       diag_set(OutOfMemory, sizeof(int) * part_count,
+                                "region", "jump_addrs");
+                       parse->rc = SQL_TARANTOOL_ERROR;
+                       parse->nErr++;
+                       return;
+               }
                /*
                 *  next_row:
-                *   regChng = 0
-                *   if( idx(0) != regPrev(0) ) goto chng_addr_0
-                *   regChng = 1
-                *   if( idx(1) != regPrev(1) ) goto chng_addr_1
+                *   chng_reg = 0
+                *   if( idx(0) != prev_reg(0) ) goto chng_addr_0
+                *   chng_reg = 1
+                *   if( idx(1) != prev_reg(1) ) goto chng_addr_1
                 *   ...
-                *   regChng = N
-                *   goto endDistinctTest
+                *   chng_reg = N
+                *   goto distinct_addr
                 */
                sqlite3VdbeAddOp0(v, OP_Goto);
-               addrNextRow = sqlite3VdbeCurrentAddr(v);
+               int next_row_addr = sqlite3VdbeCurrentAddr(v);
                if (part_count == 1 && idx->def->opts.is_unique) {
                        /*
                         * For a single-column UNIQUE index, once
@@ -943,122 +938,110 @@ vdbe_emit_analyze_space(struct Parse *pParse, struct space *space,
                         * that all the rest will be distinct, so
                         * skip subsequent distinctness tests.
                         */
-                       sqlite3VdbeAddOp2(v, OP_NotNull, regPrev,
-                                         endDistinctTest);
-                       VdbeCoverage(v);
+                       sqlite3VdbeAddOp2(v, OP_NotNull, prev_reg,
+                                         distinct_addr);
                }
                struct key_part *part = idx->def->key_def->parts;
-               for (i = 0; i < part_count; ++i, ++part) {
+               for (int i = 0; i < part_count; ++i, ++part) {
                        struct coll *coll = part->coll;
-                       sqlite3VdbeAddOp2(v, OP_Integer, i, regChng);
-                       sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, part->fieldno,
-                                         regTemp);
-                       aGotoChng[i] = sqlite3VdbeAddOp4(v, OP_Ne, regTemp, 0,
-                                                        regPrev + i,
+                       sqlite3VdbeAddOp2(v, OP_Integer, i, chng_reg);
+                       sqlite3VdbeAddOp3(v, OP_Column, idx_cursor,
+                                         part->fieldno, tmp_reg);
+                       jump_addrs[i] = sqlite3VdbeAddOp4(v, OP_Ne, tmp_reg, 0,
+                                                        prev_reg + i,
                                                         (char *)coll,
                                                         P4_COLLSEQ);
                        sqlite3VdbeChangeP5(v, SQLITE_NULLEQ);
-                       VdbeCoverage(v);
                }
-               sqlite3VdbeAddOp2(v, OP_Integer, part_count, regChng);
-               sqlite3VdbeGoto(v, endDistinctTest);
-
+               sqlite3VdbeAddOp2(v, OP_Integer, part_count, chng_reg);
+               sqlite3VdbeGoto(v, distinct_addr);
                /*
                 *  chng_addr_0:
-                *   regPrev(0) = idx(0)
+                *   prev_reg(0) = idx(0)
                 *  chng_addr_1:
-                *   regPrev(1) = idx(1)
+                *   prev_reg(1) = idx(1)
                 *  ...
                 */
-               sqlite3VdbeJumpHere(v, addrNextRow - 1);
+               sqlite3VdbeJumpHere(v, next_row_addr - 1);
                part = idx->def->key_def->parts;
-               for (i = 0; i < part_count; ++i, ++part) {
-                       sqlite3VdbeJumpHere(v, aGotoChng[i]);
-                       sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, part->fieldno,
-                                         regPrev + i);
+               for (int i = 0; i < part_count; ++i, ++part) {
+                       sqlite3VdbeJumpHere(v, jump_addrs[i]);
+                       sqlite3VdbeAddOp3(v, OP_Column, idx_cursor,
+                                         part->fieldno, prev_reg + i);
                }
-               sqlite3VdbeResolveLabel(v, endDistinctTest);
-               sqlite3DbFree(db, aGotoChng);
-
+               sqlite3VdbeResolveLabel(v, distinct_addr);
                /*
                 *  chng_addr_N:
-                *   regKey = idx(key)              // STAT4 only
-                *   stat_push(P, regChng, regKey)  // 3rd parameter STAT4 only
+                *   key_reg = idx(key)
+                *   stat_push(P, chng_reg, key_reg)
                 *   Next csr
                 *   if !eof(csr) goto next_row;
                 */
-               assert(regKey == (regStat4 + 2));
+               assert(key_reg == (stat4_reg + 2));
                struct index *pk = space_index(space, 0);
                int pk_part_count = pk->def->key_def->part_count;
                /* Allocate memory for array. */
-               pParse->nMem = MAX(pParse->nMem,
-                                  regPrev + part_count + pk_part_count);
-               int regKeyStat = regPrev + part_count;
-               for (i = 0; i < pk_part_count; i++) {
+               parse->nMem = MAX(parse->nMem,
+                                 prev_reg + part_count + pk_part_count);
+               int stat_key_reg = prev_reg + part_count;
+               for (int i = 0; i < pk_part_count; i++) {
                        uint32_t k = pk->def->key_def->parts[i].fieldno;
                        assert(k < space->def->field_count);
-                       sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, k,
-                                         regKeyStat + i);
+                       sqlite3VdbeAddOp3(v, OP_Column, idx_cursor, k,
+                                         stat_key_reg + i);
                        VdbeComment((v, "%s", space->def->fields[k].name));
                }
-               sqlite3VdbeAddOp3(v, OP_MakeRecord, regKeyStat,
-                                 pk_part_count, regKey);
-
-               assert(regChng == (regStat4 + 1));
-               sqlite3VdbeAddOp4(v, OP_Function0, 1, regStat4, regTemp,
+               sqlite3VdbeAddOp3(v, OP_MakeRecord, stat_key_reg,
+                                 pk_part_count, key_reg);
+               assert(chng_reg == (stat4_reg + 1));
+               sqlite3VdbeAddOp4(v, OP_Function0, 1, stat4_reg, tmp_reg,
                                  (char *)&statPushFuncdef, P4_FUNCDEF);
                sqlite3VdbeChangeP5(v, 3);
-               sqlite3VdbeAddOp2(v, OP_Next, iIdxCur, addrNextRow);
-               VdbeCoverage(v);
-
+               sqlite3VdbeAddOp2(v, OP_Next, idx_cursor, next_row_addr);
                /* Add the entry to the stat1 table. */
-               callStatGet(v, regStat4, STAT_GET_STAT1, regStat1);
+               callStatGet(v, stat4_reg, STAT_GET_STAT1, stat1_reg);
                assert("BBB"[0] == AFFINITY_TEXT);
-               sqlite3VdbeAddOp4(v, OP_MakeRecord, regTabname, 3, regTemp,
+               sqlite3VdbeAddOp4(v, OP_MakeRecord, tab_name_reg, 3, tmp_reg,
                                  "BBB", 0);
-               sqlite3VdbeAddOp2(v, OP_IdxInsert, stat_cursor, regTemp);
-
+               sqlite3VdbeAddOp2(v, OP_IdxInsert, stat_cursor, tmp_reg);
                /* Add the entries to the stat4 table. */
-
-               int regEq = regStat1;
-               int regLt = regStat1 + 1;
-               int regDLt = regStat1 + 2;
-               int regSample = regStat1 + 3;
-               int regCol = regStat1 + 4;
-               int regSampleKey = regCol + part_count;
-               int addrNext;
-               int addrIsNull;
-
-               pParse->nMem = MAX(pParse->nMem, regCol + part_count);
-
-               addrNext = sqlite3VdbeCurrentAddr(v);
-               callStatGet(v, regStat4, STAT_GET_KEY, regSampleKey);
-               addrIsNull = sqlite3VdbeAddOp1(v, OP_IsNull, regSampleKey);
-               VdbeCoverage(v);
-               callStatGet(v, regStat4, STAT_GET_NEQ, regEq);
-               callStatGet(v, regStat4, STAT_GET_NLT, regLt);
-               callStatGet(v, regStat4, STAT_GET_NDLT, regDLt);
-               sqlite3VdbeAddOp4Int(v, OP_NotFound, iTabCur, addrNext,
-                                    regSampleKey, 0);
-               /* We know that the regSampleKey row exists because it was read by
-                * the previous loop.  Thus the not-found jump of seekOp will never
-                * be taken
+               int eq_reg = stat1_reg;
+               int lt_reg = stat1_reg + 1;
+               int dlt_reg = stat1_reg + 2;
+               int sample_reg = stat1_reg + 3;
+               int col_reg = stat1_reg + 4;
+               int sample_key_reg = col_reg + part_count;
+               parse->nMem = MAX(parse->nMem, col_reg + part_count);
+               int next_addr = sqlite3VdbeCurrentAddr(v);
+               callStatGet(v, stat4_reg, STAT_GET_KEY, sample_key_reg);
+               int is_null_addr = sqlite3VdbeAddOp1(v, OP_IsNull,
+                                                    sample_key_reg);
+               callStatGet(v, stat4_reg, STAT_GET_NEQ, eq_reg);
+               callStatGet(v, stat4_reg, STAT_GET_NLT, lt_reg);
+               callStatGet(v, stat4_reg, STAT_GET_NDLT, dlt_reg);
+               sqlite3VdbeAddOp4Int(v, OP_NotFound, tab_cursor, next_addr,
+                                    sample_key_reg, 0);
+               /*
+                * We know that the sample_key_reg row exists
+                * because it was read by the previous loop.
+                * Thus the not-found jump of seekOp will never
+                * be taken.
                 */
-               VdbeCoverageNeverTaken(v);
-               for (i = 0; i < part_count; i++) {
+               for (int i = 0; i < part_count; i++) {
                        uint32_t tabl_col = idx->def->key_def->parts[i].fieldno;
-                       sqlite3ExprCodeGetColumnOfTable(v, space->def, iTabCur,
-                                                       tabl_col, regCol + i);
+                       sqlite3ExprCodeGetColumnOfTable(v, space->def,
+                                                       tab_cursor, tabl_col,
+                                                       col_reg + i);
                }
-               sqlite3VdbeAddOp3(v, OP_MakeRecord, regCol, part_count,
-                                 regSample);
-               sqlite3VdbeAddOp3(v, OP_MakeRecord, regTabname, 6, regTemp);
-               sqlite3VdbeAddOp2(v, OP_IdxReplace, stat_cursor + 1, regTemp);
-               sqlite3VdbeAddOp2(v, OP_Goto, 1, addrNext);     /* P1==1 for end-of-loop */
-               sqlite3VdbeJumpHere(v, addrIsNull);
-
-               /* End of analysis */
-               sqlite3VdbeJumpHere(v, addrRewind);
+               sqlite3VdbeAddOp3(v, OP_MakeRecord, col_reg, part_count,
+                                 sample_reg);
+               sqlite3VdbeAddOp3(v, OP_MakeRecord, tab_name_reg, 6, tmp_reg);
+               sqlite3VdbeAddOp2(v, OP_IdxReplace, stat_cursor + 1, tmp_reg);
+               /* P1==1 for end-of-loop. */
+               sqlite3VdbeAddOp2(v, OP_Goto, 1, next_addr);
+               sqlite3VdbeJumpHere(v, is_null_addr);
+               /* End of analysis. */
+               sqlite3VdbeJumpHere(v, rewind_addr);
        }
 }



More information about the Tarantool-patches mailing list