[tarantool-patches] Re: [PATCH] sql: remove struct Table

["n.pettik" <korablev@tarantool.org>]

> On 12 Feb 2019, at 16:55, i.koptelov <ivan.koptelov@tarantool.org> wrote:
>> On 11 Feb 2019, at 20:58, n.pettik <korablev@tarantool.org> wrote:
>>> On 6 Feb 2019, at 20:17, i.koptelov <ivan.koptelov@tarantool.org> wrote:
>>> 
>>>>> On 1 Feb 2019, at 14:05, Ivan Koptelov<ivan.koptelov@tarantool.org>  wrote:
>>>>> 
>>>>> 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’t 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.
>> 
>> Well, thank you. But don’t 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.
>> 
>> Travis status was entirely negative: It can’t be compiled. 
>> You forgot to add forward declaration of space_def.
>> I fixed that and a couple of minor issues more. Here is a diff
>> (I’ve squashed your commits and pushed on top of your branch).
>> 
>> 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’s ok for me except one small issue. I’ve marked
> it below.
>> 
>> 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?