[tarantool-patches] Re: [PATCH v7 3/7] sql: introduce expr_len for sql_expr_compile

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch! See 2 comments below.

On 23/05/2018 17:05, Kirill Shcherbatov wrote:
> As we need to use parser in the cases with not null-terminated
> strings expr_len argument for sql_expr_compile is required.
> 
> Part of #3272.
> ---
>   src/box/alter.cc       | 1 +
>   src/box/sql.h          | 4 +++-
>   src/box/sql/tokenize.c | 7 ++++---
>   3 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/src/box/alter.cc b/src/box/alter.cc
> index e87bbce..e4780da 100644
> --- a/src/box/alter.cc
> +++ b/src/box/alter.cc
> @@ -408,6 +408,7 @@ field_def_decode(struct field_def *field, const char **data,
>   
>   	if (field->default_value != NULL &&
>   	    sql_expr_compile(sql_get(), field->default_value,
> +			     strlen(field->default_value),
>   			     &field->default_value_expr) != 0)
>   		diag_raise();
>   }
> diff --git a/src/box/sql.h b/src/box/sql.h
> index ff8ab6f..a7f2500 100644
> --- a/src/box/sql.h
> +++ b/src/box/sql.h
> @@ -74,12 +74,14 @@ struct Table;
>    * stuct Select and return it.
>    * @param db SQL context handle.
>    * @param expr Expression to parse.
> + * @param expr_len Expression to parse length.

1. Looks like expr_len is the expression that is intended to
parse a length.
Do you mean this?: "Length of @an expr."

>    * @param[out] result Result: AST structure.
>    *
>    * @retval Error code if any.
>    */
>   int
> -sql_expr_compile(struct sqlite3 *db, const char *expr, struct Expr **result);
> +sql_expr_compile(struct sqlite3 *db, const char *expr, int expr_len,
> +	struct Expr **result);

2. Bad indentation.