Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Imeev Mergen <imeevma@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH v2 1/1] sql: hold in stat tables space/index id instead of name
Date: Thu, 11 Oct 2018 14:41:52 +0300	[thread overview]
Message-ID: <D391288E-A449-4A64-9978-06274A9570E5@tarantool.org> (raw)
In-Reply-To: <ad8339a2-0fec-1014-b891-7434f0e2f593@tarantool.org>

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


>> Please, next time attach intermediate diff (i.e. diff between two versions) or
>> inline each hunk as answer to comment. It is quite complicated to review
>> full patch (especially when it comes to patches containing hundreds of lines)
>> each time.
>> 
>> https://travis-ci.org/tarantool/tarantool/jobs/436232731 <https://travis-ci.org/tarantool/tarantool/jobs/436232731>
>> 
>> box/sql.test.lua fails on Travis. Please, fix it.
>> 
> Now it is good on travis:
> https://travis-ci.org/tarantool/tarantool/builds/439704198 <https://travis-ci.org/tarantool/tarantool/builds/439704198>
> 
> Diff between two last versions:
> 
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 95c516a..a886d8a 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -797,13 +797,13 @@ vdbe_emit_analyze_space(struct Parse *parse, struct space *space)
>      assert(space->index_count != 0);
>      struct Vdbe *v = sqlite3GetVdbe(parse);
>      assert(v != NULL);
> -    const char *tab_name = space_name(space);
> +    MAYBE_UNUSED const char *tab_name = space_name(space);
>      sqlite3VdbeAddOp4(v, OP_IteratorOpen, tab_cursor, 0, 0, (void *) space,
>                P4_SPACEPTR);
>      sqlite3VdbeAddOp2(v, OP_Integer, space->def->id, space_id_reg);
>      for (uint32_t j = 0; j < space->index_count; ++j) {
>          struct index *idx = space->index[j];
> -        const char *idx_name;
> +        MAYBE_UNUSED const char *idx_name;
>          /*
>           * Primary indexes feature automatically generated
>           * names. Thus, for the sake of clarity, use

Actually, this diff doesn’t look like fix of that failed test.
I guess it is simply flaky, so this time you get lucky and it is passed.
Did you checked that test-trace from Travis fails on fresh 2.0 as well?
Did you manage to understand the reason of failure? Otherwise there is
no guarantee that you patch is innocent in this situation.
Without any investigation I can give my approval on this patch.


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

  reply	other threads:[~2018-10-11 11:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-31 10:39 [tarantool-patches] " imeevma
2018-09-05 23:13 ` [tarantool-patches] " n.pettik
2018-09-07 16:49   ` Imeev Mergen
2018-09-30 23:40     ` n.pettik
2018-10-01 11:20       ` n.pettik
2018-10-02 16:42       ` [tarantool-patches] Re[2]: [tarantool-patches] " Мерген Имеев
2018-10-09 12:36         ` [tarantool-patches] " n.pettik
2018-10-10 17:17           ` Imeev Mergen
2018-10-11 11:41             ` n.pettik [this message]
2018-10-11 14:56               ` Imeev Mergen
2018-10-11 15:00                 ` n.pettik
2018-11-17 14:09                   ` Imeev Mergen
2018-11-19 10:27                     ` Vladislav Shpilevoy
2018-11-19 10:38                       ` Vladislav Shpilevoy
2018-11-21 16:00                       ` Konstantin Osipov
2018-11-21 16:11                         ` Vladislav Shpilevoy
2018-12-08 12:39                       ` Imeev Mergen

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=D391288E-A449-4A64-9978-06274A9570E5@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='[tarantool-patches] Re: [PATCH v2 1/1] sql: hold in stat tables space/index id instead of name' \
    /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