[tarantool-patches] Re: [PATCH v2 1/1] sql: hold in stat tables space/index id instead of name
Imeev Mergen
imeevma at tarantool.org
Thu Oct 11 17:56:17 MSK 2018
Hello! Thank you for review! My answer below.
On 10/11/2018 02:41 PM, n.pettik wrote:
>
>>> 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
>>>
>>> box/sql.test.lua fails on Travis. Please, fix it.
>>>
>> Now it is good on travis:
>> 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.
>
I was able to reproduce this failure on current 2.0:
https://github.com/tarantool/tarantool/issues/3737
I think my patch do not affect this test in the way it showed in
error.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20181011/34a2ece0/attachment.html>
More information about the Tarantool-patches
mailing list