Re: [Tarantool-patches] [PATCH 6/6] sql: extend result set with alias

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

Thanks for the fixes!

On 05/12/2019 12:51, Nikita Pettik wrote:
> On 28 Nov 23:41, Vladislav Shpilevoy wrote:
>> Thanks for the patch!
>>
>> On 27/11/2019 13:15, Nikita Pettik wrote:
>>> Each column of result set can feature its name alias. For instance:
>>>
>>> SELECT x + 1 AS add FROM ...;
>>>
>>> In this case real name of resulting set column is "x + 1" meanwhile
>>> "add" is its alias. This patch extends metadata with optional metadata
>>> member which corresponds to column's alias.
>>
>> I was always thinking that the alias should be returned as a
>> name. And the real name should be returned as meta. And looks
>> like it is so:
>>
>>     tarantool> box.execute('SELECT 1 AS kek')
>>     ---
>>     - metadata:
>>       - name: KEK
>>         type: integer
>>       rows:
>>       - [1]
>>     ...
>>
>> That makes me think we should not break it. And
>> meta should return the real name in case there is
>> an alias. Because otherwise the aliases are useless
>> in meta.
> 
> (ANSI parts which concern Java and CLI are quite complicated to read
>  and understand, so I refer to Oracle docs).
> 
> https://docs.oracle.com/javase/8/docs/api/java/sql/ResultSetMetaData.html#getColumnLabel-int-
> 
> 'The suggested title is usually specified by the SQL AS clause.'
> 
> https://stackoverflow.com/questions/4271152/getcolumnlabel-vs-getcolumnname
> 
> I assume that :getColumnLabel() returns name of label, not real name
> (at least it seems to be rational).
>  

I discussed it with Alexander. And that subject is complicated.
I don't know exactly what do we need to return. All the drivers
work differently.

Here is what I understood from the discussion:

    Metadata contains 'label' and 'name'.
    There are 2 cases: SELECTed column is an expression, or a
    table column.

    - In case the result set column is an expression, the
      label is the value after 'AS'. If the alias is not
      specified, the label may be anything. For example,
      the expression string representation. 'Name' is
      undefined.

    - In case the result set column is a table column,
      label is the value after 'AS'. If the alias is not
      specified, it is the original column name (i.e. the
      value before 'AS'). 'Name' is the original column
      name.

'Label' is just something printable to show to a user.
'Name' is something functional. This may be used to generate
an update request.

In our current implementation IPROTO_FIELD_NAME is in fact
'label'. And we don't have 'name'. I think you need to keep
the IPROTO_FIELD_NAME as is, and add 'name' as the driver
expects it. For example, IPROTO_FIELD_ORIG_NAME. I.e. the
original column name for table columns. That will keep
backward compatibility, and will provide all the needed meta.

You also need to get in touch with Alexander about that. I may
be wrong about the points above, and he knows more.

>> Btw the example above is executed on this commit. So
>> now the results are inconsistent. Some queries return
>> alias in 'name'. Some return a real name in 'name'. I
>> think we should keep it as was, and return alias in
>> 'name'.
> 
> Sorry, now it is fixed:
> 
> - metadata:
>   - type: integer
>     name: '1'
>     alias: KEK
>   rows:
>   - [1]
> ...
> 
> I've extended commit message with doc bot request:
> 
>     sql: extend result set with alias
>     
>     Each column of result set can feature its name alias. For instance:
>     
>     SELECT x + 1 AS add FROM ...;
>     
>     In this case real name of resulting set column is "x + 1" meanwhile
>     "add" is its alias. This patch extends metadata with optional metadata
>     member which corresponds to column's alias.
>     
>     Closes #4407
>     
>     @TarantoolBot document
>     Title: extended SQL metadata
>     
>     Before this patch metadata for SQL DQL contained only two fields:
>     name and type of each column of result set. Now it may contain
>     following properties:
>      - collation (in case type of resulting set column is string and
>                   collation is different from default "none");
>        is encoded with IPROTO_FIELD_COLL key in IPROTO_METADATA map;
>      - is_nullable (in case column of result set corresponds to space's
>                     field; for expressions like x+1 for the sake of
>                     simplicity nullability is omitted);
>        is encoded with IPROTO_FIELD_IS_NULLABLE key in IPROTO_METADATA;
>      - is_autoincrement (is set only for autoincrement column in result
>                          set);
>        is encoded with IPROTO_FIELD_IS_AUNTOINCREMENT key in IPROTO_METADATA;
>      - alias (if column of result set is specified with AS label);
>        is encoded with IPROTO_FIELD_ALIAS key in IPROTO_METADATA map.
> 

Sorry, this is not enough. You need to describe the
binary protocol. With exact numeric values for the new
IProto keys. And exact MessagePack types.

Also you didn't say, that omitted nullable means unknown
nullability.