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

[Vladislav Shpilevoy]

Thanks for the discussion!

>>>>> diff --git a/test/sql/full_metadata.result b/test/sql/full_metadata.result
>>>>> index 7c2982682..8f0bced36 100644
>>>>> --- a/test/sql/full_metadata.result
>>>>> +++ b/test/sql/full_metadata.result
>>>>> @@ -118,6 +121,35 @@ execute("SELECT * FROM t;")
>>>>>   |   - [1, 1, 'aSd']
>>>>>   | ...
>>>>>  
>>>>> +-- Alias is always set in extended metadata. If column label in
>>>>> +-- form of AS clause is set, then this alias is presented in
>>>>> +-- metadata. Otherwise, alias is just the same as name.
>>>>
>>>> 4. But it is not true. The alias is not set always:
>>>>
>>>> =========================================================
>>>>
>>>> tarantool> box.execute('PRAGMA full_metadata = true;')
>>>> ---
>>>> - row_count: 0
>>>> ...
>>>>
>>>> tarantool> box.execute('SELECT 1')
>>>> ---
>>>> - metadata:
>>>>   - name: '1'
>>>>     type: integer
>>>>   rows:
>>>>   - [1]
>>>> ...
>>>>
>>>> =========================================================
>>>>
>>>> As you can see, no AS = no alias.
>>>
>>> Always displaying alias (or label - call it whatever you want)
>>> even in full_metadata mode doesn't seem to be reasonable for me.
>>> Users can easily handle it in their code. Sending one map field
>>> with string value duplicating the name and decoding it may affect
>>> performance much more significant than one NULL check (IMHO).
>>
>> As I said already, I don't propose to send a duplicate name. I
>> propose to handle it on the client side. In Lua you can push one
>> string 2 times, and it will cost nothing - they will reference
>> one string object.
>>
>> In IProto you still can send one name, when possible, and on the
>> netbox side push both names.
> 
> From performance point of view this is ok. But still some
> users may wonder why alias/label is displayed while there's
> no explicit AS clause (even in full_metadata mode).

When it is called 'label', I don't see a problem in showing it
always. It does not say, that it is something after AS. Now we
show 'name', even for expressions, and IMO this is more confusing.
Because expressions have no name, strictly speaking.

> Moreover, now client only decodes received metadata, but in your
> suggestion it is assumed that full_metadata mode is the same on
> client and server. Otherwise, it would look inconsistent:

I don't propose to introduce client-side full_metadata, it makes
no sense. But I do understand the problem.

Then we could send the not specified 'label' as MP_NIL. That will
occupy a couple of bytes per column, and will mean, that it is the
same as the 'name'. Or we could send a flag in IPROTO_FLAGS, that
the response contains full metadata even if it does not look so. It
depends on whether we need to know fullness of the metadata on the
client for anything else or not.

> server (full_metadata = true), client (full_metadata = off):
> server sends name, is_autoincrement, but client ignores extended
> metadata and displays only name;
> 
> server (full_metadata = false), clien (full_metadata = true):
> server sends name (but in full_metadata mode there's also is_autoincrement
> for instance), but client displays only half of extended metadata (alias,
> but not is_autoincrement or other fields).
> 
> Surely, we can send server's metadata status, but is it worth it?

I don't know. This is purely subjective thing - should we show both
name and label in full_metadata always or not. I think we should, and is
worth doing. You think we shouldn't. I provide arguments, that it will
simplify client's code. You provide that without this we don't need to
send MP_NIL as a not specified name / shouldn't push 2 names, when there
is only one, and it is maybe faster.

Also my argument is that client-side code is not so perf sensitive as
server's. So even if it will show sometimes 2 names instead of 1, it
won't really affect anything.

You can try asking in the red chat, what behaviour they expect.

>>>> This is what I don't like
>>>> in this patch the most. A user does not have a reliable always
>>>> present column with a displayable name: either the column
>>>> span or its alias after AS. He can't use name, because it
>>>> changes its behavour depending on AS. And he can't use alias,
>>>> because sometimes it is not present.
>>>
>>>> That leads to confusion,
>>>> and necessity to branch in user code a lot.
>>>
>>> Yep, if user wants (for some reason; I don't have statistics how
>>> often alias value can be useful or/and is used in production code)
>>> to get alias value, one should make a small effort and add one
>>> 'if' condition:
>>>
>>> if (is_full_metadata && alias == NULL)
>>>     alias = name;
>>>
>>> That's it.
>>>
>>
>> Why, if we can remove that effort easily? I still don't see any
>> real arguments why can't we always push a label. The argument
>> about sending duplicate names is really weak - you can avoid
>> sending two names, and push them on the client side, as I said
>> above. No extra payload, not extra MessagePack decoding.
>>
>> Even the standard driver, to which you appeal so much, always
>> returns a label.
>>> Alternatively (I don't take into consideration option of swapping
>>> name and alias as you suggest) we can change current default behaviour
>>> and in 'casual metadata mode' return name as name (not alias):
>>>
>>> SELECT 1 AS x; -- would return '1' as name (now it returns 'x' as name). 
>>>
>>> But I guess some users may turn out to be confused (in case they
>>> haven't red about full_metadata pragma in docs).
>>>
>>>> The same example shows, that alias is a bad name. Here you need
>>>> to return it, but it is not alias like AS. 'Label' would fit
>>>> better here.
>>>
>>> I don't care much how to call it. If you prefer 'label', I will rename it.
>>
>> Yes, I prefer label. Not because this is my personal taste. I provided
>> reasons, why it can't be called 'alias'. If you think alias is better,
>> they say why. That is a discussion.
> 
> There are no strict definitions under these terms, so it is just
> matter of taste. Let's call it label, I am not against (I guess
> you assume that label = (name ∪ alias), meanwhile alias is supposed
> only to be idenfitier after AS clause).
Yes, this is why I vote for label.