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

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

Hi! Thanks for the fixes!

>>> +		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 ==?
>>
> I did this because function sqlVdbeCompareMsgpack() compared packed value as
> left operand and MEM as right operand. In mem_cmp_msgpack() order was reversed,
> now MEM is left operand and packed value is right operand.

Shouldn't you then change the other place too?

> @@ -2721,13 +2586,15 @@ 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;
> +		if (mem_cmp_msgpack(mem, (const char **)&key1, &rc, coll) != 0)
> +			rc = 0;
>  		if (rc != 0) {
> -			if (key2->key_def->parts[i].sort_order !=
> -			    SORT_ORDER_ASC) {
> -				rc = -rc;
> -			}
> -			return rc;
> +			if (part->sort_order != SORT_ORDER_ASC)
> +				return rc;
> +			return -rc;
>  		}

Here it was cmp(key1, key), now it is cmp(key2, key1). Shouldn't
then the sort_order check become `== SORT_ORDER_ASC`?