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

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

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.

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

> 
> Closes #4407
> ---
>  src/box/execute.c              | 18 +++++++++++++----
>  src/box/iproto_constants.h     |  1 +
>  src/box/lua/execute.c          |  5 +++++
>  src/box/lua/net_box.c          |  6 +++++-
>  src/box/sql/select.c           |  9 +++++++--
>  src/box/sql/sqlInt.h           |  3 +++
>  src/box/sql/vdbe.h             |  3 +++
>  src/box/sql/vdbeInt.h          |  1 +
>  src/box/sql/vdbeapi.c          |  8 ++++++++
>  src/box/sql/vdbeaux.c          | 15 ++++++++++++++
>  test/sql-tap/badutf1.test.lua  | 46 +++++++++++++++++++++---------------------
>  test/sql-tap/colname.test.lua  | 16 +++++++--------
>  test/sql-tap/lua/sqltester.lua | 29 ++++++++++++++++++++++++++
>  test/sql-tap/select1.test.lua  | 18 ++++++++---------
>  test/sql-tap/select4.test.lua  |  4 ++--
>  test/sql-tap/view.test.lua     |  2 +-
>  test/sql/bind.result           | 15 ++++++++------
>  test/sql/boolean.result        |  6 ++++--
>  test/sql/collation.result      | 16 +++++++++------
>  test/sql/iproto.result         |  5 +++--
>  20 files changed, 160 insertions(+), 66 deletions(-)
> 
> diff --git a/test/sql-tap/badutf1.test.lua b/test/sql-tap/badutf1.test.lua
> index 9079dfe25..6623999d0 100755
> --- a/test/sql-tap/badutf1.test.lua
> +++ b/test/sql-tap/badutf1.test.lua
> @@ -248,6 +269,14 @@ local function execsql2(self, sql)
>  end
>  test.execsql2 = execsql2
>  
> +local function execsql_aliases(self, sql)
> +    local result, metadata = execsql_one_by_one(sql)
> +    if type(result) ~= 'table' then return end
> +    result = flattern_with_column_aliases(result, metadata)
> +    return result
> +end
> +test.execsql_aliases = execsql_aliases

Well, these dancing with aliases is really weird. Seems like
your patch broke the names. AFAIU, as a name you should return
the alias, when it is specified. And the real name is returned
optionally, in meta.

> +
>  local function sortsql(self, sql)
>      local result = execsql(self, sql)
>      table.sort(result, function(a,b) return a[2] < b[2] end)