<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p> Hi! Thank you for review! Diff between versions and new patch<br>
below.</p>
<div class="moz-cite-prefix">On 11/19/18 8:27 PM, n.pettik wrote:<br>
</div>
<blockquote type="cite"
cite="mid:5233E694-9416-4094-8E73-1F46B3205C91@tarantool.org">
<pre class="moz-quote-pre" wrap="">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.
</pre>
</blockquote>
<p><b>Diff between versions:</b></p>
<p>diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>
index b866140..6c50781 100644<br>
--- a/test/sql/iproto.result<br>
+++ b/test/sql/iproto.result<br>
@@ -811,14 +811,37 @@ cn:execute("PRAGMA TABLE_INFO(test);")<br>
box.sql.execute('DROP TABLE test')<br>
---<br>
...<br>
--- SELECT from system spaces returns unpacked msgpack.<br>
-res = cn:execute('select "format" from "_space" limit 1;')<br>
+-- SELECT returns unpacked msgpack.<br>
+format = {{name = 'id', type = 'integer'}, {name = 'x', type =
'any'}}<br>
---<br>
...<br>
-res.rows<br>
+s = box.schema.space.create('test', {format=format})<br>
+---<br>
+...<br>
+i1 = s:create_index('i1', {parts = {1, 'int'}})<br>
+---<br>
+...<br>
+s:insert({1, {1,2,3}})<br>
+---<br>
+- [1, [1, 2, 3]]<br>
+...<br>
+s:insert({2, {a = 3}})<br>
+---<br>
+- [2, {'a': 3}]<br>
+...<br>
+cn:execute('select * from "test"')<br>
+---<br>
+- metadata:<br>
+ - name: id<br>
+ type: UNKNOWN<br>
+ - name: x<br>
+ type: UNKNOWN<br>
+ rows:<br>
+ - [1, [1, 2, 3]]<br>
+ - [2, {'a': 3}]<br>
+...<br>
+s:drop()<br>
---<br>
-- - [[{'name': 'space_id', 'type': 'unsigned'}, {'name': 'lsn',
'type': 'unsigned'},<br>
- {'name': 'tuple', 'type': 'array'}]]<br>
...<br>
cn:close()<br>
---<br>
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>
index 6299814..e12decd 100644<br>
--- a/test/sql/iproto.test.lua<br>
+++ b/test/sql/iproto.test.lua<br>
@@ -263,9 +263,14 @@ box.sql.execute('CREATE TABLE test (id INT
PRIMARY KEY)')<br>
cn:execute("PRAGMA TABLE_INFO(test);")<br>
box.sql.execute('DROP TABLE test')<br>
<br>
--- SELECT from system spaces returns unpacked msgpack.<br>
-res = cn:execute('select "format" from "_space" limit 1;')<br>
-res.rows<br>
+-- SELECT returns unpacked msgpack.<br>
+format = {{name = 'id', type = 'integer'}, {name = 'x', type =
'any'}}<br>
+s = box.schema.space.create('test', {format=format})<br>
+i1 = s:create_index('i1', {parts = {1, 'int'}})<br>
+s:insert({1, {1,2,3}})<br>
+s:insert({2, {a = 3}})<br>
+cn:execute('select * from "test"')<br>
+s:drop()<br>
<br>
cn:close()<br>
<br>
<b>New patch:</b></p>
<p>commit 8482e0c0bc1fa44faa6abf7b7b8a27b77e91db92<br>
Author: Mergen Imeev <a class="moz-txt-link-rfc2396E"
href="mailto:imeevma@gmail.com"><imeevma@gmail.com></a><br>
Date: Fri Nov 16 14:23:39 2018 +0300<br>
<br>
sql: decode ARRAY and MAP types after SELECT<br>
<br>
Before this patch MSGPACK received using SELECT statement
through<br>
net.box was unpacked. Fixed in this patch.<br>
<br>
diff --git a/src/box/execute.c b/src/box/execute.c<br>
index cd809f8..fb3e08b 100644<br>
--- a/src/box/execute.c<br>
+++ b/src/box/execute.c<br>
@@ -351,13 +351,21 @@ sql_column_to_messagepack(struct
sqlite3_stmt *stmt, int i,<br>
}<br>
case SQLITE_BLOB: {<br>
uint32_t len = sqlite3_column_bytes(stmt, i);<br>
- size = mp_sizeof_bin(len);<br>
- char *pos = (char *) region_alloc(region, size);<br>
- if (pos == NULL)<br>
- goto oom;<br>
- const char *s;<br>
- s = (const char *)sqlite3_column_blob(stmt, i);<br>
- mp_encode_bin(pos, s, len);<br>
+ const char *s =<br>
+ (const char *)sqlite3_column_blob(stmt, i);<br>
+ if (sql_column_subtype(stmt, i) == SQL_SUBTYPE_MSGPACK) {<br>
+ size = len;<br>
+ char *pos = (char *)region_alloc(region, size);<br>
+ if (pos == NULL)<br>
+ goto oom;<br>
+ memcpy(pos, s, len);<br>
+ } else {<br>
+ size = mp_sizeof_bin(len);<br>
+ char *pos = (char *)region_alloc(region, size);<br>
+ if (pos == NULL)<br>
+ goto oom;<br>
+ mp_encode_bin(pos, s, len);<br>
+ }<br>
break;<br>
}<br>
case SQLITE_NULL: {<br>
diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>
index f229427..6c50781 100644<br>
--- a/test/sql/iproto.result<br>
+++ b/test/sql/iproto.result<br>
@@ -811,6 +811,38 @@ cn:execute("PRAGMA TABLE_INFO(test);")<br>
box.sql.execute('DROP TABLE test')<br>
---<br>
...<br>
+-- SELECT returns unpacked msgpack.<br>
+format = {{name = 'id', type = 'integer'}, {name = 'x', type =
'any'}}<br>
+---<br>
+...<br>
+s = box.schema.space.create('test', {format=format})<br>
+---<br>
+...<br>
+i1 = s:create_index('i1', {parts = {1, 'int'}})<br>
+---<br>
+...<br>
+s:insert({1, {1,2,3}})<br>
+---<br>
+- [1, [1, 2, 3]]<br>
+...<br>
+s:insert({2, {a = 3}})<br>
+---<br>
+- [2, {'a': 3}]<br>
+...<br>
+cn:execute('select * from "test"')<br>
+---<br>
+- metadata:<br>
+ - name: id<br>
+ type: UNKNOWN<br>
+ - name: x<br>
+ type: UNKNOWN<br>
+ rows:<br>
+ - [1, [1, 2, 3]]<br>
+ - [2, {'a': 3}]<br>
+...<br>
+s:drop()<br>
+---<br>
+...<br>
cn:close()<br>
---<br>
...<br>
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>
index 9bcc7ef..e12decd 100644<br>
--- a/test/sql/iproto.test.lua<br>
+++ b/test/sql/iproto.test.lua<br>
@@ -263,6 +263,15 @@ box.sql.execute('CREATE TABLE test (id INT
PRIMARY KEY)')<br>
cn:execute("PRAGMA TABLE_INFO(test);")<br>
box.sql.execute('DROP TABLE test')<br>
<br>
+-- SELECT returns unpacked msgpack.<br>
+format = {{name = 'id', type = 'integer'}, {name = 'x', type =
'any'}}<br>
+s = box.schema.space.create('test', {format=format})<br>
+i1 = s:create_index('i1', {parts = {1, 'int'}})<br>
+s:insert({1, {1,2,3}})<br>
+s:insert({2, {a = 3}})<br>
+cn:execute('select * from "test"')<br>
+s:drop()<br>
+<br>
cn:close()<br>
<br>
box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>
<br>
</p>
</body>
</html>