Re: [Tarantool-patches] [PATCH v2 4/4] sql: introduce mem_cmp_msgpack()

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the fixes!

> diff --git a/src/box/sql.c b/src/box/sql.c
> index 790ca7f70..c1271ee0a 100644
> --- a/src/box/sql.c
> +++ b/src/box/sql.c
> @@ -765,10 +765,14 @@ tarantoolsqlIdxKeyCompare(struct BtCursor *cursor,
>  			}
>  		}
>  		next_fieldno = fieldno + 1;
> -		rc = sqlVdbeCompareMsgpack(&p, unpacked, i);
> +		struct key_part *part = &unpacked->key_def->parts[i];
> +		struct Mem *mem = unpacked->aMem + i;
> +		struct coll *coll = part->coll;
> +		/* In case of fail make rc equal to 0. */

The comment isn't really necessary I would say. Does not
help much. The same in the other place below.

> +		if (mem_cmp_msgpack(mem, &p, &rc, coll) != 0)
> +			rc = 0;
>  		if (rc != 0) {
> -			if (unpacked->key_def->parts[i].sort_order !=
> -			    SORT_ORDER_ASC)
> +			if (part->sort_order == SORT_ORDER_ASC)

Why did you change != to ==?

> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
> index da27cd191..8e176e418 100644
> --- a/src/box/sql/mem.c
> +++ b/src/box/sql/mem.c
> @@ -2721,13 +2586,16 @@ sqlVdbeRecordCompareMsgpack(const void *key1,
>  	n = MIN(n, key2->nField);
>  
>  	for (i = 0; i != n; i++) {
> -		rc = sqlVdbeCompareMsgpack((const char**)&key1, key2, i);
> +		struct key_part *part = &key2->key_def->parts[i];
> +		struct Mem *mem = key2->aMem + i;
> +		struct coll *coll = part->coll;
> +		/* In case of fail make rc equal to 0. */
> +		if (mem_cmp_msgpack(mem, (const char **)&key1, &rc, coll) != 0)
> +			rc = 0;