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

[Hollow111 <hollow653@gmail.com>]

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

пт, 6 апр. 2018 г. в 3:42, Hollow111 <hollow653@gmail.com>:

>
>
> чт, 5 апр. 2018 г. в 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 <hollow653@gmail.com> 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 != NULL) {
>> >          sqlite3NestedParse(parse,
>> >                      "DELETE FROM \"_sql_stat1\" WHERE (\"idx\"=%Q AND "
>> >                      "\"tbl\"=%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\"=%Q",
> -    zType, zName);
> - sqlite3NestedParse(pParse, "DELETE FROM \"_sql_stat4\" WHERE \"%s\"=%Q",
> -    zType, zName);
> +    if (idx_name != NULL) {
> +        sqlite3NestedParse(parse,
> +                    "DELETE FROM \"_sql_stat1\" WHERE (\"idx\"=%Q AND "
> +                    "\"tbl\"=%Q)",
> +                    idx_name, table_name);
> +        sqlite3NestedParse(parse,
> +                    "DELETE FROM \"_sql_stat4\" WHERE (\"idx\"=%Q AND "
> +                    "\"tbl\"=%Q)",
> +                    idx_name, table_name);
> +    } else {
> +        sqlite3NestedParse(parse,
> +                    "DELETE FROM \"_sql_stat1\" WHERE \"tbl\"=%Q",
> +                    table_name);
> +        sqlite3NestedParse(parse,
> +                    "DELETE FROM \"_sql_stat4\" WHERE \"tbl\"=%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 = 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 = ++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: 8033 bytes --]