From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B7E43200A4 for ; Wed, 11 Jul 2018 03:22:35 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id EtLQ6iv1gViA for ; Wed, 11 Jul 2018 03:22:35 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 71C332005A for ; Wed, 11 Jul 2018 03:22:35 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats References: <3017d0e640eebdc6c3f28b5b4a4cbd2ef5646882.1531242355.git.kshcherbatov@tarantool.org> From: Kirill Shcherbatov Message-ID: <22654f98-1eb9-9196-4400-648670afc12e@tarantool.org> Date: Wed, 11 Jul 2018 10:22:33 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, Nikita Pettik 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]);