[tarantool-patches] Re: [PATCH 2/2] sql: statistics removal after dropping an index

[Hollow111 <hollow653@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4674 bytes --]

Hello. I'd like to ask about 'one kind of corrections".

пт, 13 апр. 2018 г. в 12:50, n.pettik <korablev@tarantool.org>:

> Hello. I still notice several codestyle violations.
> This patch can’t 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)
> >   * 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, ….
>           y char, ….
>           z double …);
>

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\"=%Q",
> > -                        zType, zName);
> > -     sqlite3NestedParse(pParse, "DELETE FROM \"_sql_stat4\" WHERE
> \"%s\"=%Q",
> > -                        zType, zName);
> > +     if (idx_name != NULL) {
> > +     sqlite3NestedParse(parse,
>
> Wrong indentation.
>
> > +                                     "DELETE FROM \"_sql_stat1\" WHERE
> (\"idx\"=%Q AND "
> > +                                     "\"tbl\"=%Q)",
> > +                                     idx_name, table_name);
> > +             sqlite3NestedParse(parse,
> > +                                     "DELETE FROM \"_sql_stat4\" WHERE
> (\"idx\"=%Q AND “
>
> Arguments which you carry to the next line, should start right below
> previous.
>
> > +                                     "\"tbl\"=%Q)",
> > +                                     idx_name, table_name);
> > +     } else {
> > +             sqlite3NestedParse(parse,
> > +                                     "DELETE FROM \"_sql_stat1\" WHERE
> \"tbl\"=%Q”,
>
> Arguments which you carry to the next line, should start right below
> previous.
>
> > +                                     table_name);
> > +             sqlite3NestedParse(parse,
> > +                    "DELETE FROM \"_sql_stat4\" WHERE \"tbl\"=%Q",
> > +                                     table_name);
> > +                                     }
>
> Wrong indentation.
>
> >  }
> >
> >  /**
> > @@ -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_spaces(parse_context, space_name, NULL);
> >       struct Table *tab = 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_spaces(parse_context, table_name, index->def->name);
> >       int record_reg = ++parse_context->nMem;
> >       int space_id_reg = ++parse_context->nMem;
> >       sqlite3VdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
>
>

[-- Attachment #2: Type: text/html, Size: 6814 bytes --]