[tarantool-patches] Re: [PATCH 2/2] sql: don't change type of function's retval after codegen

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

On 21/03/2019 15:51, n.pettik wrote:
> 
>> On 11/03/2019 21:10, Nikita Pettik wrote:
>>> Proper type of function's returning value is set during names resolution
>>> (i.e. promotion from struct FuncDef to struct Expr, see
>>> resolveExprStep()). Accidentally, it was set again during byte-code
>>> generation for VDBE. What is more, in some cases it was set to a wrong
>>> type. For instance, built-in function randomblob() returns value to be
>>> encoded as MP_BIN, so its returning type is SCALAR. However, it was
>>> reset to INTEGER (as its first argument). This patch simply removes this
>>> second type promotion.
>>> ---
>>>   src/box/sql/expr.c       | 11 -----------
>>>   test/sql/iproto.result   | 41 +++++++++++++++++++++++++++++++++++++++++
>>>   test/sql/iproto.test.lua | 15 +++++++++++++++
>>>   3 files changed, 56 insertions(+), 11 deletions(-)
>>> diff --git a/test/sql/iproto.result b/test/sql/iproto.result
>>> index 3a77c8e93..26d55b60c 100644
>>> --- a/test/sql/iproto.result
>>> +++ b/test/sql/iproto.result
>>> +-- Type set during compilation stage, and since min/max are accept
>>> +-- arguments of all scalar type, we can't say nothing more than
>>> +-- SCALAR.
>>> +--
>>> +cn:execute("SELECT min(1, 2, 3);")
>>> +---
>>> +- metadata:
>>> +  - name: min(1, 2, 3)
>>> +    type: scalar
>>
>> This is the case, when a first argument type would be ok. As I
>> understand, it is illegal to calculate min of multiple different
>> types, so its rettype should be equal to its first argument. It
>> is not?
> 
> Unfortunately, it is legal:
> 
> tarantool> SELECT min('abc', 1);
> ---
> - - [1]
> …
> 
> It is to be resolved in scope of this issue:
> https://github.com/tarantool/tarantool/issues/4032
> 
> 

Thanks for the patchset and the explanations! As I see, Kostja
is ok with your type name changes. Technically the patch LGTM.