[tarantool-patches] Re: [PATCH 4/8] sql: refactor sql_prepare() and sqlPrepare()

[Vladislav Shpilevoy]

Thanks for the patch!

On 27/08/2019 15:34, Nikita Pettik wrote:
> - Removed saveSqlFlag as argument from sqlPrepare(). It was used to
>   indicate that its caller is sql_prepare_v2() not sql_prepare().
>   Since in previous commit we've left only one version of this function
>   let's remove this flag at all.
> 
> - Removed struct db from list of sql_prepare() arguments. There's one
>   global database handler and it can be obtained by sql_get() call.
>   Hence, it makes no sense to pass around this argument.
> 
> Needed for #3292

Same as the previous patch - I think, it can be pushed out of
order.

See 2 comments below.

> ---
>  src/box/execute.c     |  3 +--
>  src/box/sql/analyze.c | 15 +++++++--------
>  src/box/sql/legacy.c  |  2 +-
>  src/box/sql/prepare.c | 10 ++++------
>  src/box/sql/sqlInt.h  |  3 +--
>  src/box/sql/vdbe.h    |  2 +-
>  src/box/sql/vdbeInt.h |  1 -
>  src/box/sql/vdbeapi.c |  7 +++----
>  src/box/sql/vdbeaux.c |  5 +----
>  9 files changed, 19 insertions(+), 29 deletions(-)
> 
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index bd52d12df..fa3d364e9 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -1344,7 +1344,7 @@ sample_compare(const void *a, const void *b, void *arg)
>   * @retval 0 on success, -1 otherwise.
>   */
>  static int
> -load_stat_from_space(struct sql *db, const char *sql_select_prepare,
> +load_stat_from_space(const char *sql_select_prepare,
>  		     const char *sql_select_load, struct index_stat *stats)

1. Please, drop the comment about 'db'.

>  {
>  	struct index **indexes = NULL;
> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
> index 0b54a9429..a25f063cc 100644
> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -845,8 +845,7 @@ vdbeUnbind(Vdbe * p, int i)
>  	 * as if there had been a schema change, on the first sql_step() call
>  	 * following any change to the bindings of that parameter.
>  	 */
> -	if (p->isPrepareV2 &&
> -	    ((i < 32 && p->expmask & ((u32) 1 << i))
> +	if (((i < 32 && p->expmask & ((u32) 1 << i))

2. Kostja is right, braces.