From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 272FE2CFA3 for ; Thu, 11 Oct 2018 07:41:57 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id m4osZLuiiINK for ; Thu, 11 Oct 2018 07:41:57 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id D6EFB2CE5E for ; Thu, 11 Oct 2018 07:41:56 -0400 (EDT) From: "n.pettik" Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_B66D7064-E43D-4547-91FB-B967D14137F4" Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) 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 In-Reply-To: References: <1f1960f842e9443511b4bf2712a0b79bd7fb0764.1535711802.git.imeevma@gmail.com> <0bbf0c18-9d9b-4849-5a01-24729ab08468@tarantool.org> <70A5C3C7-8069-4FF0-A653-46B3B82C9716@tarantool.org> <1538498570.8155792@f528.i.mail.ru> <16CEE3B0-B6FC-4BE0-B381-704AF1794585@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Imeev Mergen --Apple-Mail=_B66D7064-E43D-4547-91FB-B967D14137F4 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 >> 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. >>=20 >> https://travis-ci.org/tarantool/tarantool/jobs/436232731 = >>=20 >> box/sql.test.lua fails on Travis. Please, fix it. >>=20 > Now it is good on travis: > https://travis-ci.org/tarantool/tarantool/builds/439704198 = >=20 > Diff between two last versions: >=20 > 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 !=3D 0); > struct Vdbe *v =3D sqlite3GetVdbe(parse); > assert(v !=3D NULL); > - const char *tab_name =3D space_name(space); > + MAYBE_UNUSED const char *tab_name =3D 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 =3D 0; j < space->index_count; ++j) { > struct index *idx =3D 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=E2=80=99t 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. --Apple-Mail=_B66D7064-E43D-4547-91FB-B967D14137F4 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 
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 !=3D 0);
     struct Vdbe *v =3D = sqlite3GetVdbe(parse);
     assert(v !=3D NULL);
-    const char *tab_name =3D space_name(space);
+    MAYBE_UNUSED const char *tab_name =3D = 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 =3D 0; j < = space->index_count; ++j) {
         struct index *idx =3D = 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=E2=80=99t 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.

= --Apple-Mail=_B66D7064-E43D-4547-91FB-B967D14137F4--