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 C50922282A for ; Tue, 10 Jul 2018 13:52:51 -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 225xeb0b2WGz for ; Tue, 10 Jul 2018 13:52:51 -0400 (EDT) Received: from smtp32.i.mail.ru (smtp32.i.mail.ru [94.100.177.92]) (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 25434217F3 for ; Tue, 10 Jul 2018 13:52:51 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH v2 1/4] sql: get rid off sqlite3NestedParse in clean stats From: "n.pettik" In-Reply-To: <3017d0e640eebdc6c3f28b5b4a4cbd2ef5646882.1531242355.git.kshcherbatov@tarantool.org> Date: Tue, 10 Jul 2018 20:52:47 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <3017d0e640eebdc6c3f28b5b4a4cbd2ef5646882.1531242355.git.kshcherbatov@tarantool.org> 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 Cc: Kirill Shcherbatov > On 10 Jul 2018, at 20:08, Kirill Shcherbatov = wrote: >=20 > 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. >=20 > --- 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[] =3D { > - "_sql_stat1", > - "_sql_stat4", > - NULL}; > - int i; > - sqlite3 *db =3D pParse->db; > - Vdbe *v =3D sqlite3GetVdbe(pParse); > - int aRoot[ArraySize(aTable)]; > - u8 aCreateTbl[ArraySize(aTable)]; > + const char *space_names[] =3D {"_sql_stat1", "_sql_stat4"}; You already have argument =E2=80=99table_name=E2=80=99, so it looks = confusing. Lets name it like =E2=80=98stat_names=E2=80=99. The same for space_ids. > + const uint32_t space_ids[] =3D {BOX_SQL_STAT1_ID, = BOX_SQL_STAT4_ID}; > + struct Vdbe *v =3D sqlite3GetVdbe(parse); > + assert(v !=3D NULL); > + assert(sqlite3VdbeDb(v) =3D=3D parse->db); >=20 > - if (v =3D=3D 0) > - return; > - assert(sqlite3VdbeDb(v) =3D=3D 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 =E2=80=99tables already exist since they are = system=E2=80=99. > + * clear them if they do already exist. > */ > - for (i =3D 0; aTable[i]; i++) { > - const char *zTab =3D aTable[i]; > - Table *pStat; > - /* The table already exists, because it is a system = space */ > - pStat =3D sqlite3HashFind(&db->pSchema->tblHash, zTab); > - assert(pStat !=3D NULL); > - aRoot[i] =3D pStat->tnum; > - aCreateTbl[i] =3D 0; > - if (zWhere) { > - sqlite3NestedParse(pParse, > - "DELETE FROM \"%s\" WHERE = \"%s\"=3D%Q", > - zTab, zWhereType, zWhere); > + for (uint i =3D 0; i < lengthof(space_names); ++i) { > + const char *space_name =3D space_names[i]; > + /* > + * The table already exists, because it is a > + * system space. > + */ > + assert(sqlite3HashFind(&parse->db->pSchema->tblHash, > + space_name) !=3D NULL); > + if (table_name !=3D NULL || index_name !=3D NULL) { I don=E2=80=99t understand situation when index_name !=3D NULL but = table_name =3D=3D 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 =3D=3D NULL. Hence, in vdbe_emit_stat_space_open() index_name is always =3D=3D 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();