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 5DC0F2DE38 for ; Fri, 6 Apr 2018 22:12:58 -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 CDz8MRPalGzk for ; Fri, 6 Apr 2018 22:12:58 -0400 (EDT) Received: from mail-lf0-f46.google.com (mail-lf0-f46.google.com [209.85.215.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id AF19D2DE37 for ; Fri, 6 Apr 2018 22:12:57 -0400 (EDT) Received: by mail-lf0-f46.google.com with SMTP id e5-v6so2920708lfb.7 for ; Fri, 06 Apr 2018 19:12:57 -0700 (PDT) MIME-Version: 1.0 References: <1522791436-8221-1-git-send-email-hollow653@gmail.com> <926E041C-FCCB-46B0-B49B-21292CB70813@tarantool.org> In-Reply-To: From: Hollow111 Date: Sat, 07 Apr 2018 02:12:45 +0000 Message-ID: Subject: [tarantool-patches] Re: [PATCH 2/2] sql: statistics removal after dropping an index Content-Type: multipart/alternative; boundary="f4f5e80742b8fe3142056938b519" 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: korablev@tarantool.org, tarantool-patches@freelists.org --f4f5e80742b8fe3142056938b519 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable =D0=BF=D1=82, 6 =D0=B0=D0=BF=D1=80. 2018 =D0=B3. =D0=B2 3:42, Hollow111 : > > > =D1=87=D1=82, 5 =D0=B0=D0=BF=D1=80. 2018 =D0=B3. =D0=B2 21:01, n.pettik <= korablev@tarantool.org>: > >> Hello. >> >> See 2 minor remarks. The rest seems to be OK. >> >> I have noticed, that there are wrong indentations on your branch: >> >> @@ -2415,7 +2426,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, >> int isView, int noErr) >> */ >> >> sqlite3BeginWriteOperation(pParse, 1); >> - sqlite3ClearStatTables(pParse, "tbl", pTab->zName); >> + sql_clear_stat_tables(pParse, pTab->zName, NULL); >> >> @@ -3417,7 +3428,7 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, >> Token * pName2, int ifExists) >> * But firstly, delete statistics since schema >> * changes after DDL. >> */ >> - sqlite3ClearStatTables(pParse, "idx", pIndex->zName); >> + sql_clear_stat_tables(pParse, pIndex->pTable->zName, pIndex->zName)= ; >> >> Please, fix them. >> >> > On 4 Apr 2018, at 19:34, Hollow111 wrote: >> > >> > @@ -2210,16 +2210,17 @@ sqliteViewResetAll(sqlite3 * db) >> > * Remove entries from the _sql_statN tables (for N in (1, 4)) >> > * after a DROP INDEX or DROP TABLE command. >> > * >> > - * @param table_name table to be dropped or >> > - * the table that contains index to be dropped >> > - * @param idx_name index to be dropped >> > + * @param parse The parsing context. >> > + * @param table_name The table to be dropped or >> > + * the table that contains index to be dropped. >> > + * @param idx_name Index to be dropped. >> > */ >> > static void >> > -sql_clear_stat_tables(Parse *parse, >> > - const char *table_name, >> > - const char *idx_name) >> > +sql_clear_stat_tables(Parse *parse, const char *table_name, >> > + const char *idx_name >> > + ) >> >> This bracket should be on previous line. >> >> > { >> > - if(idx_name) { >> > + if(idx_name !=3D NULL) { >> > sqlite3NestedParse(parse, >> > "DELETE FROM \"_sql_stat1\" WHERE (\"idx\"=3D%Q A= ND " >> > "\"tbl\"=3D%Q)", >> > >> >> > After considering the remarks changes were made for the newer build of > Tarantool 2.0: > > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > index 92f3cb6..7ca4191 100644 > --- a/src/box/sql/build.c > +++ b/src/box/sql/build.c > @@ -2128,19 +2128,33 @@ sqliteViewResetAll(sqlite3 * db) > /** > * Remove entries from the _sql_stat1 and _sql_stat4 > * system spaces after a DROP INDEX or DROP TABLE command. > - * > - * @param pParse Parsing context. > - * @param zType Type of entry to be deleted: > - * 'idx' or 'tbl' string literal. > - * @param zName Name of index or table. > + * > + * @param parse The parsing context. > + * @param table_name The table to be dropped or > + * the table that contains index to be dropped. > + * @param idx_name Index to be dropped. > */ > static void > -sql_clear_stat_spaces(Parse * pParse, const char *zType, const char > *zName) > +sql_clear_stat_tables(Parse *parse, const char *table_name, > + const char *idx_name) > { > - sqlite3NestedParse(pParse, "DELETE FROM \"_sql_stat1\" WHERE \"%s\"=3D%= Q", > - zType, zName); > - sqlite3NestedParse(pParse, "DELETE FROM \"_sql_stat4\" WHERE \"%s\"=3D%= Q", > - zType, zName); > + if (idx_name !=3D NULL) { > + sqlite3NestedParse(parse, > + "DELETE FROM \"_sql_stat1\" WHERE (\"idx\"=3D%Q AND = " > + "\"tbl\"=3D%Q)", > + idx_name, table_name); > + sqlite3NestedParse(parse, > + "DELETE FROM \"_sql_stat4\" WHERE (\"idx\"=3D%Q AND = " > + "\"tbl\"=3D%Q)", > + idx_name, table_name); > + } else { > + sqlite3NestedParse(parse, > + "DELETE FROM \"_sql_stat1\" WHERE \"tbl\"=3D%Q", > + table_name); > + sqlite3NestedParse(parse, > + "DELETE FROM \"_sql_stat4\" WHERE \"tbl\"=3D%Q", > + table_name); > + } > } > > /** > @@ -2325,7 +2339,7 @@ sql_drop_table(struct Parse *parse_context, struct > SrcList *table_name_list, > * tuple with corresponding space_id from _space. > */ > > - sql_clear_stat_spaces(parse_context, "tbl", space_name); > + sql_clear_stat_tables(parse_context, space_name, NULL); > struct Table *tab =3D sqlite3HashFind(&db->pSchema->tblHash, space_name= ); > sqlite3FkDropTable(parse_context, table_name_list, tab); > sql_code_drop_table(parse_context, space, is_view); > @@ -3328,7 +3342,7 @@ sql_drop_index(struct Parse *parse_context, struct > SrcList *index_name_list, > * But firstly, delete statistics since schema > * changes after DDL. > */ > - sql_clear_stat_spaces(parse_context, "idx", index->def->name); > + sql_clear_stat_tables(parse_context, table_name, index->def->name); > int record_reg =3D ++parse_context->nMem; > int space_id_reg =3D ++parse_context->nMem; > sqlite3VdbeAddOp2(v, OP_Integer, space_id, space_id_reg); > > --f4f5e80742b8fe3142056938b519 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=D0=BF= =D1=82, 6 =D0=B0=D0=BF=D1=80. 2018 =D0=B3. =D0=B2 3:42, Hollow111 <hollow653@gmail.com>:


=D1=87=D1=82, 5 =D0=B0=D0=BF=D1=80. 2018 =D0=B3. =D0= =B2 21:01, n.pettik <korablev@tarantool.org>:
Hello.

See 2 minor remarks. The rest seems to be OK.

I have noticed, that there are wrong indentations on your branch:

@@ -2415,7 +2426,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int= isView, int noErr)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/

=C2=A0 =C2=A0 =C2=A0 =C2=A0 sqlite3BeginWriteOperation(pParse, 1);
-=C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3ClearStatTables(pParse, "tbl",= pTab->zName);
+=C2=A0 =C2=A0 sql_clear_stat_tables(pParse, pTab->zName, NULL);

@@ -3417,7 +3428,7 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Tok= en * pName2, int ifExists)
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* But firstly, delete statistics since sc= hema
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* changes after DDL.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
-=C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3ClearStatTables(pParse, "idx",= pIndex->zName);
+=C2=A0 =C2=A0 sql_clear_stat_tables(pParse, pIndex->pTable->zName, p= Index->zName);

Please, fix them.

> On 4 Apr 2018, at 19:34, Hollow111 <hollow653@gmail.com> wrote:
>
> @@ -2210,16 +2210,17 @@ sqliteViewResetAll(sqlite3 * db)
>=C2=A0 =C2=A0* Remove entries from the _sql_statN tables (for N in (1, = 4))
>=C2=A0 =C2=A0* after a DROP INDEX or DROP TABLE command.
>=C2=A0 =C2=A0*
> - * @param table_name table to be dropped or
> - *=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0the table that contains index to be dropped
> - * @param idx_name index to be dropped
> + * @param parse=C2=A0 =C2=A0 =C2=A0 The parsing context.
> + * @param table_name The table to be dropped or
> + *=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0the table that contains index to be dropped.
> + * @param idx_name=C2=A0 =C2=A0Index to be dropped.
>=C2=A0 =C2=A0*/
>=C2=A0 static void
> -sql_clear_stat_tables(Parse *parse,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0const char *ta= ble_name,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= const char *idx_name)
> +sql_clear_stat_tables(Parse *parse, const char *table_name,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 const char *idx_name
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0)

This bracket should be on previous line.

>=C2=A0 {
> -=C2=A0 =C2=A0 if(idx_name) {
> +=C2=A0 =C2=A0 if(idx_name !=3D NULL) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sqlite3NestedParse(parse,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 "DELETE FROM \"_sql_stat1\" WHERE (\"idx\"= =3D%Q AND "
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 "\"tbl\"=3D%Q)",
>


After considering the remarks changes = were made for the newer build of Tarantool 2.0:

di= ff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 92f3cb= 6..7ca4191 100644
--- a/src/box/sql/build.c
+++ b/src/b= ox/sql/build.c
@@ -2128,19 +2128,33 @@ sqliteViewResetAll(sqlite3= * db)
=C2=A0/**
=C2=A0 * Remove entries from the _sql_= stat1 and _sql_stat4
=C2=A0 * system spaces after a DROP INDEX or= DROP TABLE command.
- *
- * @param pParse Parsing cont= ext.
- * @param zType Type of entry to be deleted:
- * = 'idx' or 'tbl'= ; string literal.
- * @param zName Name of index or table.
<= div>+ *=C2=A0
+ * @param parse=C2=A0 =C2=A0 =C2=A0 The parsing co= ntext.
+ * @param table_name The table to be dropped or
+ *=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0th= e table that contains index to be dropped.
+ * @param idx_name=C2= =A0 =C2=A0Index to be dropped.
=C2=A0 */
=C2=A0static v= oid
-sql_clear_stat_spaces(Parse * pParse, const char *zType, con= st char *zName)
+sql_clear_stat_tables(Parse *parse, const char *= table_name,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 const char *idx_name)
=C2=A0{
= - sqlite3NestedParse(pParse, &q= uot;DELETE FROM \"_sql_stat1\" WHERE \"%s\"=3D%Q",=
- =C2=A0 =C2=A0zTy= pe, zName);
- sqlite3= NestedParse(pParse, "DELETE FROM \"_sql_stat4\" WHERE \"= ;%s\"=3D%Q",
- <= /span>=C2=A0 =C2=A0zType, zName);
+=C2=A0 =C2=A0 if (idx_name != =3D NULL) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 sqlite3NestedParse(parse= ,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 "DELETE FROM \"_sql_stat1\" WHERE (\"idx\"= =3D%Q AND "
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 "\"tbl\"=3D%Q)",
+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 idx_n= ame, table_name);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 sqlite3NestedParse= (parse,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 "DELETE FROM \"_sql_stat4\" WHERE (\"idx\= "=3D%Q AND "
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 "\"tbl\"=3D%Q)",
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 idx= _name, table_name);
+=C2=A0 =C2=A0 } else {
+=C2=A0 =C2= =A0 =C2=A0 =C2=A0 sqlite3NestedParse(parse,
+=C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "DELETE FROM \"= _sql_stat1\" WHERE \"tbl\"=3D%Q",
+=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 table_name);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 sqlite3NestedParse(parse,
+= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "= ;DELETE FROM \"_sql_stat4\" WHERE \"tbl\"=3D%Q",
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 table_name);
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
=C2= =A0}
=C2=A0
=C2=A0/**
@@ -2325,7 +2339,7 @@ s= ql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,=
=C2=A0 *=C2=A0 =C2= =A0 tuple with corresponding space_id from _space.
=C2=A0 */
=C2=A0
- sql_clear_stat_spaces(parse_context,= "tbl", space_name);
+ sql_clear_stat_tables(parse_context, space_name, NULL);
=C2=A0 struct Table *tab =3D= sqlite3HashFind(&db->pSchema->tblHash, space_name);
= =C2=A0 sqlite3FkDropTable(parse= _context, table_name_list, tab);
=C2=A0 sql_code_drop_table(parse_context, space, is_view);
@@ -3328,7 +3342,7 @@ sql_drop_index(struct Parse *parse_context, str= uct SrcList *index_name_list,
=C2=A0 * But firstly, delete statistics since schema
= =C2=A0 * changes after DDL.
=C2=A0 */
-<= span style=3D"white-space:pre-wrap"> sql_clear_stat_spaces(parse_con= text, "idx", index->def->name);
+ sql_clear_stat_tables(parse_context, table_nam= e, index->def->name);
=C2=A0 int record_reg =3D ++parse_context->nMem;
=C2=A0= int space_id_reg =3D ++parse_c= ontext->nMem;
=C2=A0 sqlite3VdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
=C2= =A0
--f4f5e80742b8fe3142056938b519--