Tarantool development patches archive
 help / color / mirror / Atom feed
From: "n.pettik" <korablev@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Imeev Mergen <imeevma@tarantool.org>,
	Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Subject: [tarantool-patches] Re: [PATCH 2/3] sql: SELECT from system spaces returns unpacked msgpack.
Date: Mon, 19 Nov 2018 20:27:39 +0300	[thread overview]
Message-ID: <5233E694-9416-4094-8E73-1F46B3205C91@tarantool.org> (raw)
In-Reply-To: <9e22a12510a952d2378d8b926db266f4a426c332.1542635129.git.v.shpilevoy@tarantool.org>

Please, don’t state problem in commit message. Instead, it would be
better to indicate fix of the problem. I would call it like:

sql: decode mgspack after SELECT from system spaces

Otherwise it looks like you are introducing new bug :)

Also, your patch affects not only system spaces.
For example:

box.schema.space.create('t’)
box.space.t:format({{name = 'id', type = 'integer'}, {name = 'x', type = 'any'}})
box.space.t:create_index('i1', {parts = {1, 'int'}})
box.space.t:insert({1, {1,2,3}})
box.space.t:insert({2, {a = 3}})

Before your patch:

tarantool> cn:execute('select * from \"t\"')
---
- metadata:
  - name: id
    type: UNKNOWN
  - name: x
    type: UNKNOWN
  rows:
  - [1, !!binary kwECAw==]
  - [2, !!binary gaFhAw==]
...

After:

tarantool> cn:execute('select * from \"t\"')
---
- metadata:
  - name: id
    type: UNKNOWN
  - name: x
    type: UNKNOWN
  rows:
  - [1, [1, 2, 3]]
  - [2, {'a': 3}]
…

I guess more precise to say sort of:

sql: decode ARRAY and MAP types after SELECT

Patch itself is OK, but I would rework test in order to avoid
using system spaces. It is up to you.

  reply	other threads:[~2018-11-19 17:27 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-19 13:47 [tarantool-patches] [PATCH 0/3] A set of small SQL fixes Vladislav Shpilevoy
2018-11-19 13:47 ` [tarantool-patches] [PATCH 1/3] sql: EXPLAIN through net.box leads to SEGFAULT Vladislav Shpilevoy
2018-11-19 17:27   ` [tarantool-patches] " n.pettik
2018-11-22 18:08     ` Imeev Mergen
2018-11-22 18:31       ` Vladislav Shpilevoy
2018-11-22 20:55         ` [tarantool-patches] Re[2]: " Мерген Имеев
2018-11-23  6:04           ` [tarantool-patches] Re: Re[2]: " Kirill Yukhin
2018-11-28 12:36           ` [tarantool-patches] " n.pettik
2018-11-29 14:58             ` Konstantin Osipov
2018-11-19 13:47 ` [tarantool-patches] [PATCH 2/3] sql: SELECT from system spaces returns unpacked msgpack Vladislav Shpilevoy
2018-11-19 17:27   ` n.pettik [this message]
2018-11-22 18:09     ` [tarantool-patches] " Imeev Mergen
2018-11-27  8:51       ` n.pettik
2018-11-19 13:47 ` [tarantool-patches] [PATCH 3/3] sql: too many autogenerated ids leads to SEGFAULT Vladislav Shpilevoy
2018-11-19 17:27   ` [tarantool-patches] " n.pettik
2018-11-22 18:09     ` Imeev Mergen
2018-11-27  8:50       ` n.pettik
2018-11-27 10:09 ` [tarantool-patches] Re: [PATCH 0/3] A set of small SQL fixes Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5233E694-9416-4094-8E73-1F46B3205C91@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=imeevma@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='[tarantool-patches] Re: [PATCH 2/3] sql: SELECT from system spaces returns unpacked msgpack.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox