[tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats
n.pettik
korablev at tarantool.org
Tue Jul 10 20:52:47 MSK 2018
> On 10 Jul 2018, at 20:08, Kirill Shcherbatov <kshcherbatov at tarantool.org> 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();
More information about the Tarantool-patches
mailing list