<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hello! Thank you for review! My answer below.<br>
</p>
<br>
<div class="moz-cite-prefix">On 10/11/2018 02:41 PM, n.pettik wrote:<br>
</div>
<blockquote type="cite"
cite="mid:D391288E-A449-4A64-9978-06274A9570E5@tarantool.org">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<br class="">
<div>
<blockquote type="cite" class="">
<div class="">
<div text="#000000" bgcolor="#FFFFFF" class="">
<blockquote type="cite"
cite="mid:16CEE3B0-B6FC-4BE0-B381-704AF1794585@tarantool.org"
class="">
<pre class="" wrap="">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.
<a class="moz-txt-link-freetext" href="https://travis-ci.org/tarantool/tarantool/jobs/436232731" moz-do-not-send="true">https://travis-ci.org/tarantool/tarantool/jobs/436232731</a>
box/sql.test.lua fails on Travis. Please, fix it.
</pre>
</blockquote>
Now it is good on travis:<br class="">
<a class="moz-txt-link-freetext"
href="https://travis-ci.org/tarantool/tarantool/builds/439704198"
moz-do-not-send="true">https://travis-ci.org/tarantool/tarantool/builds/439704198</a><b
class=""><br class="">
<br class="">
Diff between two last versions:</b><br class="">
<br class="">
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c<br
class="">
index 95c516a..a886d8a 100644<br class="">
--- a/src/box/sql/analyze.c<br class="">
+++ b/src/box/sql/analyze.c<br class="">
@@ -797,13 +797,13 @@ vdbe_emit_analyze_space(struct Parse
*parse, struct space *space)<br class="">
assert(space->index_count != 0);<br class="">
struct Vdbe *v = sqlite3GetVdbe(parse);<br class="">
assert(v != NULL);<br class="">
- const char *tab_name = space_name(space);<br class="">
+ MAYBE_UNUSED const char *tab_name =
space_name(space);<br class="">
sqlite3VdbeAddOp4(v, OP_IteratorOpen, tab_cursor, 0,
0, (void *) space,<br class="">
P4_SPACEPTR);<br class="">
sqlite3VdbeAddOp2(v, OP_Integer,
space->def->id, space_id_reg);<br class="">
for (uint32_t j = 0; j < space->index_count;
++j) {<br class="">
struct index *idx = space->index[j];<br
class="">
- const char *idx_name;<br class="">
+ MAYBE_UNUSED const char *idx_name;<br class="">
/*<br class="">
* Primary indexes feature automatically
generated<br class="">
* names. Thus, for the sake of clarity, use<br
class="">
</div>
</div>
</blockquote>
<br class="">
</div>
<div>Actually, this diff doesn’t look like fix of that failed
test.</div>
<div>I guess it is simply flaky, so this time you get lucky and it
is passed.</div>
<div>Did you checked that test-trace from Travis fails on fresh
2.0 as well?</div>
<div>Did you manage to understand the reason of failure? Otherwise
there is</div>
<div>no guarantee that you patch is innocent in this situation.</div>
<div>Without any investigation I can give my approval on this
patch.</div>
<div><br class="">
</div>
</blockquote>
I was able to reproduce this failure on current 2.0:<br>
<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/3737">https://github.com/tarantool/tarantool/issues/3737</a><br>
<br>
I think my patch do not affect this test in the way it showed in<br>
error.<br>
<br>
</body>
</html>