From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4979246971A for ; Fri, 6 Dec 2019 03:02:11 +0300 (MSK) References: <8652d840-2b81-9e49-eb5a-1133673b11bf@tarantool.org> <20191205115121.GA4490@tarantool.org> From: Vladislav Shpilevoy Message-ID: <5126f45a-d7b2-dca5-8fb6-823b1163d2f7@tarantool.org> Date: Fri, 6 Dec 2019 01:02:09 +0100 MIME-Version: 1.0 In-Reply-To: <20191205115121.GA4490@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 6/6] sql: extend result set with alias List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.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.