Tarantool development patches archive
 help / color / mirror / Atom feed
From: Hollow111 <hollow653@gmail.com>
To: korablev@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] Re: [PATCH 2/2] sql: statistics removal after dropping an index
Date: Sat, 14 Apr 2018 04:29:34 +0000	[thread overview]
Message-ID: <CAEi+_apaynDub6vGyVN=nkO=De6cRvFu-t6v72bCbUj0QcGGwQ@mail.gmail.com> (raw)
In-Reply-To: <51BFB6B0-C9CE-4380-8758-7F991512A752@tarantool.org>

[-- 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 --]

  reply	other threads:[~2018-04-14  4:29 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-03 21:37 [tarantool-patches] " N.Tatunov
2018-04-04 14:06 ` [tarantool-patches] " n.pettik
2018-04-04 15:46   ` Hollow111
2018-04-04 16:11     ` n.pettik
2018-04-04 16:34       ` Hollow111
2018-04-05 18:01         ` n.pettik
     [not found]           ` <CAEi+_aq5oyeB0cbnxAXXjQqu=h+PCGaaZuLkk3p33yq371+Xog@mail.gmail.com>
2018-04-07  2:12             ` Hollow111
2018-04-09 12:16               ` n.pettik
2018-04-12  6:06                 ` Hollow111
2018-04-13  8:50                   ` n.pettik
2018-04-14  4:29                     ` Hollow111 [this message]
2018-04-14  8:13                       ` n.pettik
2018-04-15  6:09                         ` Hollow111
2018-04-15  6:35                           ` Hollow111
2018-04-15 22:41                             ` n.pettik
2018-04-16 13:19                               ` Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEi+_apaynDub6vGyVN=nkO=De6cRvFu-t6v72bCbUj0QcGGwQ@mail.gmail.com' \
    --to=hollow653@gmail.com \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH 2/2] sql: statistics removal after dropping an index' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox