[tarantool-patches] Re: [PATCH v2 3/7] sql: refactor sql_src_list_append to set diag

[Vladislav Shpilevoy]

Hi! Thanks for the fixes! See 2 comments below.

> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index f53b2561c..04f65f165 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -2821,6 +2785,7 @@ sqlSrcListAppendFromTerm(Parse * pParse,	/* Parsing context */
>   	sqlIdListDelete(db, pUsing);
>   	sql_select_delete(db, pSubquery);
>   	return 0;
> +
>   }

1. Stray empty line. Please, do self-review before sending a patch.
Such a stray change says that you did not scan the patch before a
send.

> diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
> index 0895f7de1..2df1830d6 100644
> --- a/src/box/sql/sqlInt.h
> +++ b/src/box/sql/sqlInt.h
> @@ -3428,7 +3428,27 @@ sql_src_list_enlarge(struct sql *db, struct SrcList *src_list, int new_slots,
>   struct SrcList *
>   sql_src_list_new(struct sql *db);
>   
> -SrcList *sqlSrcListAppend(sql *, SrcList *, Token *);
> +/**
> + * Append a new table name to the given list.  Create a new
> + * SrcList if need be. A new entry is created in the list even
> + * if name_token is NULL.
> + *
> + * A SrcList is returned, or NULL if there is an OOM error.

2. This should be said in @retval sections.

> + * The returned SrcList might be the same as the list that was
> + * input or it might be a new one. If an OOM error does occurs,
> + * then the prior value of list that is input to this routine is
> + * automatically freed.
> + *
> + * @param db The database connection.
> + * @param list Append to this SrcList. NULL creates a new SrcList.
> + * @param name_token Token representing table name.
> + * @retval Not NULL SrcList pointer on success.
> + * @retval NULL Otherwise. The diag message is set.
> + */
> +struct SrcList *
> +sql_src_list_append(struct sql *db, struct SrcList *list,
> +		    struct Token *name_token);
> +
>   SrcList *sqlSrcListAppendFromTerm(Parse *, SrcList *, Token *,
>   				      Token *, Select *, Expr *, IdList *);
>   void sqlSrcListIndexedBy(Parse *, SrcList *, Token *);