[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