[tarantool-patches] [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[tarantool-patches] [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[Kirill Shcherbatov]

Previously MsgPacks BLOB objects were shown as base64 strings.
Now we print them in human-readable form.

Resolves #2369.
---
https://github.com/tarantool/tarantool/tree/kshch/gh-2369-display-tuple-msgpucks
https://github.com/tarantool/tarantool/issues/2369

 src/box/lua/sql.c                                |  8 +++++++-
 src/box/sql/sqliteInt.h                          | 12 ++++++++++++
 src/box/sql/vdbeInt.h                            |  3 +--
 src/box/sql/vdbeapi.c                            |  8 ++++++++
 test/sql/gh2141-delete-trigger-drop-table.result | 12 ++++++------
 test/sql/persistency.result                      |  9 ++++++---
 6 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c
index e693aea..7a49aac 100644
--- a/src/box/lua/sql.c
+++ b/src/box/lua/sql.c
@@ -1,5 +1,6 @@
 #include "sql.h"
 #include "box/sql.h"
+#include "lua/msgpack.h"
 
 #include "box/sql/sqliteInt.h"
 #include "box/info.h"
@@ -44,8 +45,13 @@ lua_push_row(struct lua_State *L, struct sqlite3_stmt *stmt)
 		}
 		case SQLITE_BLOB: {
 			const void *blob = sqlite3_column_blob(stmt, i);
-			lua_pushlstring(L, blob,
+			if (sqlite3_column_subtype(stmt,i) == MSGPACK_SUBTYPE) {
+				luamp_decode(L, luaL_msgpack_default,
+					     (const char **)&blob);
+			} else {
+				lua_pushlstring(L, blob,
 					sqlite3_column_bytes(stmt, i));
+			}
 			break;
 		}
 		case SQLITE_NULL:
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index e939663..3854482 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -482,6 +482,15 @@ sqlite3_value_double(sqlite3_value *);
 int
 sqlite3_value_int(sqlite3_value *);
 
+/**
+ * Get row column subtype.
+ * @param stmt row data to process.
+ * @param i column index.
+ * @retval sqlute3 subtype if any, 0 else.
+ */
+unsigned int
+sqlite3_column_subtype(sqlite3_stmt *stmt, int i);
+
 sqlite3_int64
 sqlite3_value_int64(sqlite3_value *);
 
@@ -672,6 +681,9 @@ enum sql_type {
 	SQLITE_NULL = 5,
 };
 
+/** The "subtype" set for MsgPack values.*/
+#define MSGPACK_SUBTYPE  77
+
 /**
  * Structure for internal usage during INSERT/UPDATE
  * statements compilation.
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 0115a0d..1b767a5 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -253,8 +253,7 @@ struct Mem {
 #define MEM_Zero 0x0000
 #endif
 
-/* The "subtype" set for MsgPack values */
-#define MSGPACK_SUBTYPE  77	/* Ascii for "M" */
+
 
 /* Return TRUE if Mem X contains dynamically allocated content - anything
  * that needs to be deallocated to avoid a leak.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index d35338a..d1ae976 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -1047,6 +1047,14 @@ sqlite3_column_type(sqlite3_stmt * pStmt, int i)
 	return iType;
 }
 
+unsigned int
+sqlite3_column_subtype(sqlite3_stmt * pStmt, int i)
+{
+	unsigned int type = sqlite3_value_subtype(columnMem(pStmt, i));
+	columnMallocFailure(pStmt);
+	return type;
+}
+
 /*
  * Convert the N-th element of pStmt->pColName[] into a string using
  * xFunc() then return that string.  If N is out of range, return 0.
diff --git a/test/sql/gh2141-delete-trigger-drop-table.result b/test/sql/gh2141-delete-trigger-drop-table.result
index ec5a380..3ccb416 100644
--- a/test/sql/gh2141-delete-trigger-drop-table.result
+++ b/test/sql/gh2141-delete-trigger-drop-table.result
@@ -26,12 +26,12 @@ box.sql.execute("CREATE TRIGGER tt_ad AFTER DELETE ON t BEGIN SELECT 1; END")
 -- check that these triggers exist
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"")
 ---
-- - ['TT_AD', !!binary gaNzcWzZOkNSRUFURSBUUklHR0VSIHR0X2FkIEFGVEVSIERFTEVURSBPTiB0IEJFR0lOIFNFTEVDVCAxOyBFTkQ=]
-  - ['TT_AI', !!binary gaNzcWzZOkNSRUFURSBUUklHR0VSIHR0X2FpIEFGVEVSIElOU0VSVCBPTiB0IEJFR0lOIFNFTEVDVCAxOyBFTkQ=]
-  - ['TT_AU', !!binary gaNzcWzZOkNSRUFURSBUUklHR0VSIHR0X2F1IEFGVEVSIFVQREFURSBPTiB0IEJFR0lOIFNFTEVDVCAxOyBFTkQ=]
-  - ['TT_BD', !!binary gaNzcWzZO0NSRUFURSBUUklHR0VSIHR0X2JkIEJFRk9SRSBERUxFVEUgT04gdCBCRUdJTiBTRUxFQ1QgMTsgRU5E]
-  - ['TT_BI', !!binary gaNzcWzZO0NSRUFURSBUUklHR0VSIHR0X2JpIEJFRk9SRSBJTlNFUlQgT04gdCBCRUdJTiBTRUxFQ1QgMTsgRU5E]
-  - ['TT_BU', !!binary gaNzcWzZO0NSRUFURSBUUklHR0VSIHR0X2J1IEJFRk9SRSBVUERBVEUgT04gdCBCRUdJTiBTRUxFQ1QgMTsgRU5E]
+- - ['TT_AD', {'sql': 'CREATE TRIGGER tt_ad AFTER DELETE ON t BEGIN SELECT 1; END'}]
+  - ['TT_AI', {'sql': 'CREATE TRIGGER tt_ai AFTER INSERT ON t BEGIN SELECT 1; END'}]
+  - ['TT_AU', {'sql': 'CREATE TRIGGER tt_au AFTER UPDATE ON t BEGIN SELECT 1; END'}]
+  - ['TT_BD', {'sql': 'CREATE TRIGGER tt_bd BEFORE DELETE ON t BEGIN SELECT 1; END'}]
+  - ['TT_BI', {'sql': 'CREATE TRIGGER tt_bi BEFORE INSERT ON t BEGIN SELECT 1; END'}]
+  - ['TT_BU', {'sql': 'CREATE TRIGGER tt_bu BEFORE UPDATE ON t BEGIN SELECT 1; END'}]
 ...
 -- drop table
 box.sql.execute("DROP TABLE t")
diff --git a/test/sql/persistency.result b/test/sql/persistency.result
index 8f89039..10b3dd7 100644
--- a/test/sql/persistency.result
+++ b/test/sql/persistency.result
@@ -142,7 +142,8 @@ box.sql.execute("CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT INTO
 ...
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"");
 ---
-- - ['TFOOBAR', !!binary gaNzcWzZaUNSRUFURSBUUklHR0VSIHRmb29iYXIgQUZURVIgSU5TRVJUIE9OIGZvb2JhciBCRUdJTiBJTlNFUlQgSU5UTyBiYXJmb28gVkFMVUVTICgndHJpZ2dlciB0ZXN0JywgOTk5OSk7IEVORA==]
+- - ['TFOOBAR', {'sql': 'CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT
+        INTO barfoo VALUES (''trigger test'', 9999); END'}]
 ...
 -- Many entries
 box.sql.execute("CREATE TABLE t1(a,b,c,PRIMARY KEY(b,c));")
@@ -168,7 +169,8 @@ test_run:cmd('restart server default');
 -- prove that trigger survived
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"");
 ---
-- - ['TFOOBAR', !!binary gaNzcWzZaUNSRUFURSBUUklHR0VSIHRmb29iYXIgQUZURVIgSU5TRVJUIE9OIGZvb2JhciBCRUdJTiBJTlNFUlQgSU5UTyBiYXJmb28gVkFMVUVTICgndHJpZ2dlciB0ZXN0JywgOTk5OSk7IEVORA==]
+- - ['TFOOBAR', {'sql': 'CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT
+        INTO barfoo VALUES (''trigger test'', 9999); END'}]
 ...
 -- ... functional
 box.sql.execute("INSERT INTO foobar VALUES ('foobar trigger test', 8888)")
@@ -181,7 +183,8 @@ box.sql.execute("SELECT * FROM barfoo WHERE foo = 9999");
 -- and still persistent
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"")
 ---
-- - ['TFOOBAR', !!binary gaNzcWzZaUNSRUFURSBUUklHR0VSIHRmb29iYXIgQUZURVIgSU5TRVJUIE9OIGZvb2JhciBCRUdJTiBJTlNFUlQgSU5UTyBiYXJmb28gVkFMVUVTICgndHJpZ2dlciB0ZXN0JywgOTk5OSk7IEVORA==]
+- - ['TFOOBAR', {'sql': 'CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT
+        INTO barfoo VALUES (''trigger test'', 9999); END'}]
 ...
 -- and can be dropped just once
 box.sql.execute("DROP TRIGGER tfoobar")
-- 
2.7.4

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[n.pettik]

> diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c
> index e693aea..7a49aac 100644
> --- a/src/box/lua/sql.c
> +++ b/src/box/lua/sql.c
> @@ -1,5 +1,6 @@
> #include "sql.h"
> #include "box/sql.h"
> +#include "lua/msgpack.h"
> 
> #include "box/sql/sqliteInt.h"
> #include "box/info.h"
> @@ -44,8 +45,13 @@ lua_push_row(struct lua_State *L, struct sqlite3_stmt *stmt)
> 		}
> 		case SQLITE_BLOB: {
> 			const void *blob = sqlite3_column_blob(stmt, i);
> -			lua_pushlstring(L, blob,
> +			if (sqlite3_column_subtype(stmt,i) == MSGPACK_SUBTYPE) {
> +				luamp_decode(L, luaL_msgpack_default,
> +					     (const char **)&blob);
> +			} else {
> +				lua_pushlstring(L, blob,
> 					sqlite3_column_bytes(stmt, i));
> +			}
> 			break;
> 		}
> 		case SQLITE_NULL:
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index e939663..3854482 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -482,6 +482,15 @@ sqlite3_value_double(sqlite3_value *);
> int
> sqlite3_value_int(sqlite3_value *);
> 
> +/**
> + * Get row column subtype.
> + * @param stmt row data to process.
> + * @param i column index.
> + * @retval sqlute3 subtype if any, 0 else.
> + */
> +unsigned int
> +sqlite3_column_subtype(sqlite3_stmt *stmt, int i);

Lets move from sqlite3_ prefixes and use simple sql_
(If you decide to keep this func).

> +
> sqlite3_int64
> sqlite3_value_int64(sqlite3_value *);
> 
> @@ -672,6 +681,9 @@ enum sql_type {
> 	SQLITE_NULL = 5,
> };
> 
> +/** The "subtype" set for MsgPack values.*/
> +#define MSGPACK_SUBTYPE  77
> +
> /**
>  * Structure for internal usage during INSERT/UPDATE
>  * statements compilation.
> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> index 0115a0d..1b767a5 100644
> --- a/src/box/sql/vdbeInt.h
> +++ b/src/box/sql/vdbeInt.h
> @@ -253,8 +253,7 @@ struct Mem {
> #define MEM_Zero 0x0000
> #endif
> 
> -/* The "subtype" set for MsgPack values */
> -#define MSGPACK_SUBTYPE  77	/* Ascii for "M" */
> +

Redundant empty line.

> 
> /* Return TRUE if Mem X contains dynamically allocated content - anything
>  * that needs to be deallocated to avoid a leak.
> diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
> index d35338a..d1ae976 100644
> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -1047,6 +1047,14 @@ sqlite3_column_type(sqlite3_stmt * pStmt, int i)
> 	return iType;
> }
> 
> +unsigned int
> +sqlite3_column_subtype(sqlite3_stmt * pStmt, int i)
> +{
> +	unsigned int type = sqlite3_value_subtype(columnMem(pStmt, i));
> +	columnMallocFailure(pStmt);
> +	return type;
> +}

Do we really need that call of columnMallocFailure()? It seems to be useless:
firstly, it comes in pair with sqlite3_column_blob() call; secondly, it is likely to be
one of SQLite artefacts. Anyway, you can just inline this function. If you want
to keep it, fix code style (remove extra space :sqlite3_stmt * pStmt, add struct prefix etc). 

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[Kirill Shcherbatov]

> Lets move from sqlite3_ prefixes and use simple sql_
> (If you decide to keep this func).
I don't like export columnMem function, so let's use sql_column_subtype.
> Redundant empty line.
Dropped.

> Do we really need that call of columnMallocFailure()? It seems to be useless:
> firstly, it comes in pair with sqlite3_column_blob() call; secondly, it is likely to be
> one of SQLite artefacts. Anyway, you can just inline this function. If you want
> to keep it, fix code style (remove extra space :sqlite3_stmt * pStmt, add struct prefix etc). 
No, not really.

unsigned int
sql_column_subtype(struct sqlite3_stmt *stmt, int i)
{
	return sqlite3_value_subtype(columnMem(stmt, i));;
}

=====================================

diff --git a/src/box/lua/sql.c b/src/box/lua/sql.c
index e693aea..917331c 100644
--- a/src/box/lua/sql.c
+++ b/src/box/lua/sql.c
@@ -1,5 +1,6 @@
 #include "sql.h"
 #include "box/sql.h"
+#include "lua/msgpack.h"
 
 #include "box/sql/sqliteInt.h"
 #include "box/info.h"
@@ -44,8 +45,13 @@ lua_push_row(struct lua_State *L, struct sqlite3_stmt *stmt)
 		}
 		case SQLITE_BLOB: {
 			const void *blob = sqlite3_column_blob(stmt, i);
-			lua_pushlstring(L, blob,
+			if (sql_column_subtype(stmt,i) == MSGPACK_SUBTYPE) {
+				luamp_decode(L, luaL_msgpack_default,
+					     (const char **)&blob);
+			} else {
+				lua_pushlstring(L, blob,
 					sqlite3_column_bytes(stmt, i));
+			}
 			break;
 		}
 		case SQLITE_NULL:
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index e939663..ca1df49 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -482,6 +482,15 @@ sqlite3_value_double(sqlite3_value *);
 int
 sqlite3_value_int(sqlite3_value *);
 
+/**
+ * Get row column subtype.
+ * @param stmt row data to process.
+ * @param i column index.
+ * @retval sqlute3 subtype if any, 0 else.
+ */
+unsigned int
+sql_column_subtype(struct sqlite3_stmt *stmt, int i);
+
 sqlite3_int64
 sqlite3_value_int64(sqlite3_value *);
 
@@ -672,6 +681,9 @@ enum sql_type {
 	SQLITE_NULL = 5,
 };
 
+/** The "subtype" set for MsgPack values.*/
+#define MSGPACK_SUBTYPE  77
+
 /**
  * Structure for internal usage during INSERT/UPDATE
  * statements compilation.
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 0115a0d..b76ba38 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -253,8 +253,6 @@ struct Mem {
 #define MEM_Zero 0x0000
 #endif
 
-/* The "subtype" set for MsgPack values */
-#define MSGPACK_SUBTYPE  77	/* Ascii for "M" */
 
 /* Return TRUE if Mem X contains dynamically allocated content - anything
  * that needs to be deallocated to avoid a leak.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index d35338a..bf552c8 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -1047,6 +1047,12 @@ sqlite3_column_type(sqlite3_stmt * pStmt, int i)
 	return iType;
 }
 
+unsigned int
+sql_column_subtype(struct sqlite3_stmt *stmt, int i)
+{
+	return sqlite3_value_subtype(columnMem(stmt, i));;
+}
+
 /*
  * Convert the N-th element of pStmt->pColName[] into a string using
  * xFunc() then return that string.  If N is out of range, return 0.
diff --git a/test/sql/gh2141-delete-trigger-drop-table.result b/test/sql/gh2141-delete-trigger-drop-table.result
index ec5a380..3ccb416 100644
--- a/test/sql/gh2141-delete-trigger-drop-table.result
+++ b/test/sql/gh2141-delete-trigger-drop-table.result
@@ -26,12 +26,12 @@ box.sql.execute("CREATE TRIGGER tt_ad AFTER DELETE ON t BEGIN SELECT 1; END")
 -- check that these triggers exist
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"")
 ---
-- - ['TT_AD', !!binary gaNzcWzZOkNSRUFURSBUUklHR0VSIHR0X2FkIEFGVEVSIERFTEVURSBPTiB0IEJFR0lOIFNFTEVDVCAxOyBFTkQ=]
-  - ['TT_AI', !!binary gaNzcWzZOkNSRUFURSBUUklHR0VSIHR0X2FpIEFGVEVSIElOU0VSVCBPTiB0IEJFR0lOIFNFTEVDVCAxOyBFTkQ=]
-  - ['TT_AU', !!binary gaNzcWzZOkNSRUFURSBUUklHR0VSIHR0X2F1IEFGVEVSIFVQREFURSBPTiB0IEJFR0lOIFNFTEVDVCAxOyBFTkQ=]
-  - ['TT_BD', !!binary gaNzcWzZO0NSRUFURSBUUklHR0VSIHR0X2JkIEJFRk9SRSBERUxFVEUgT04gdCBCRUdJTiBTRUxFQ1QgMTsgRU5E]
-  - ['TT_BI', !!binary gaNzcWzZO0NSRUFURSBUUklHR0VSIHR0X2JpIEJFRk9SRSBJTlNFUlQgT04gdCBCRUdJTiBTRUxFQ1QgMTsgRU5E]
-  - ['TT_BU', !!binary gaNzcWzZO0NSRUFURSBUUklHR0VSIHR0X2J1IEJFRk9SRSBVUERBVEUgT04gdCBCRUdJTiBTRUxFQ1QgMTsgRU5E]
+- - ['TT_AD', {'sql': 'CREATE TRIGGER tt_ad AFTER DELETE ON t BEGIN SELECT 1; END'}]
+  - ['TT_AI', {'sql': 'CREATE TRIGGER tt_ai AFTER INSERT ON t BEGIN SELECT 1; END'}]
+  - ['TT_AU', {'sql': 'CREATE TRIGGER tt_au AFTER UPDATE ON t BEGIN SELECT 1; END'}]
+  - ['TT_BD', {'sql': 'CREATE TRIGGER tt_bd BEFORE DELETE ON t BEGIN SELECT 1; END'}]
+  - ['TT_BI', {'sql': 'CREATE TRIGGER tt_bi BEFORE INSERT ON t BEGIN SELECT 1; END'}]
+  - ['TT_BU', {'sql': 'CREATE TRIGGER tt_bu BEFORE UPDATE ON t BEGIN SELECT 1; END'}]
 ...
 -- drop table
 box.sql.execute("DROP TABLE t")
diff --git a/test/sql/persistency.result b/test/sql/persistency.result
index 8f89039..10b3dd7 100644
--- a/test/sql/persistency.result
+++ b/test/sql/persistency.result
@@ -142,7 +142,8 @@ box.sql.execute("CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT INTO
 ...
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"");
 ---
-- - ['TFOOBAR', !!binary gaNzcWzZaUNSRUFURSBUUklHR0VSIHRmb29iYXIgQUZURVIgSU5TRVJUIE9OIGZvb2JhciBCRUdJTiBJTlNFUlQgSU5UTyBiYXJmb28gVkFMVUVTICgndHJpZ2dlciB0ZXN0JywgOTk5OSk7IEVORA==]
+- - ['TFOOBAR', {'sql': 'CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT
+        INTO barfoo VALUES (''trigger test'', 9999); END'}]
 ...
 -- Many entries
 box.sql.execute("CREATE TABLE t1(a,b,c,PRIMARY KEY(b,c));")
@@ -168,7 +169,8 @@ test_run:cmd('restart server default');
 -- prove that trigger survived
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"");
 ---
-- - ['TFOOBAR', !!binary gaNzcWzZaUNSRUFURSBUUklHR0VSIHRmb29iYXIgQUZURVIgSU5TRVJUIE9OIGZvb2JhciBCRUdJTiBJTlNFUlQgSU5UTyBiYXJmb28gVkFMVUVTICgndHJpZ2dlciB0ZXN0JywgOTk5OSk7IEVORA==]
+- - ['TFOOBAR', {'sql': 'CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT
+        INTO barfoo VALUES (''trigger test'', 9999); END'}]
 ...
 -- ... functional
 box.sql.execute("INSERT INTO foobar VALUES ('foobar trigger test', 8888)")
@@ -181,7 +183,8 @@ box.sql.execute("SELECT * FROM barfoo WHERE foo = 9999");
 -- and still persistent
 box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"")
 ---
-- - ['TFOOBAR', !!binary gaNzcWzZaUNSRUFURSBUUklHR0VSIHRmb29iYXIgQUZURVIgSU5TRVJUIE9OIGZvb2JhciBCRUdJTiBJTlNFUlQgSU5UTyBiYXJmb28gVkFMVUVTICgndHJpZ2dlciB0ZXN0JywgOTk5OSk7IEVORA==]
+- - ['TFOOBAR', {'sql': 'CREATE TRIGGER tfoobar AFTER INSERT ON foobar BEGIN INSERT
+        INTO barfoo VALUES (''trigger test'', 9999); END'}]
 ...
 -- and can be dropped just once
 box.sql.execute("DROP TRIGGER tfoobar")

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[n.pettik]

> --- a/src/box/sql/vdbeapi.c
> +++ b/src/box/sql/vdbeapi.c
> @@ -1047,6 +1047,12 @@ sqlite3_column_type(sqlite3_stmt * pStmt, int i)
> 	return iType;
> }
> 
> +unsigned int
> +sql_column_subtype(struct sqlite3_stmt *stmt, int i)
> +{
> +	return sqlite3_value_subtype(columnMem(stmt, i));;

Double ‘;’ at the end of line.

Except for this, LGTM.

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[Kirill Shcherbatov]

On 02.07.2018 17:55, n.pettik wrote:
> Double ‘;’ at the end of line.
> Except for this, LGTM.

Vlad, please, take a look when you would have time.

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[Vladislav Shpilevoy]

Hello. Thanks for the patch! I have pushed my fixes on the
branch in a separate commit. Please, look and squash if you
are ok with it.

Nikita, please, look at my changes too, they are not minor.


On 03/07/2018 15:48, Kirill Shcherbatov wrote:
> On 02.07.2018 17:55, n.pettik wrote:
>> Double ‘;’ at the end of line.
>> Except for this, LGTM.
> 
> Vlad, please, take a look when you would have time.
> 

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[Kirill Shcherbatov]

> Hello. Thanks for the patch! I have pushed my fixes on the
> branch in a separate commit. Please, look and squash if you
> are ok with it.
Tnx. Ok for me, excluding this:

+++ b/src/box/sql/vdbeapi.c
@@ -224,8 +224,7 @@ sqlite3_value_int64(sqlite3_value * pVal)
enum sql_subtype
sqlite3_value_subtype(sqlite3_value * pVal)
{
- Mem *pMem = (Mem *) pVal;
- return (pMem->flags & MEM_Subtype) != 0 ? pMem->subtype : 0;
+ return (pVal->flags & MEM_Subtype) != 0 ? pVal->subtype : SQL_SUBTYPE_NO;
}

on head "review review fixes"

> Nikita, please, look at my changes too, they are not minor.

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[n.pettik]

> Tnx. Ok for me, excluding this:
> 
> +++ b/src/box/sql/vdbeapi.c
> @@ -224,8 +224,7 @@ sqlite3_value_int64(sqlite3_value * pVal)
> enum sql_subtype
> sqlite3_value_subtype(sqlite3_value * pVal)
> {
> - Mem *pMem = (Mem *) pVal;
> - return (pMem->flags & MEM_Subtype) != 0 ? pMem->subtype : 0;
> + return (pVal->flags & MEM_Subtype) != 0 ? pVal->subtype : SQL_SUBTYPE_NO;
> }
> 
> on head "review review fixes"
> 
>> Nikita, please, look at my changes too, they are not minor.

I am OK with these fixes (and Kirill’s one too). Thanks.

*Nerd mode on*
However, I don’t stick to the point that introducing separate enum for
one subtype seems to be really vital.
*Nerd mode off*

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[Kirill Shcherbatov]

>>> Nikita, please, look at my changes too, they are not minor.
> I am OK with these fixes (and Kirill’s one too). Thanks.
I've squashed changes.
We are ready to merge it.

[tarantool-patches] Re: [PATCH v1 1/1] sql: show formated MsgPack BLOB on select

[Kirill Yukhin]

Hello,
On 02 июл 15:33, Kirill Shcherbatov wrote:
> Previously MsgPacks BLOB objects were shown as base64 strings.
> Now we print them in human-readable form.
> 
> Resolves #2369.
> ---
> https://github.com/tarantool/tarantool/tree/kshch/gh-2369-display-tuple-msgpucks
> https://github.com/tarantool/tarantool/issues/2369
I've checked the patch into 2.0 branch.

--
Regards, Kirill Yukhin