[patches] Re: [PATCH v2 2/2] sql: use Tarantool routine to get collation

[v.shpilevoy at tarantool.org]

See below 3 comments.

> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 9f45c6224..ca4291bc3 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1165,6 +1167,73 @@ sqlite3AddCollateType(Parse * pParse, Token * pToken)
> 	}
> }
> 
> +/**
> + * Return name of given column collation from table.
> + *
> + * @param table Table which is used to fetch column.
> + * @param column Number of column.
> + * @retval Pointer to collation's name.
> + */
> +char *

1. How about to return const char * instead of char *? It would be nice if compiler checks for us that this name is not modified or destroyed by free().

> +column_collation_name(Table *table, uint32_t column)
> +{
> +	assert(table != NULL);
> 
> +
> +/**
> + * Return name of given column collation from index.
> + *
> + * @param idx Index which is used to fetch column.
> + * @param column Number of column.
> + * @retval Pointer to collation's name.
> + */
> +char *

2. Same as 1.

> +index_collation_name(Index *idx, uint32_t column)
> +{
> +	assert(idx != NULL);
> 
> diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
> index 5f2ddeaee..270265725 100644
> --- a/src/box/sql/callback.c
> +++ b/src/box/sql/callback.c
> @@ -60,11 +60,12 @@ sqlite3GetCollSeq(Parse * pParse,	/* Parsing context */
> 		  const char *zName	/* Collating sequence name */
>     )
> {
> +	(void)db;

3. Why do not remove this argument?