[tarantool-patches] Re: [PATCH v2 3/7] sql: refactor sql_src_list_append to set diag
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Mar 18 22:33:59 MSK 2019
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 *);
More information about the Tarantool-patches
mailing list