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 E59262DB8B for ; Sat, 14 Apr 2018 00:29:47 -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 DlkR7BCEmUoE for ; Sat, 14 Apr 2018 00:29:47 -0400 (EDT) Received: from mail-lf0-f48.google.com (mail-lf0-f48.google.com [209.85.215.48]) (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 5EFA42DB83 for ; Sat, 14 Apr 2018 00:29:47 -0400 (EDT) Received: by mail-lf0-f48.google.com with SMTP id j68-v6so15168867lfg.13 for ; Fri, 13 Apr 2018 21:29:46 -0700 (PDT) MIME-Version: 1.0 References: <1522791436-8221-1-git-send-email-hollow653@gmail.com> <926E041C-FCCB-46B0-B49B-21292CB70813@tarantool.org> <4E93854E-25F3-45F7-8590-5CD96991E365@tarantool.org> <51BFB6B0-C9CE-4380-8758-7F991512A752@tarantool.org> In-Reply-To: <51BFB6B0-C9CE-4380-8758-7F991512A752@tarantool.org> From: Hollow111 Date: Sat, 14 Apr 2018 04:29:34 +0000 Message-ID: Subject: [tarantool-patches] Re: [PATCH 2/2] sql: statistics removal after dropping an index Content-Type: multipart/alternative; boundary="00000000000036161a0569c77061" 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 Cc: tarantool-patches@freelists.org --00000000000036161a0569c77061 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hello. I'd like to ask about 'one kind of corrections". =D0=BF=D1=82, 13 =D0=B0=D0=BF=D1=80. 2018 =D0=B3. =D0=B2 12:50, n.pettik : > Hello. I still notice several codestyle violations. > This patch can=E2=80=99t be pushed until all of them are fixed. > Please, consider them. > > > On 12 Apr 2018, at 09:06, Hollow111 wrote: > > > > Hello. Here's the diff for the changes: > > > > diff --git a/src/box/sql/build.c b/src/box/sql/build.c > > index 92f3cb6..0287183 100644 > > --- a/src/box/sql/build.c > > +++ b/src/box/sql/build.c > > @@ -2129,18 +2129,32 @@ 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_spaces(Parse *parse, const char *table_name, > > + const char *idx_name) > > Arguments which you carry to the next line, should start right below > previous: > > void f(x int, =E2=80=A6. > y char, =E2=80=A6. > z double =E2=80=A6); > I couldnt find this in "C style guide" ( https://tarantool.io/en/doc/2.0/dev_guide/c_style_guide.html) Moreover according to this guide: "Outside of comments, documentation and except in Kconfig, spaces are never used for indentation...". Which means we're supposed to violate this rule in case first argument is positioned not on the same range from left corner of screen as any amount of 8-character tabs. Maybe I'm wrong at smth but I'd like to have an explanation. > > { > > - 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, > > Wrong indentation. > > > + "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 =E2=80=9C > > Arguments which you carry to the next line, should start right below > previous. > > > + "\"tbl\"=3D%Q)", > > + idx_name, table_name); > > + } else { > > + sqlite3NestedParse(parse, > > + "DELETE FROM \"_sql_stat1\" WHERE > \"tbl\"=3D%Q=E2=80=9D, > > Arguments which you carry to the next line, should start right below > previous. > > > + table_name); > > + sqlite3NestedParse(parse, > > + "DELETE FROM \"_sql_stat4\" WHERE \"tbl\"=3D%Q", > > + table_name); > > + } > > Wrong indentation. > > > } > > > > /** > > @@ -2325,7 +2339,7 @@ sql_drop_table(struct Parse *parse_context, struc= t > SrcList *table_name_list, > > * tuple with corresponding space_id from _space. > > */ > > > > - sql_clear_stat_spaces(parse_context, "tbl", space_name); > > + sql_clear_stat_spaces(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, struc= t > 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_spaces(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); > > --00000000000036161a0569c77061 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello. I'd like to ask about 'one kind of correcti= ons".

=D0=BF=D1=82= , 13 =D0=B0=D0=BF=D1=80. 2018 =D0=B3. =D0=B2 12:50, n.pettik <korablev@tarantool.org>:
Hello. I = still notice several codestyle violations.
This patch can=E2=80=99t be pushed until all of them are fixed.
Please, consider them.

> On 12 Apr 2018, at 09:06, Hollow111 <hollow653@gmail.com> wrote:
>
> Hello. Here's the diff for the changes:
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 92f3cb6..0287183 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2129,18 +2129,32 @@ sqliteViewResetAll(sqlite3 * db)
>=C2=A0 =C2=A0* Remove entries from the _sql_stat1 and _sql_stat4
>=C2=A0 =C2=A0* system spaces after a DROP INDEX or DROP TABLE command.<= br> >=C2=A0 =C2=A0*
> - * @param pParse Parsing context.
> - * @param zType Type of entry to be deleted:
> - *=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0'idx' or 'tbl&= #39; string literal.
> - * @param zName Name of index or table.
> + * @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_spaces(Parse * pParse, const char *zType, const char *= zName)
> +sql_clear_stat_spaces(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 =C2=A0 =C2=A0 =C2=A0 =C2=A0const char *idx_name)

Arguments which you carry to the next line, should start right below previo= us:

void f(x int, =E2=80=A6.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 y char, =E2=80=A6.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 z double =E2=80=A6);

I couldnt find this in=C2=A0=C2=A0"C style guide" (https://tarantool.io/en/doc/2.0/dev= _guide/c_style_guide.html)
Moreover according to this guide:
"Outside of com= ments, documentation and except in Kconfig, spaces are never used for inden= tation...".
Which means we're supposed to violate this r= ule
in case first argument is positioned not on the same range=C2= =A0
from left corner of screen as any amount of 8-character tabs.= =C2=A0
Maybe I'm wrong at smth but I'd like to have an ex= planation.


>=C2=A0 {
> -=C2=A0 =C2=A0 =C2=A0sqlite3NestedParse(pParse, "DELETE FROM \&qu= ot;_sql_stat1\" WHERE \"%s\"=3D%Q",
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 zType, zName);
> -=C2=A0 =C2=A0 =C2=A0sqlite3NestedParse(pParse, "DELETE FROM \&qu= ot;_sql_stat4\" WHERE \"%s\"=3D%Q",
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 zType, zName);
> +=C2=A0 =C2=A0 =C2=A0if (idx_name !=3D NULL) {
> +=C2=A0 =C2=A0 =C2=A0sqlite3NestedParse(parse,

Wrong indentation.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =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 =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= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0idx_name, ta= ble_name);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3NestedParse(pa= rse,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =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_stat4\" WHERE (\"idx\"=3D%Q AND =E2=80=9C<= br>
Arguments which you carry to the next line, should start right below previo= us.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =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)",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0idx_name, ta= ble_name);
> +=C2=A0 =C2=A0 =C2=A0} else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3NestedParse(pa= rse,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =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 \"tbl\"=3D%Q=E2=80=9D,

Arguments which you carry to the next line, should start right below previo= us.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0table_name);=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3NestedParse(pa= rse,
> +=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&qu= ot;,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0table_name);=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}

Wrong indentation.

>=C2=A0 }
>=C2=A0
>=C2=A0 /**
> @@ -2325,7 +2339,7 @@ sql_drop_table(struct Parse *parse_context, stru= ct SrcList *table_name_list,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 *=C2=A0 =C2=A0 tuple with corresponding spa= ce_id from _space.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 */
>=C2=A0
> -=C2=A0 =C2=A0 =C2=A0sql_clear_stat_spaces(parse_context, "tbl&qu= ot;, space_name);
> +=C2=A0 =C2=A0 =C2=A0sql_clear_stat_spaces(parse_context, space_name, = NULL);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct Table *tab =3D sqlite3HashFind(&d= b->pSchema->tblHash, space_name);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3FkDropTable(parse_context, table_name= _list, tab);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0sql_code_drop_table(parse_context, space, is= _view);
> @@ -3328,7 +3342,7 @@ sql_drop_index(struct Parse *parse_context, stru= ct SrcList *index_name_list,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 * But firstly, delete statistics since sche= ma
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 * changes after DDL.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 */
> -=C2=A0 =C2=A0 =C2=A0sql_clear_stat_spaces(parse_context, "idx&qu= ot;, index->def->name);
> +=C2=A0 =C2=A0 =C2=A0sql_clear_stat_spaces(parse_context, table_name, = index->def->name);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int record_reg =3D ++parse_context->nMem;=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int space_id_reg =3D ++parse_context->nMe= m;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0sqlite3VdbeAddOp2(v, OP_Integer, space_id, s= pace_id_reg);

--00000000000036161a0569c77061--