[tarantool-patches] Re: [PATCH 2/2] sql: compute resulting collation for concatenation

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the fixes!

>>> a) If some data type  has an explicit collation EC1, then every data
>>
>> 2. Double space after 'type'. In some other places below too.
> 
> Ok, fixed. As a rule I use vim auto-formatting utility, which aligns
> borders to 72 chars automatically putting extra spaces between
> sentences. I think it is kind of normal.

I tried to do it too back in the days, but Kostja said
we must not justify text.

>>> +			bool is_rhs_forced;
>>> +			uint32_t rhs_coll_id;
>>> +			if (sql_expr_coll(parse, p->pRight, &is_rhs_forced,
>>> +					  &rhs_coll_id) != 0)
>>> +				return -1;
>>> +			if (is_lhs_forced && is_rhs_forced) {
>>> +				if (lhs_coll_id != rhs_coll_id)
>>> +					return -1;
>>
>> 5. Did you miss diag_set?
> 
> No, I did it on purpose. Firstly, this function is recursive,
> so in case error occurred on bottom levels of recursion,
> diag would be reseted on each level above (and parser’s
> counter of errors would be incremented several times as
> well). No terrible consequences would take place in this case,
> but it looks like a bad pattern. Anyway, fixed it according to
> your suggestion.

Each ClientError is accounted in a global errors counter. It
means, that the same error should be set multiple times, otherwise
the statistics would be wrong. Instead of resetting diag each time
check if parser->nErr > 0, and if it is, then do nothing.
diag_is_empty() can not be used, because it happens sometimes, that
there are no errors, but diag is not cleared from a previous error.