[Tarantool-patches] [PATCH v5 18/52] sql: introduce mem_concat()

[Vladislav Shpilevoy]

Thanks for the discussion!

>>> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
>>> index b417c1007..2d76ef88d 100644
>>> --- a/src/box/sql/mem.c
>>> +++ b/src/box/sql/mem.c
>>> @@ -326,6 +326,70 @@ mem_move(struct Mem *to, struct Mem *from)
>>>  	return 0;
>>>  }
>>>  
>>> +static bool
>>> +is_result_null(const struct Mem *a, const struct Mem *b, struct Mem *result,
>>> +	       enum field_type type)
>>
>> 1. Functions called 'is_*' never should change anything.
>>
> Fixed. Renamed to check_result_null().
> 
>> Another question is why do you even need it? It is used in a single place,
>> where it could be just inlined. And is not used in a place, where it could
>> be applied.
>>
> I added it here since it was the first commit, which used it. This functions
> will be used in all arithmetic and bitwise operations with two operands.

I hope it is not too late to rename it to try_return_null()? Because
sorry, but 'check' also has a meaning - it is just like 'is' (does not
change anything), but sets a diag. At least this is what I see in the
code usually, and try to force in the new patches.