[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).