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

v.shpilevoy at tarantool.org v.shpilevoy at tarantool.org
Tue Mar 6 15:36:52 MSK 2018


See 5 comments below.

> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 9f45c6224..498abd7a1 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1165,6 +1167,61 @@ sqlite3AddCollateType(Parse * pParse, Token * pToken)
> 	}
> }
> 
> +/* Return name of collation for given column. */

1. Please, put function comments into /** instead of /*. The second type we use inside functions.

> +char *
> +column_collation_name(Table *table, uint32_t column)
> +{
> +	assert(table != NULL);
> +	uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum);
> +	struct space *space = space_by_id(space_id);
> +	/**

2. Same as one, but vice versa. Fix this in other places too, please.

> diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c
> index 5f2ddeaee..8cc224253 100644
> --- a/src/box/sql/callback.c
> +++ b/src/box/sql/callback.c
> @@ -141,13 +142,15 @@ static struct coll_plus_name_struct binary_coll_with_name =
> 		"BINARY"};
> static struct coll * binary_coll = (struct coll*)&binary_coll_with_name;
> 
> +struct coll *
> +sql_default_coll()
> +{
> +	return binary_coll;
> +}
> +
> /*
>  * Parameter zName points to a UTF-8 encoded string nName bytes long.
> - * Return the CollSeq* pointer for the collation sequence named zName
> - * for the encoding 'enc' from the database 'db'.
> - *
> - * If the entry specified is not found and 'create' is true, then create a
> - * new entry.  Otherwise return NULL.
> + * Return the CollSeq* pointer for the collation sequence named zName.

3. Now it is not CollSeq*, it is coll *. And I think, it is better to rewrite this comment in Tarantool code style (with @param, @retval).

> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index b69a176cb..7ca6bef34 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -195,9 +195,9 @@ sqlite3ExprCollSeq(Parse * pParse, Expr * pExpr)
> 			 */
> 			int j = p->iColumn;
> 			if (j >= 0) {
> -				const char *zColl = p->pTab->aCol[j].zColl;
> +				const char *zColl = column_collation_name(p->pTab, j);
> 				pColl =
> -				    sqlite3FindCollSeq(db, zColl, 0);
> +				    sqlite3FindCollSeq(zColl);

4. Seems, thar this string now can fit onto the previous line.

> diff --git a/src/box/sql/main.c b/src/box/sql/main.c
> index 9cda5f3e9..d58b3e8b6 100644
> --- a/src/box/sql/main.c
> +++ b/src/box/sql/main.c
> @@ -2417,9 +2417,6 @@ openDatabase(const char *zFilename,	/* Database filename UTF-8 encoded */
> 	/* EVIDENCE-OF: R-08308-17224 The default collating function for all
> 	 * strings is BINARY.
> 	 */

5. Seems that this comment is not actual now.
> 




More information about the Tarantool-patches mailing list