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 E9F7D267C5 for ; Tue, 12 Feb 2019 09:56:45 -0500 (EST) 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 psONzLsmf7Lm for ; Tue, 12 Feb 2019 09:56:45 -0500 (EST) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 2E41E264FC for ; Tue, 12 Feb 2019 09:56:44 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: [tarantool-patches] Re: [PATCH] sql: remove struct Table From: "n.pettik" In-Reply-To: Date: Tue, 12 Feb 2019 17:56:41 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <4c2b87b5-9e83-f96f-4350-edf7a12e5593@tarantool.org> <9f612f00-6222-e3cb-642c-04bfec116fd4@tarantool.org> <3438F900-EF6D-4475-B3F1-BD4971F78186@tarantool.org> <7CB3BC6E-B4CF-4EE0-BFB7-F5E7CFF89947@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: Ivan Koptelov > On 12 Feb 2019, at 16:55, i.koptelov = wrote: >> On 11 Feb 2019, at 20:58, n.pettik wrote: >>> On 6 Feb 2019, at 20:17, i.koptelov = wrote: >>>=20 >>>>> On 1 Feb 2019, at 14:05, Ivan = Koptelov wrote: >>>>>=20 >>>>> Thank you for the review! Sorry for all these code style errors. >>>>> I consider review changes minor, so I put diff b/w first and >>>>> second version of patch at the end of the letter. >>>>> (second commit on the branch 'review fixes' would be squashed) >>>> Don=E2=80=99t do it next time, pls. Instead, inline fixes as an = answers to comments, >>>> especially when it comes for huge diff ( 209 insertions(+), 259 = deletions(-)) >>>> Otherwise, it takes a while to track fixed chunks of code in a = whole diff. >>> Sorry for this. All review fixes below is inlined. >>=20 >> Well, thank you. But don=E2=80=99t put fixes in a separate commit, at = least when >> you push your branch to the remote repository. You may *accidentally* >> fail during final commit squashing and no-one will notice that. >>=20 >> Travis status was entirely negative: It can=E2=80=99t be compiled.=20 >> You forgot to add forward declaration of space_def. >> I fixed that and a couple of minor issues more. Here is a diff >> (I=E2=80=99ve squashed your commits and pushed on top of your = branch). >>=20 >> Please, check it out. If it is OK, squash them and then patch LGTM. > Thank you for the fixes! Ok, I would not put my fixes in separate = commit in future. > I checked out the diff, it=E2=80=99s ok for me except one small issue. = I=E2=80=99ve marked > it below. >>=20 >> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h >> index 874fd682f..1352f6efe 100644 >> --- a/src/box/sql/sqliteInt.h >> +++ b/src/box/sql/sqliteInt.h >> @@ -2759,6 +2760,9 @@ struct Parse { >> TriggerPrg *pTriggerPrg; /* Linked list of coded = triggers */ >> With *pWith; /* Current WITH clause, or NULL */ >> With *pWithToFree; /* Free this WITH object at the end of = the parse */ >> + /** Space triggers are being coded for. */ >> + struct space *triggered_space; >> + /** A space being constructed by CREATE TABLE */ > Are you sure that this comment (last line) should be here? Is it about = all lines below? I forgot to move struct space *new_space; below. Thanks for cross-check. Could you please fix this?