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

[Nikita Pettik <korablev@tarantool.org>]

On 06 Dec 01:02, Vladislav Shpilevoy wrote:
> 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.

Why name is undefined? I'd say vice versa - alias is undefined
(for the simplicity sake let's say it equals to name); name is
string representation (in most cases). 

https://grokbase.com/t/postgresql/pgsql-jdbc/047wwctbyf/wrong-column-names-in-resultsetmetadata
'''
Most return the same thing as getColumnName(); not surprising
since that's a good default display title.
'''

Some time ago, there was discussion in dev mailing list concerning
default column names btw.

>     - 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.

For me it sounds extremely wierd, I can't realize how did
you come up with these thoughts...Could you please provide
examples of DBs/drivers where such behavior can be observer?
The only thing I found is an ancient discussion in PostgreSQL
mailing list:
https://grokbase.com/t/postgresql/pgsql-jdbc/047wwctbyf/wrong-column-names-in-resultsetmetadata

All other resources I've visited say that alias is considered to be the
indentifier coming after AS clause; name is always indentifier that
is specified in result set (regardless presence of AS clause).

For instance, DB2 (which we consider to be the closest to ANSI):
https://www.ibm.com/support/knowledgecenter/en/SSEPGG_9.7.0/com.ibm.db2.luw.apdv.java.doc/src/tpc/imjcc_c0052593.html

The only exception is MimerSQL where both getColumnLabel() and
getColumnName() always return alias (p.37 changes in 2.2):
https://download.mimer.com/pub/developer/docs/latest_jdbcguide/mimjdben.pdf

> 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.

Updated request:

    @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 (0x2) key in IPROTO_METADATA map;
       in msgpack is encoded as string and held with MP_STR type;
     - 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 (0x3) in IPROTO_METADATA;
       in msgpack is encoded as boolean and held with MP_BOOL type;
       note that absence of this field implies that nullability is unknown;
     - is_autoincrement (is set only for autoincrement column in result
                         set);
       is encoded with IPROTO_FIELD_IS_AUNTOINCREMENT (0x4) key in IPROTO_METADATA;
       in msgpack is encoded as boolean and held with MP_BOOL type;
     - alias (if column of result set is specified with AS label);
       is encoded with IPROTO_FIELD_ALIAS (0x5) key in IPROTO_METADATA map;
       in msgpack is encoded as string and held with MP_STR type.