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

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

> 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")