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

["i.koptelov" <ivan.koptelov@tarantool.org>]

> 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?

>        /**
>         * Number of FK constraints declared within
>         * CREATE TABLE statement.